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

Lack of DynamicallyAccessedMemberTypes for any default constructor forces keeping unused types #50353

Closed
marek-safar opened this issue Mar 29, 2021 · 5 comments · Fixed by #50390
Labels
area-System.Runtime size-reduction Issues impacting final app size primary for size sensitive workloads

Comments

@marek-safar
Copy link
Contributor

One example is

internal static object CreateInstanceForAnotherGenericParameter([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type genericType, RuntimeType genericArgument)
which has an annotation for any constructor even if only the default constructor is considered. This keeps unnecessary dependencies like System.Runtime.Serialization.StreamingContext via EnumComparer<T> private ctors. I far as I know there is no workaround for this.

/cc @eerhardt @vitek-karas

@marek-safar marek-safar added the size-reduction Issues impacting final app size primary for size sensitive workloads label Mar 29, 2021
@ghost
Copy link

ghost commented Mar 29, 2021

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar, @tannergooding, @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

One example is

internal static object CreateInstanceForAnotherGenericParameter([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type genericType, RuntimeType genericArgument)
which has an annotation for any constructor even if only the default constructor is considered. This keeps unnecessary dependencies like System.Runtime.Serialization.StreamingContext via EnumComparer<T> private ctors. I far as I know there is no workaround for this.

/cc @eerhardt @vitek-karas

Author: marek-safar
Assignees: -
Labels:

size-reduction

Milestone: -

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 29, 2021
@eerhardt
Copy link
Member

Looking through all the callsites of CreateInstanceForAnotherGenericParameter, it looks like either all the types passed into it already have a public parameterless constructor, or they are internal classes, so we can make the constructor public on them.

The only one that doesn't look this way is

// Needs to be public to support binary serialization compatibility
public sealed partial class EnumEqualityComparer<T> : EqualityComparer<T>, ISerializable where T : struct, Enum
{
internal EnumEqualityComparer() { }

But that class isn't public in a contract - just in the implementation, and the comment indicates it needs to be public due to binary serialization. Since this isn't in a public contract, we could make its parameterless constructor public.

Then we can change CreateInstanceForAnotherGenericParameter implementation to only look for public parameterless constructors.

@marek-safar
Copy link
Contributor Author

Then we can change CreateInstanceForAnotherGenericParameter implementation to only look for public parameterless constructors.

I guess that could work.

@eerhardt
Copy link
Member

I can put up a PR after #50232 is merged.

eerhardt added a commit to eerhardt/runtime that referenced this issue Mar 29, 2021
…essConstructor

This allows private constructors on the Types to be trimmed.

Fix dotnet#50353
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 29, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 30, 2021
jkotas pushed a commit that referenced this issue Mar 30, 2021
…essConstructor (#50390)

This allows private constructors on the Types to be trimmed.

Fix #50353
@ghost ghost locked as resolved and limited conversation to collaborators Apr 29, 2021
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants