-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Replace DAMT.All with DAMT.AllMethods in delegate creation #109772
Conversation
Note regarding the
|
1 similar comment
Note regarding the
|
Tagging subscribers to this area: @dotnet/area-system-reflection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks!
Are there any upstream callers that can be changed from All to AllMethods? I’m not sure an easy way to check. It might be nice if the analyzer had an option to alert when you are preserving too much.
I did a search in this repo - we don't use these overloads.
Yup, would be a nice feature! Cc @dotnet/illink |
/ba-g the only not-known-issues are timeouts in unrelated tests |
DynamicallyAccessedMemberTypes.All
can be replaced with the newly addedAllMethods
since it has the right semantics (keep privates in base types).Cc @dotnet/illink @eerhardt