Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DynamicallyAccessedMemberTypes::NonPublicDefaultCtor #1688

Open
marek-safar opened this issue Dec 11, 2020 · 9 comments
Open

Add DynamicallyAccessedMemberTypes::NonPublicDefaultCtor #1688

marek-safar opened this issue Dec 11, 2020 · 9 comments

Comments

@marek-safar
Copy link
Contributor

The default constructors are used via reflection quite often. Right now we have no way to mark any default constructors only. It'd be useful to have it as there are code pattern in SPC which are overmarking types because of this.

The closest pattern today is

[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor | DynamicallyAccessedMemberTypes.NonPublicConstructors)]

@vitek-karas

@vitek-karas
Copy link
Member

How common is this - most reflection APIs only work with public default .ctors.
I understand that this is in corelib, so it affects every app, but I would like to avoid adding new public API (effectively) just for 2 or 3 cases in corelib.

@marek-safar
Copy link
Contributor Author

Is there any alternative? This seems to be very cheap to implement in the linker and we could hide it with special code inside enum if necessary but why if we do have the implementation.

@vitek-karas
Copy link
Member

We could make these couple of places intrinsics.
Why?: If it's really only going to be used from CoreLib (which is pretty likely) then why not hide it

@MichalStrehovsky
Copy link
Member

Out of curiosity - what are the places that need this?

The places I know of are in the "runtime magic" category that wouldn't be possible to annotate (like p/invoke for blittable classes, or safehandles).

@marek-safar
Copy link
Contributor Author

This seems to be mostly used for things that know what type to instantiate and don't want to use Activator (e.g. for performance reasons).

@MichalStrehovsky
Copy link
Member

This seems to be mostly used for things that know what type to instantiate and don't want to use Activator (e.g. for performance reasons).

You mean places like CreateInstanceForAnotherGenericParameter? Those don't run any constructors.

Activator has been recently tuned to be very fast so the internal need for special private APIs will be going away.

The exact use cases would definitely help to see if there's any alternatives.

The PublicParameterlessConstructor has been added to the enum because it closely mirrors what the most common activator APIs (and the new() constraint) does. Basically all the values that are currently there are there to match the reflection API shape.

@vitek-karas
Copy link
Member

@marek-safar do you have a use case for this? I'm leaning towards just closing this.

@marek-safar
Copy link
Contributor Author

I think we changed the affected code because of this limitation to use public constructors but there are still cases like

https://github.com/dotnet/runtime/blob/c2f559cf0d41cc099c8f34e50a68021b4fc0ef8e/src/libraries/System.Data.Common/src/System/Data/DataSet.cs#L1101

which might be worth addressing

@vitek-karas
Copy link
Member

I find this relatively corner case requirement. The DataSet example is valid, but it's relatively weird (the DataSet itself has public parameterless .ctor, so this only applies to derived types, not sure how common it's there).

The downside of doing this is that it requires adding a new public annotation (basically public API) and support it everywhere. The number of use cases seems really low for that.

@sbomer sbomer added this to AppModel Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants