-
Notifications
You must be signed in to change notification settings - Fork 1.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
[Trimming] Enable trimming and AOT analyzers in Core #21076
Merged
+92
−69
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a60a97a
Mark Core as AOT compatible
simonrozsival 6203cc7
Annotate trim and AOT incompatible methods of IImageSourceServiceProv…
simonrozsival 3a8f5a1
Annotate trim and AOT incompatible Hot Reload methods
simonrozsival 985e247
Use generic Enum.GetValues on Tizen
simonrozsival 2227630
Enable analyzers instead of marking the assembly as AOT compatible fo…
simonrozsival b4aa065
Fix resolving bindable properties
simonrozsival f2421a5
Remove GetField and GetFields from ReflectionExtensions
simonrozsival 033e709
Ignore IL2059 in WinUI generated code
simonrozsival 7654738
Fix typo
simonrozsival File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just for future reference:
We've been burned in other projects by adding RUC (and other annotations) onto interface members. The problem is that doing so effectively marks all implementations of the interface with RUC. But RUC is a statement about the implementation, not about the interface.
For example Type.GetMembers - this has some annotation on it, but it's an abstract class. The reflection based implementation needs that annotation, but we also have
MetadataLoadContext
which also implementsType
and that one doesn't need the annotation. But there's no way to tell that to the system and it will produce warnings and so on. Not that we will change Type.GetMembers, but it shows how the statement applies to all implementations, regardless if they are affected or not. (and also, removing the annotation from the interface member is a breaking change!)I guess in this specific case, we basically don't care, since the methods are obsolete anyway. But it's a good rule to remember for the future: "Think hard if the annotation should be on the interface, really hard!"
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.
I struggled with this one. It feels wrong adding those attributes to the interface. I just couldn't figure out how else to apply the annotation in this case. I arrived at the same conclusion that we don't care too much since we already marked it obsolte and we don't know about any customer who would have their custom implementation of IImageSourceProvider at the moment.