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.

Sign up for GitHub

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 proposal for enhanced dataflow analysis #988

Merged
merged 7 commits into from
Apr 30, 2020
Merged

Add proposal for enhanced dataflow analysis #988

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cf009d8
Add proposal for enhanced dataflow analysis
MichalStrehovsky Mar 11, 2020
69c55f6
Update docs/design/reflection-flow.md
MichalStrehovsky Mar 18, 2020
0f9d0e9
/s/safe/friendly
MichalStrehovsky Apr 7, 2020
6e7e143
Add note on intrinsic handling
MichalStrehovsky Apr 7, 2020
e897582
Update docs/design/reflection-flow.md
MichalStrehovsky Apr 9, 2020
417ad0e
Update reflection-flow.md
MichalStrehovsky Apr 9, 2020
acd3597
Update based on API review
MichalStrehovsky Apr 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/design/reflection-flow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,20 @@ class Program
TODO: Creating a delegate to a potentially linker unfriendly method could be solvable. Reflection invoking a potentially linker unfriendly method is hard. Both out of scope?
TODO: It might be possible to apply similar pattern to generic parameters. The DynamizallAccessedMembers could be added to the generic parameter declaration and linker could make sure that when it's "assigned to" the requirements are met.

## Intrinsic recognition of reflection APIs

While it would be possible to annotate reflection primitives with the proposed DynamicallyAccessedMembers attribute, linker is going to intrinsically recongnize some of the reflection primitives so that it can do a better job at preserving just the pieces that are really needed.

* `Type.GetMethod`
* `Type.GetProperty`
* `Type.GetField`
* `Type.GetMember`
* `Type.GetNestedType`
Copy link
Member

Choose a reason for hiding this comment

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

Are GetDeclaredMethod, etc. intentionally missing in this list?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I added this as a response to Vitek's comment - it's not trying to be an exhaustive list. We already e.g. special case methods on System.Linq.Expressions, so this will be a pretty long list. I'll add "for example" because trying to keep this up-to-date would be a vain effort.


Copy link
Member

Choose a reason for hiding this comment

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

"Are going ..." reads like a question, but it is not actually question. Maybe say "The following methods are going to be special cased by linker:" in front of the list instead.

Are going to be special cased so that if the type and name is exactly known at linking time, only the specific member will be preserved. If the name is not known, all matching members are going to be preserved instead. Liker may look at other parameters to these methods, such as the binding flags and parameter counts to further restrict the set of members preserved.
MichalStrehovsky marked this conversation as resolved.
Show resolved Hide resolved

The special casing will also help in situations such as when the type is not statically known and we only have an annotated value - e.g. calling `GetMethod(...BindingFlags.Public)` on a `System.Type` instance annotated as `MemberKinds.PublicMethods` should be considered valid.

## Linker unfriendly annotations

Another annotation will be used to mark methods that are never linker friendly:
Expand Down