-
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 dataflow warnings for invalid trim annotations #106672
Conversation
@@ -191,7 +191,7 @@ void GetDiagnosticsForField (Location location, IFieldSymbol fieldSymbol) | |||
if (fieldSymbol.TryGetRequiresUnreferencedCodeAttribute (out var requiresUnreferencedCodeAttributeData)) | |||
ReportRequiresUnreferencedCodeDiagnostic (location, requiresUnreferencedCodeAttributeData, fieldSymbol); | |||
|
|||
if (fieldSymbol.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None) { | |||
if (FlowAnnotations.GetFieldAnnotation (fieldSymbol) != DynamicallyAccessedMemberTypes.None) { |
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.
Should we change most of the uses of ISymbol.GetDynamicallyAccessedMemberTypes()
to instead use FlowAnnotations.GetXAnnotation()
(except for when we warn on annotations on unsupported types)? Or at least a doc comment on it pointing to the FlowAnnotations
for most use cases.
Also, I think we might still have extra warnings about DAM annotation mismatch property definition and accessor, but that can be a separate issue.
runtime/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Lines 248 to 254 in 7cd266e
if (!overrideMethod.IsStatic && overrideMethod.GetDynamicallyAccessedMemberTypes () != baseMethod.GetDynamicallyAccessedMemberTypes ()) { | |
var methodOrigin = origin ?? overrideMethod; | |
context.ReportDiagnostic (Diagnostic.Create ( | |
DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersMismatchOnImplicitThisBetweenOverrides), | |
GetPrimaryLocation (methodOrigin.Locations), | |
overrideMethod.GetDisplayName (), baseMethod.GetDisplayName ())); | |
} |
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.
Good catch, I fixed the property/accessor mismatch, and one other case (virtual/override mismatch - this was still calling GetDynamicallyAccessedMemberTypes).
- Add comment - Fix remaining uses of GetDynamicallyAccessedMemberTypes outside of invalid annotation warnings This adds some tests for more cases that are fixed now: - Property definition and accessor mismatch with invalid annotations - Virtual and override mismatch with invalid annotations
Fixes #101211