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

Consider targets of delegates reflection-visible #70198

Merged
merged 1 commit into from
Jun 3, 2022

Conversation

MichalStrehovsky
Copy link
Member

This is to make sure Delegate.GetMethodInfo API works in IlcTrimMetadata=true mode. In this mode, the presence of code doesn't automatically mean the method is visible to reflection (we only consider results of dataflow analysis or XML inputs, or DynamicDependency). Add delegate targets to this list so that GetMethodInfo API reliably works.

The compiler already tracks delegate creation sequence, so adding a callback to MetadataManager to inject the dependencies that make the method reflection-visible.

We also handle the situation when the delegate was created to a virtual method and the exact target isn't known until runtime. We do this by injecting conditional dependencies on virtual method implementations.

This change causes a 0.3% size regression on ASP.NET WebApi template with IlcTrimMetadata=true. I spot checked the diffs and they all look correct (there's a lot of delegates being created to support various captures and suddenly those things become reflectable - it's what we want).

This is to make sure `Delegate.GetMethodInfo` API works in `IlcTrimMetadata=true` mode. In this mode, the presence of code doesn't automatically mean the method is visible to reflection (we only consider results of dataflow analysis or XML inputs, or `DynamicDependency`). Add delegate targets to this list so that `GetMethodInfo` API reliably works.

The compiler already tracks delegate creation sequence, so adding a callback to `MetadataManager` to inject the dependencies that make the method reflection-visible.

We also handle the situation when the delegate was created to a virtual method and the exact target isn't known until runtime. We do this by injecting conditional dependencies on virtual method implementations.

This change causes a 0.3% size regression on ASP.NET WebApi template with IlcTrimMetadata=true. I spot checked the diffs and they all look correct (there's a lot of delegates being created to support various captures and suddenly those things become reflectable - it's what we want).
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MichalStrehovsky
Copy link
Member Author

MichalStrehovsky commented Jun 3, 2022

Cc @dotnet/ilc-contrib

This is implementing a thing that IL Linker currently doesn't do:

Action<string> a1 = new Base().Method;
Console.WriteLine(a1.Method.GetParameters()[0].Name);

Action<string> a2 = new Derived().Method;
Console.WriteLine(a2.Method.GetParameters()[0].Name);

class Base
{
    public virtual void Method(string arg) { }
}

class Derived : Base
{
    public override void Method(string arg) { }
}

Will produce a different output after trimming with IL Linker because we stripped the parameter names. The behavioral difference with NativeAOT would be more catastrophic because we strip more than parameter names and implementing this was more urgent.

@MichalStrehovsky MichalStrehovsky merged commit ff8393d into dotnet:main Jun 3, 2022
@MichalStrehovsky MichalStrehovsky deleted the delreflect branch June 3, 2022 12:14
@ghost ghost locked as resolved and limited conversation to collaborators Jul 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants