-
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
Fix bug in virtual annotation logic for RequiresUnreferencedCode #100707
Conversation
Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas |
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar |
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.
What exactly are the semantics of RUC on a type? Is it like putting it on the .ctor and static methods, or all methods, static and instance? LGTM if it's the former, but if it's the latter then the existing behavior doesn't seem right.
src/tools/illink/src/linker/Linker.Steps/ValidateVirtualMethodAnnotationsStep.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jackson Schuster <[email protected]>
It's a bit of both - it will silence warnings inside of all methods, static and instance, but it will only produce warnings for callsites to static methods and constructors. Since we protect constructors, there's no need to warn for calls to virtual methods. By the same reasoning, we don't need to warn for the example above because we've already prevented calls to constructors of the type. |
Failure is known: #100249. I've updated the error text to be slightly looser, but build analysis hasn't matched it with this build yet. |
Ah, it figured it out after a rerun. |
…net#100707) The following virtual methods are correctly annotated and do not warn: ```csharp class Base { virtual void M() {} } [RequiresUnreferencedCode("Derived")] class Derived : Base { override void M() {} } ``` However, if the method also happens to have DynamicallyAccessedMembers annotations, there's a bug in the logic that causes this to produce warnings in illink. This change fixes the bug. Hit this while experimenting with ComponentModel annotations. --------- Co-authored-by: Jackson Schuster <[email protected]>
The following virtual methods are correctly annotated and do not warn:
However, if the method also happens to have DynamicallyAccessedMembers annotations, there's a bug in the logic that causes this to produce warnings in illink. This change fixes the bug.
Hit this while experimenting with ComponentModel annotations.