From bb293b1b030407e2cd30122a1070e7e58df7e59e Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 17 Sep 2025 11:14:59 -0700 Subject: [PATCH 1/7] Don't warn if we can't find the backing field --- .../Compiler/Dataflow/FlowAnnotations.cs | 44 ++++++------------- .../TrimAnalysis/FlowAnnotations.cs | 6 +-- .../linker/Linker.Dataflow/FlowAnnotations.cs | 43 ++++++------------ 3 files changed, 30 insertions(+), 63 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index cddf5abf3deb8a..cc2b2376e5b099 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -592,41 +592,25 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key) } } - if (IsAutoProperty(property)) + bool backingFieldFound = backingFieldFromGetter is not null + || backingFieldFromSetter is not null; + bool mismatchingBackingField = backingFieldFromGetter is not null + && backingFieldFromSetter is not null + && backingFieldFromGetter != backingFieldFromSetter; + + if (backingFieldFound + && !mismatchingBackingField + && IsAutoProperty(property)) { - FieldDesc? backingField = null; - if ((property.SetMethod is not null - && property.SetMethod.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute") - && backingFieldFromSetter is null) - || (property.GetMethod is not null - && property.GetMethod.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute") - && backingFieldFromGetter is null)) + // We either have a single auto-property accessor or both accessors point to the same backing field + FieldDesc backingField = backingFieldFromSetter ?? backingFieldFromGetter!; + if (annotatedFields.Any(a => a.Field == backingField)) { - // We failed to find the backing field of an auto-property accessor - _logger.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); - } - else if (backingFieldFromGetter is not null && backingFieldFromSetter is not null - && backingFieldFromSetter != backingFieldFromGetter) - { - // We found two different backing fields for the getter and the setter - _logger.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); + _logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); } else { - // We either have a single auto-property accessor or both accessors point to the same backing field - backingField = backingFieldFromSetter ?? backingFieldFromGetter; - } - - if (backingField != null) - { - if (annotatedFields.Any(a => a.Field == backingField)) - { - _logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); - } - else - { - annotatedFields.Add(new FieldAnnotation(backingField, annotation)); - } + annotatedFields.Add(new FieldAnnotation(backingField, annotation)); } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs index dd210c6eea3297..5846ce2ca201d5 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs @@ -117,11 +117,11 @@ internal static DynamicallyAccessedMemberTypes GetFieldAnnotation(IFieldSymbol f if (!field.OriginalDefinition.Type.IsTypeInterestingForDataflow(isByRef: field.RefKind is not RefKind.None)) return DynamicallyAccessedMemberTypes.None; - if (field.AssociatedSymbol is IPropertySymbol property) + if (field.AssociatedSymbol is IPropertySymbol property + && property.IsAutoProperty()) { // If this is an auto property, we get the property annotation - if (property.IsAutoProperty()) - return property.GetDynamicallyAccessedMemberTypes(); + return property.GetDynamicallyAccessedMemberTypes(); } return field.GetDynamicallyAccessedMemberTypes(); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index 07471d6b89f728..e98effe342b26b 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -417,41 +417,24 @@ TypeAnnotations BuildTypeAnnotations(TypeDefinition type) } } - if (IsAutoProperty(property)) + bool backingFieldFound = backingFieldFromGetter is not null + || backingFieldFromSetter is not null; + bool mismatchingBackingField = backingFieldFromGetter is not null + && backingFieldFromSetter is not null + && backingFieldFromGetter != backingFieldFromSetter; + + if (backingFieldFound + && !mismatchingBackingField + && IsAutoProperty(property)) { - FieldDefinition? backingField = null; - // If it's an annotated auto-prop, we should be able to find the compiler generated field - if ((property.SetMethod is not null - && property.SetMethod.IsCompilerGenerated() - && backingFieldFromSetter is null) - || (property.GetMethod is not null - && property.GetMethod.IsCompilerGenerated() - && backingFieldFromGetter is null)) + FieldDefinition backingField = backingFieldFromSetter ?? backingFieldFromGetter!; + if (annotatedFields.Any(a => a.Field == backingField)) { - // We failed to find the backing field of an auto-property accessor - _context.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); - } - else if (backingFieldFromGetter is not null && backingFieldFromSetter is not null - && backingFieldFromSetter != backingFieldFromGetter) - { - // We found two different backing fields for the getter and the setter - _context.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); + _context.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); } else { - // We either have a single auto-property accessor or both accessors point to the same backing field - backingField = backingFieldFromSetter ?? backingFieldFromGetter; - } - if (backingField != null) - { - if (annotatedFields.Any(a => a.Field == backingField)) - { - _context.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); - } - else - { - annotatedFields.Add(new FieldAnnotation(backingField, annotation)); - } + annotatedFields.Add(new FieldAnnotation(backingField, annotation)); } } } From 3e1a2d04488c680dc846aace5f86b3ec5ba66d29 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 17 Sep 2025 14:52:29 -0700 Subject: [PATCH 2/7] Re-add warning for mismatched backing fields found in get vs set --- .../Compiler/Dataflow/FlowAnnotations.cs | 19 ++++++++----------- .../linker/Linker.Dataflow/FlowAnnotations.cs | 19 +++++++++---------- .../DataFlow/PropertyDataFlow.cs | 8 -------- 3 files changed, 17 insertions(+), 29 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index cc2b2376e5b099..a8c60154473e6f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -592,22 +592,19 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key) } } - bool backingFieldFound = backingFieldFromGetter is not null - || backingFieldFromSetter is not null; - bool mismatchingBackingField = backingFieldFromGetter is not null - && backingFieldFromSetter is not null - && backingFieldFromGetter != backingFieldFromSetter; - - if (backingFieldFound - && !mismatchingBackingField - && IsAutoProperty(property)) + FieldDesc backingField = backingFieldFromSetter ?? backingFieldFromGetter!; + if (backingField is not null) { - // We either have a single auto-property accessor or both accessors point to the same backing field - FieldDesc backingField = backingFieldFromSetter ?? backingFieldFromGetter!; if (annotatedFields.Any(a => a.Field == backingField)) { _logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); } + else if (backingFieldFromGetter is not null + && backingFieldFromSetter is not null + && backingFieldFromGetter != backingFieldFromSetter) + { + _logger.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); + } else { annotatedFields.Add(new FieldAnnotation(backingField, annotation)); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index e98effe342b26b..9d9c726a93cd90 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -417,21 +417,20 @@ TypeAnnotations BuildTypeAnnotations(TypeDefinition type) } } - bool backingFieldFound = backingFieldFromGetter is not null - || backingFieldFromSetter is not null; - bool mismatchingBackingField = backingFieldFromGetter is not null - && backingFieldFromSetter is not null - && backingFieldFromGetter != backingFieldFromSetter; - - if (backingFieldFound - && !mismatchingBackingField - && IsAutoProperty(property)) + + FieldDefinition backingField = backingFieldFromSetter ?? backingFieldFromGetter!; + if (backingField is not null) { - FieldDefinition backingField = backingFieldFromSetter ?? backingFieldFromGetter!; if (annotatedFields.Any(a => a.Field == backingField)) { _context.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); } + else if (backingFieldFromGetter is not null + && backingFieldFromSetter is not null + && backingFieldFromGetter != backingFieldFromSetter) + { + _context.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); + } else { annotatedFields.Add(new FieldAnnotation(backingField, annotation)); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs index 1fc8ed65bc8379..23544286221e7a 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs @@ -959,7 +959,6 @@ class AutoPropertyUnrecognizedField private Type Property_BackingField; [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - [ExpectedWarning("IL2042", Tool.NativeAot | Tool.Trimmer, "Requires IL")] // Can't find backing field public Type Property { [CompilerGenerated] @@ -980,7 +979,6 @@ public Type Property } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - [ExpectedWarning("IL2042", Tool.NativeAot | Tool.Trimmer, "Requires IL")] // Can't find backing field public Type PropertyAutoSet { [CompilerGenerated] @@ -995,10 +993,8 @@ public Type PropertyAutoSet } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - [ExpectedWarning("IL2042", Tool.NativeAot | Tool.Trimmer, "Requires IL")] // Can't find backing field public Type PropertyAutoGet { - [ExpectedWarning("IL2078", ["return value", nameof(PropertyAutoGet), "BackingField"], producedBy: Tool.NativeAot | Tool.Trimmer, "Requires IL")] get; [CompilerGenerated] set @@ -1010,7 +1006,6 @@ public Type PropertyAutoGet } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - [ExpectedWarning("IL2042", Tool.NativeAot | Tool.Trimmer, "Requires IL")] // Can't find backing field public Type PropertyManualSet { [CompilerGenerated] @@ -1025,7 +1020,6 @@ public Type PropertyManualSet } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - [ExpectedWarning("IL2042", Tool.NativeAot | Tool.Trimmer, "Requires IL")] // Can't find backing field public Type PropertyManualGet { [ExpectedWarning("IL2078", "return value", nameof(Property_BackingField))] @@ -1040,7 +1034,6 @@ public Type PropertyManualGet } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - [ExpectedWarning("IL2042", Tool.NativeAot | Tool.Trimmer, "Requires IL")] // Can't find backing field public Type PropertyOnlyGet { [CompilerGenerated] @@ -1054,7 +1047,6 @@ public Type PropertyOnlyGet } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - [ExpectedWarning("IL2042", nameof(PropertyOnlySet), Tool.NativeAot | Tool.Trimmer, "Requires IL")] // Can't find backing field public Type PropertyOnlySet { [CompilerGenerated] From 942cb0632f558547366d185a4a423041572f11ab Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 17 Sep 2025 15:20:59 -0700 Subject: [PATCH 3/7] Don't warn for mismatching backing fields --- .../Compiler/Dataflow/FlowAnnotations.cs | 8 ++------ src/tools/illink/src/ILLink.Shared/DiagnosticId.cs | 2 +- .../illink/src/linker/Linker.Dataflow/FlowAnnotations.cs | 8 ++------ .../Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs | 2 -- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index a8c60154473e6f..6c5ea28f030e42 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -599,13 +599,9 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key) { _logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); } - else if (backingFieldFromGetter is not null + else if (!(backingFieldFromGetter is not null && backingFieldFromSetter is not null - && backingFieldFromGetter != backingFieldFromSetter) - { - _logger.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); - } - else + && backingFieldFromGetter != backingFieldFromSetter)) { annotatedFields.Add(new FieldAnnotation(backingField, annotation)); } diff --git a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs index dba2ccee7ad288..a1e0e3bc04ee91 100644 --- a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs +++ b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs @@ -101,7 +101,7 @@ public enum DiagnosticId XmlInvalidValueForAttributeActionForResource = 2039, XmlCouldNotFindResourceToRemoveInAssembly = 2040, DynamicallyAccessedMembersIsNotAllowedOnMethods = 2041, - DynamicallyAccessedMembersCouldNotFindBackingField = 2042, + unused_DynamicallyAccessedMembersCouldNotFindBackingField = 2042, DynamicallyAccessedMembersConflictsBetweenPropertyAndAccessor = 2043, XmlCouldNotFindAnyTypeInNamespace = 2044, AttributeIsReferencedButTrimmerRemoveAllInstances = 2045, diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index 9d9c726a93cd90..cf49b42203342f 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -425,13 +425,9 @@ TypeAnnotations BuildTypeAnnotations(TypeDefinition type) { _context.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); } - else if (backingFieldFromGetter is not null + if (!(backingFieldFromGetter is not null && backingFieldFromSetter is not null - && backingFieldFromGetter != backingFieldFromSetter) - { - _context.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName()); - } - else + && backingFieldFromGetter != backingFieldFromSetter)) { annotatedFields.Add(new FieldAnnotation(backingField, annotation)); } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs index 23544286221e7a..9aa451b0723903 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs @@ -288,8 +288,6 @@ public void TestPropertyWithDifferentBackingFields() private Type PropertyWithDifferentBackingFields_SetterField; // Analyzer doesn't try to detect backing fields of properties: https://github.com/dotnet/linker/issues/2273 - [ExpectedWarning("IL2042", - "Mono.Linker.Tests.Cases.DataFlow.PropertyDataFlow.TestAutomaticPropagationType.PropertyWithDifferentBackingFields", Tool.Trimmer | Tool.NativeAot, "Requires IL")] [ExpectedWarning("IL2078", nameof(TestAutomaticPropagationType) + "." + nameof(PropertyWithDifferentBackingFields) + ".get", "Type", Tool.Analyzer, "")] From 5f86950e3cb668540383d2c7574d14029d694288 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 17 Sep 2025 16:07:22 -0700 Subject: [PATCH 4/7] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index cf49b42203342f..43a57e2b15d405 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -418,7 +418,7 @@ TypeAnnotations BuildTypeAnnotations(TypeDefinition type) } - FieldDefinition backingField = backingFieldFromSetter ?? backingFieldFromGetter!; + FieldDefinition? backingField = backingFieldFromSetter ?? backingFieldFromGetter; if (backingField is not null) { if (annotatedFields.Any(a => a.Field == backingField)) From dc02a3d32b43d7cc344f959e98a05b34ce305ca7 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 17 Sep 2025 16:07:37 -0700 Subject: [PATCH 5/7] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index 6c5ea28f030e42..1043b7cfad0aa7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -592,7 +592,7 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key) } } - FieldDesc backingField = backingFieldFromSetter ?? backingFieldFromGetter!; + FieldDesc? backingField = backingFieldFromSetter ?? backingFieldFromGetter; if (backingField is not null) { if (annotatedFields.Any(a => a.Field == backingField)) From 22efb83634557dcb6f7f8fc77d986288c591fc93 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Thu, 18 Sep 2025 11:07:10 -0700 Subject: [PATCH 6/7] PR Feedback: only check for existing backing field annotation when there is a single valid field --- .../Compiler/Dataflow/FlowAnnotations.cs | 21 +++++--- .../linker/Linker.Dataflow/FlowAnnotations.cs | 21 +++++--- .../DataFlow/PropertyDataFlow.cs | 51 ++++++++++++++----- 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index 1043b7cfad0aa7..5333c2bd02af3d 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -595,15 +595,20 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key) FieldDesc? backingField = backingFieldFromSetter ?? backingFieldFromGetter; if (backingField is not null) { - if (annotatedFields.Any(a => a.Field == backingField)) + bool validBackingFieldFound = backingFieldFromGetter is null + || backingFieldFromSetter is null + || backingFieldFromGetter == backingFieldFromSetter; + if (validBackingFieldFound) { - _logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); - } - else if (!(backingFieldFromGetter is not null - && backingFieldFromSetter is not null - && backingFieldFromGetter != backingFieldFromSetter)) - { - annotatedFields.Add(new FieldAnnotation(backingField, annotation)); + if (annotatedFields.Any(a => a.Field == backingField && a.Annotation != annotation)) + { + _logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); + } + else + { + // Unique backing field with no conflicts with property or existing field + annotatedFields.Add(new FieldAnnotation(backingField, annotation)); + } } } } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index 43a57e2b15d405..1d24ffaff6b936 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -421,15 +421,20 @@ TypeAnnotations BuildTypeAnnotations(TypeDefinition type) FieldDefinition? backingField = backingFieldFromSetter ?? backingFieldFromGetter; if (backingField is not null) { - if (annotatedFields.Any(a => a.Field == backingField)) + bool validBackingFieldFound = backingFieldFromGetter is null + || backingFieldFromSetter is null + || backingFieldFromGetter == backingFieldFromSetter; + if (validBackingFieldFound) { - _context.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); - } - if (!(backingFieldFromGetter is not null - && backingFieldFromSetter is not null - && backingFieldFromGetter != backingFieldFromSetter)) - { - annotatedFields.Add(new FieldAnnotation(backingField, annotation)); + if (annotatedFields.Any(a => a.Field == backingField && a.Annotation != annotation)) + { + _context.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName()); + } + else + { + // Unique backing field with no conflicts with property or existing field + annotatedFields.Add(new FieldAnnotation(backingField, annotation)); + } } } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs index 9aa451b0723903..131a41b4d53a90 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs @@ -311,32 +311,57 @@ Type PropertyWithDifferentBackingFields public void TestPropertyWithExistingAttributes() { - _ = PropertyWithExistingAttributes; - PropertyWithExistingAttributes = null; + _ = PropertyWithExistingMatchingAttributes; + PropertyWithExistingMatchingAttributes = null; + _ = PropertyWithExistingMismatchedAttributes; + PropertyWithExistingMismatchedAttributes = null; } // Analyzer doesn't try to detect backing fields of properties: https://github.com/dotnet/linker/issues/2273 - [ExpectedWarning("IL2056", "PropertyWithExistingAttributes", "PropertyWithExistingAttributes_Field", Tool.Trimmer | Tool.NativeAot, "")] [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] [CompilerGenerated] - Type PropertyWithExistingAttributes_Field; + Type PropertyWithExistingMatchingAttributes_Field; - [ExpectedWarning("IL2043", ["PropertyWithExistingAttributes", "PropertyWithExistingAttributes.get"], Tool.Analyzer, "")] - [ExpectedWarning("IL2043", ["PropertyWithExistingAttributes", "PropertyWithExistingAttributes.set"], Tool.Analyzer, "")] + [ExpectedWarning("IL2043", ["PropertyWithExistingMatchingAttributes", "PropertyWithExistingMatchingAttributes.get"], Tool.Analyzer, "")] + [ExpectedWarning("IL2043", ["PropertyWithExistingMatchingAttributes", "PropertyWithExistingMatchingAttributes.set"], Tool.Analyzer, "")] [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] - Type PropertyWithExistingAttributes + Type PropertyWithExistingMatchingAttributes { - // On property/accessor mismatch, ILLink warns on accessor and analyzer warns on property https://github.com/dotnet/linker/issues/2654 - [ExpectedWarning("IL2043", "PropertyWithExistingAttributes", "PropertyWithExistingAttributes.get", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2043", ["PropertyWithExistingMatchingAttributes", "PropertyWithExistingMatchingAttributes.get"], Tool.Trimmer | Tool.NativeAot, "")] [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] [CompilerGenerated] - get { return PropertyWithExistingAttributes_Field; } + get { return PropertyWithExistingMatchingAttributes_Field; } - // On property/accessor mismatch, ILLink warns on accessor and analyzer warns on property https://github.com/dotnet/linker/issues/2654 - [ExpectedWarning("IL2043", "PropertyWithExistingAttributes", "PropertyWithExistingAttributes.set", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2043", ["PropertyWithExistingMatchingAttributes", "PropertyWithExistingMatchingAttributes.set"], Tool.Trimmer | Tool.NativeAot, "")] [param: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] [CompilerGenerated] - set { PropertyWithExistingAttributes_Field = value; } + set { PropertyWithExistingMatchingAttributes_Field = value; } + } + + // Analyzer doesn't try to detect backing fields of properties: https://github.com/dotnet/linker/issues/2273 + [ExpectedWarning("IL2056", "PropertyWithExistingMismatchedAttributes", "PropertyWithExistingMismatchedAttributes_Field", Tool.Trimmer | Tool.NativeAot, "")] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] + [CompilerGenerated] + Type PropertyWithExistingMismatchedAttributes_Field; + + [ExpectedWarning("IL2043", ["PropertyWithExistingMismatchedAttributes", "PropertyWithExistingMismatchedAttributes.get"], Tool.Analyzer, "")] + [ExpectedWarning("IL2043", ["PropertyWithExistingMismatchedAttributes", "PropertyWithExistingMismatchedAttributes.set"], Tool.Analyzer, "")] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] + Type PropertyWithExistingMismatchedAttributes + { + // On property/accessor mismatch, ILLink warns on accessor and analyzer warns on property https://github.com/dotnet/linker/issues/2654 + [ExpectedWarning("IL2043", "PropertyWithExistingMismatchedAttributes", "PropertyWithExistingMismatchedAttributes.get", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2078", "return value", "PropertyWithExistingMismatchedAttributes_Field", Tool.Trimmer | Tool.NativeAot, "")] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] + [CompilerGenerated] + get { return PropertyWithExistingMismatchedAttributes_Field; } + + // On property/accessor mismatch, ILLink warns on accessor and analyzer warns on property https://github.com/dotnet/linker/issues/2654 + [ExpectedWarning("IL2043", "PropertyWithExistingMismatchedAttributes", "PropertyWithExistingMismatchedAttributes.set", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2069", "PropertyWithExistingMismatchedAttributes_Field", "parameter 'value'", Tool.Trimmer | Tool.NativeAot, "")] + [param: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] + [CompilerGenerated] + set { PropertyWithExistingMismatchedAttributes_Field = value; } } // When the property annotation conflicts with the getter/setter annotation, From 6c6c9995c6680036f660180924780a4b0547d84f Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Thu, 18 Sep 2025 14:19:21 -0700 Subject: [PATCH 7/7] Fix analyzer tests --- .../test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs index 131a41b4d53a90..7b0272a85de43a 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs @@ -351,14 +351,14 @@ Type PropertyWithExistingMismatchedAttributes { // On property/accessor mismatch, ILLink warns on accessor and analyzer warns on property https://github.com/dotnet/linker/issues/2654 [ExpectedWarning("IL2043", "PropertyWithExistingMismatchedAttributes", "PropertyWithExistingMismatchedAttributes.get", Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2078", "return value", "PropertyWithExistingMismatchedAttributes_Field", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2078", "return value", "PropertyWithExistingMismatchedAttributes_Field")] [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] [CompilerGenerated] get { return PropertyWithExistingMismatchedAttributes_Field; } // On property/accessor mismatch, ILLink warns on accessor and analyzer warns on property https://github.com/dotnet/linker/issues/2654 [ExpectedWarning("IL2043", "PropertyWithExistingMismatchedAttributes", "PropertyWithExistingMismatchedAttributes.set", Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2069", "PropertyWithExistingMismatchedAttributes_Field", "parameter 'value'", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2069", "PropertyWithExistingMismatchedAttributes_Field", "parameter 'value'")] [param: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] [CompilerGenerated] set { PropertyWithExistingMismatchedAttributes_Field = value; }