From ff9216e18c9c9e6b595fe9ac1422cc23da07b9b1 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 5 Aug 2021 00:13:40 +0000 Subject: [PATCH 1/5] Fix warnings for DAM.All Fixes https://github.com/mono/linker/issues/2159 --- .../DynamicallyAccessedMembersBinder.cs | 74 +++++++++++++++++-- .../ReflectionMethodBodyScanner.cs | 7 +- src/linker/Linker.Steps/MarkStep.cs | 55 +++++--------- .../DynamicDependencyMemberTypes.cs | 15 +++- 4 files changed, 99 insertions(+), 52 deletions(-) diff --git a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs index a176f5e0eb64..ea71dba85c55 100644 --- a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs @@ -19,13 +19,13 @@ internal static class DynamicallyAccessedMemberTypesOverlay internal static class DynamicallyAccessedMembersBinder { - // Returns the members of the type bound by memberTypes. For DynamicallyAccessedMemberTypes.All, this returns a single null result. - // This sentinel value allows callers to handle the case where DynamicallyAccessedMemberTypes.All conceptually binds to the entire type - // including all recursive nested members. + // Returns the members of the type bound by memberTypes. For DynamicallyAccessedMemberTypes.All, this returns all members of the type and its + // nested types, plus the same or any base types or interfaces. The behavior for nested types public static IEnumerable GetDynamicallyAccessedMembers (this TypeDefinition typeDefinition, LinkContext context, DynamicallyAccessedMemberTypes memberTypes) { if (memberTypes == DynamicallyAccessedMemberTypes.All) { - yield return null; + foreach (var m in typeDefinition.GetAllOnType (context)) + yield return m; yield break; } @@ -65,13 +65,19 @@ public static IEnumerable GetDynamicallyAccessedMembers } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)) { - foreach (var t in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.NonPublic)) - yield return t; + foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.NonPublic)) { + yield return nested; + foreach (var m in nested.GetAllOnType (context)) + yield return m; + } } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicNestedTypes)) { - foreach (var t in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.Public)) - yield return t; + foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.Public)) { + yield return nested; + foreach (var m in nested.GetAllOnType (context)) + yield return m; + } } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicProperties)) { @@ -327,5 +333,57 @@ public static IEnumerable GetAllInterfaceImplementation type = context.TryResolve (type.BaseType); } } + + public static IEnumerable GetAllOnType (this TypeDefinition type, LinkContext context) => GetAllOnType (type, context, new HashSet ()); + + static IEnumerable GetAllOnType (TypeDefinition type, LinkContext context, HashSet types) + { + if (!types.Add (type)) + yield break; + + if (type.HasNestedTypes) { + foreach (var nested in type.NestedTypes) { + yield return nested; + foreach (var m in GetAllOnType (nested, context, types)) + yield return m; + } + } + + var baseType = context.TryResolve (type.BaseType); + if (baseType != null) { + foreach (var m in GetAllOnType (baseType, context, types)) + yield return m; + } + + if (type.HasInterfaces) { + foreach (var iface in type.Interfaces) { + var interfaceType = context.Resolve (iface.InterfaceType); + foreach (var m in GetAllOnType (interfaceType, context, types)) + yield return m; + + yield return iface; + } + } + + if (type.HasFields) { + foreach (var f in type.Fields) + yield return f; + } + + if (type.HasMethods) { + foreach (var m in type.Methods) + yield return m; + } + + if (type.HasProperties) { + foreach (var p in type.Properties) + yield return p; + } + + if (type.HasEvents) { + foreach (var e in type.Events) + yield return e; + } + } } } \ No newline at end of file diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index a460fb5d2147..76519a3d9c30 100644 --- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -2296,8 +2296,7 @@ void MarkTypeForDynamicallyAccessedMembers (ref ReflectionPatternContext reflect MarkField (ref reflectionContext, field, dependencyKind); break; case TypeDefinition nestedType: - DependencyInfo nestedDependencyInfo = new DependencyInfo (dependencyKind, reflectionContext.Source); - reflectionContext.RecordRecognizedPattern (nestedType, () => _markStep.MarkEntireType (nestedType, includeBaseAndInterfaceTypes: true, nestedDependencyInfo)); + MarkType (ref reflectionContext, nestedType, dependencyKind); break; case PropertyDefinition property: MarkProperty (ref reflectionContext, property, dependencyKind); @@ -2308,10 +2307,6 @@ void MarkTypeForDynamicallyAccessedMembers (ref ReflectionPatternContext reflect case InterfaceImplementation interfaceImplementation: MarkInterfaceImplementation (ref reflectionContext, interfaceImplementation, dependencyKind); break; - case null: - DependencyInfo dependencyInfo = new DependencyInfo (dependencyKind, reflectionContext.Source); - reflectionContext.RecordRecognizedPattern (typeDefinition, () => _markStep.MarkEntireType (typeDefinition, includeBaseAndInterfaceTypes: true, dependencyInfo)); - break; } } } diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index d7b9e31b5d34..93297e78c73d 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -59,7 +59,7 @@ public partial class MarkStep : IStep readonly List<(TypeDefinition Type, MethodBody Body, Instruction Instr)> _pending_isinst_instr; UnreachableBlocksOptimizer _unreachableBlocksOptimizer; MarkStepContext _markContext; - readonly Dictionary _entireTypesMarked; // The value is markBaseAndInterfaceTypes flag used to mark the type + readonly HashSet _entireTypesMarked; DynamicallyAccessedMembersTypeHierarchy _dynamicallyAccessedMembersTypeHierarchy; MarkScopeStack _scopeStack; @@ -71,9 +71,7 @@ internal DynamicallyAccessedMembersTypeHierarchy DynamicallyAccessedMembersTypeH static readonly DependencyKind[] _entireTypeReasons = new DependencyKind[] { DependencyKind.AccessedViaReflection, DependencyKind.BaseType, - DependencyKind.DynamicallyAccessedMember, - DependencyKind.DynamicallyAccessedMemberOnType, - DependencyKind.DynamicDependency, + DependencyKind.PreservedDependency, DependencyKind.NestedType, DependencyKind.TypeInAssembly, DependencyKind.Unspecified, @@ -196,7 +194,7 @@ public MarkStep () _dynamicInterfaceCastableImplementationTypes = new List (); _unreachableBodies = new List<(MethodBody, MarkScopeStack.Scope)> (); _pending_isinst_instr = new List<(TypeDefinition, MethodBody, Instruction)> (); - _entireTypesMarked = new Dictionary (); + _entireTypesMarked = new HashSet (); } public AnnotationStore Annotations => _context.Annotations; @@ -311,77 +309,63 @@ protected bool IsFullyPreserved (TypeDefinition type) return false; } - internal void MarkEntireType (TypeDefinition type, bool includeBaseAndInterfaceTypes, in DependencyInfo reason) + internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason) { // Prevent cases where there's nothing on the stack (can happen when marking entire assemblies) // In which case we would generate warnings with no source (hard to debug) using var _ = _scopeStack.CurrentScope.Origin.MemberDefinition == null ? _scopeStack.PushScope (new MessageOrigin (type)) : null; - MarkEntireTypeInternal (type, includeBaseAndInterfaceTypes, reason); + MarkEntireTypeInternal (type, reason); } - void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseAndInterfaceTypes, in DependencyInfo reason) + void MarkEntireTypeInternal (TypeDefinition type, in DependencyInfo reason) { #if DEBUG if (!_entireTypeReasons.Contains (reason.Kind)) throw new InternalErrorException ($"Unsupported type dependency '{reason.Kind}'."); #endif - if (_entireTypesMarked.TryGetValue (type, out bool alreadyIncludedBaseAndInterfaceTypes) && - (!includeBaseAndInterfaceTypes || alreadyIncludedBaseAndInterfaceTypes)) + if (!_entireTypesMarked.Add (type)) return; - _entireTypesMarked[type] = includeBaseAndInterfaceTypes; - - bool isDynamicDependencyReason = reason.Kind == DependencyKind.DynamicallyAccessedMember || reason.Kind == DependencyKind.DynamicDependency || reason.Kind == DependencyKind.DynamicallyAccessedMemberOnType; - if (type.HasNestedTypes) { foreach (TypeDefinition nested in type.NestedTypes) - MarkEntireTypeInternal (nested, includeBaseAndInterfaceTypes, new DependencyInfo (isDynamicDependencyReason ? reason.Kind : DependencyKind.NestedType, type)); + MarkEntireTypeInternal (nested, new DependencyInfo (DependencyKind.NestedType, type)); } Annotations.Mark (type, reason); - var baseTypeDefinition = _context.Resolve (type.BaseType); - if (includeBaseAndInterfaceTypes && baseTypeDefinition != null) { - MarkEntireTypeInternal (baseTypeDefinition, true, new DependencyInfo (isDynamicDependencyReason ? reason.Kind : DependencyKind.BaseType, type)); - } - MarkCustomAttributes (type, new DependencyInfo (isDynamicDependencyReason ? reason.Kind : DependencyKind.CustomAttribute, type)); + MarkCustomAttributes (type, new DependencyInfo (DependencyKind.CustomAttribute, type)); MarkTypeSpecialCustomAttributes (type); if (type.HasInterfaces) { - foreach (InterfaceImplementation iface in type.Interfaces) { - var interfaceTypeDefinition = _context.Resolve (iface.InterfaceType); - if (includeBaseAndInterfaceTypes && interfaceTypeDefinition != null) - MarkEntireTypeInternal (interfaceTypeDefinition, true, new DependencyInfo (reason.Kind, type)); - + foreach (InterfaceImplementation iface in type.Interfaces) MarkInterfaceImplementation (iface, new MessageOrigin (type)); - } } MarkGenericParameterProvider (type); if (type.HasFields) { foreach (FieldDefinition field in type.Fields) { - MarkField (field, new DependencyInfo (isDynamicDependencyReason ? reason.Kind : DependencyKind.MemberOfType, type)); + MarkField (field, new DependencyInfo (DependencyKind.MemberOfType, type)); } } if (type.HasMethods) { foreach (MethodDefinition method in type.Methods) { Annotations.SetAction (method, MethodAction.ForceParse); - MarkMethod (method, new DependencyInfo (isDynamicDependencyReason ? reason.Kind : DependencyKind.MemberOfType, type)); + MarkMethod (method, new DependencyInfo (DependencyKind.MemberOfType, type)); } } if (type.HasProperties) { foreach (var property in type.Properties) { - MarkProperty (property, new DependencyInfo (isDynamicDependencyReason ? reason.Kind : DependencyKind.MemberOfType, type)); + MarkProperty (property, new DependencyInfo (DependencyKind.MemberOfType, type)); } } if (type.HasEvents) { foreach (var ev in type.Events) { - MarkEvent (ev, new DependencyInfo (isDynamicDependencyReason ? reason.Kind : DependencyKind.MemberOfType, type)); + MarkEvent (ev, new DependencyInfo (DependencyKind.MemberOfType, type)); } } } @@ -944,10 +928,10 @@ void MarkDynamicDependency (DynamicDependency dynamicDependency, IMemberDefiniti } } - MarkMembersVisibleToReflection (type, members, new DependencyInfo (DependencyKind.DynamicDependency, dynamicDependency.OriginalAttribute)); + MarkMembersVisibleToReflection (members, new DependencyInfo (DependencyKind.DynamicDependency, dynamicDependency.OriginalAttribute)); } - void MarkMembersVisibleToReflection (TypeDefinition typeDefinition, IEnumerable members, in DependencyInfo reason) + void MarkMembersVisibleToReflection (IEnumerable members, in DependencyInfo reason) { foreach (var member in members) { switch (member) { @@ -969,9 +953,6 @@ void MarkMembersVisibleToReflection (TypeDefinition typeDefinition, IEnumerable< case InterfaceImplementation interfaceType: MarkInterfaceImplementation (interfaceType, null, reason); break; - case null: - MarkEntireType (typeDefinition, includeBaseAndInterfaceTypes: true, reason); - break; } } } @@ -1033,7 +1014,7 @@ protected virtual void MarkUserDependency (IMemberDefinition context, CustomAttr } if (member == "*") { - MarkEntireType (td, includeBaseAndInterfaceTypes: false, new DependencyInfo (DependencyKind.PreservedDependency, ca)); + MarkEntireType (td, new DependencyInfo (DependencyKind.PreservedDependency, ca)); return; } @@ -1411,7 +1392,7 @@ void MarkEntireAssembly (AssemblyDefinition assembly) MarkCustomAttributes (module, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, module)); foreach (TypeDefinition type in module.Types) - MarkEntireType (type, includeBaseAndInterfaceTypes: false, new DependencyInfo (DependencyKind.TypeInAssembly, assembly)); + MarkEntireType (type, new DependencyInfo (DependencyKind.TypeInAssembly, assembly)); foreach (ExportedType exportedType in module.ExportedTypes) { MarkingHelpers.MarkExportedType (exportedType, module, new DependencyInfo (DependencyKind.ExportedType, assembly)); diff --git a/test/Mono.Linker.Tests.Cases/DynamicDependencies/DynamicDependencyMemberTypes.cs b/test/Mono.Linker.Tests.Cases/DynamicDependencies/DynamicDependencyMemberTypes.cs index f4bfca071d4b..5ba9d6c25a66 100644 --- a/test/Mono.Linker.Tests.Cases/DynamicDependencies/DynamicDependencyMemberTypes.cs +++ b/test/Mono.Linker.Tests.Cases/DynamicDependencies/DynamicDependencyMemberTypes.cs @@ -119,12 +119,17 @@ void Method () { } class TypeWithPublicNestedType { [Kept] + [KeptMember (".ctor()")] public class PublicNestedType { + [Kept] public void Method () { } + [Kept] public int field; - + [Kept] void NonPublicMethod () { } + [Kept] + static class EmptyInnerNestedType { } } public void Method () { } @@ -139,6 +144,8 @@ class NonPublicNestedType [KeptBaseType (typeof (MulticastDelegate))] [KeptMember (".ctor(System.Object,System.IntPtr)")] [KeptMember ("Invoke()")] + [KeptMember ("BeginInvoke(System.AsyncCallback,System.Object)")] + [KeptMember ("EndInvoke(System.IAsyncResult)")] public delegate int PublicDelegate (); private delegate int PrivateDelegate (); @@ -148,12 +155,16 @@ class NonPublicNestedType class TypeWithNonPublicNestedType { [Kept] + [KeptMember (".ctor()")] class NonPublicNestedType { + [Kept] public void Method () { } + [Kept] public int field; + [Kept] void NonPublicMethod () { } } @@ -171,6 +182,8 @@ public class PublicNestedType [KeptBaseType (typeof (MulticastDelegate))] [KeptMember (".ctor(System.Object,System.IntPtr)")] [KeptMember ("Invoke()")] + [KeptMember ("BeginInvoke(System.AsyncCallback,System.Object)")] + [KeptMember ("EndInvoke(System.IAsyncResult)")] private delegate int PrivateDelegate (); } From 7692811cb5a77092eabc0a49e2aba45249d94a80 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 5 Aug 2021 17:19:59 +0000 Subject: [PATCH 2/5] Add tests --- .../DynamicallyAccessedMembersBinder.cs | 6 +- .../AnnotatedMembersAccessedViaReflection.cs | 97 +++++++++++++++++++ .../TypeHierarchyReflectionWarnings.cs | 24 +++++ 3 files changed, 124 insertions(+), 3 deletions(-) diff --git a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs index ea71dba85c55..4339af8a7924 100644 --- a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs @@ -20,7 +20,8 @@ internal static class DynamicallyAccessedMemberTypesOverlay internal static class DynamicallyAccessedMembersBinder { // Returns the members of the type bound by memberTypes. For DynamicallyAccessedMemberTypes.All, this returns all members of the type and its - // nested types, plus the same or any base types or interfaces. The behavior for nested types + // nested types, including interface implementations, plus the same or any base types or implemented interfaces. + // DynamicallyAccessedMemberTypes.PublicNestedTypes and NonPublicNestedTypes do the same for members of the selected nested types. public static IEnumerable GetDynamicallyAccessedMembers (this TypeDefinition typeDefinition, LinkContext context, DynamicallyAccessedMemberTypes memberTypes) { if (memberTypes == DynamicallyAccessedMemberTypes.All) { @@ -357,11 +358,10 @@ static IEnumerable GetAllOnType (TypeDefinition type, Li if (type.HasInterfaces) { foreach (var iface in type.Interfaces) { + yield return iface; var interfaceType = context.Resolve (iface.InterfaceType); foreach (var m in GetAllOnType (interfaceType, context, types)) yield return m; - - yield return iface; } } diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs b/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs index f94b9a45867e..4b626431f79a 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs @@ -84,6 +84,38 @@ static void DynamicallyAccessedMembersSuppressedByRUC () typeof (AnnotatedField).RequiresPublicFields (); } + class NestedType + { + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + public static Type _annotatedField; + } + + [ExpectedWarning ("IL2110", nameof (_annotatedField))] + [ExpectedWarning ("IL2026", "test")] + static void DynamicallyAccessedMembersAll1 () + { + typeof (AnnotatedField).RequiresAll (); + } + + [ExpectedWarning ("IL2110", nameof (_annotatedField))] + [ExpectedWarning ("IL2026", "test")] + static void DynamicallyAccessedMembersAll2 () + { + typeof (AnnotatedField).RequiresAll (); + } + + [ExpectedWarning ("IL2110", nameof (NestedType), nameof (NestedType._annotatedField))] + static void DynamicallyAccessedMembersNestedTypes1 () + { + typeof (AnnotatedField).RequiresNonPublicNestedTypes (); + } + + [ExpectedWarning ("IL2110", nameof (NestedType), nameof (NestedType._annotatedField))] + static void DynamicallyAccessedMembersNestedTypes2 () + { + typeof (AnnotatedField).RequiresNonPublicNestedTypes (); + } + [UnconditionalSuppressMessage ("test", "IL2026")] public static void Test () { @@ -95,6 +127,10 @@ public static void Test () DynamicDependencyByName (); DynamicallyAccessedMembers (); DynamicallyAccessedMembersSuppressedByRUC (); + DynamicallyAccessedMembersAll1 (); + DynamicallyAccessedMembersAll2 (); + DynamicallyAccessedMembersNestedTypes1 (); + DynamicallyAccessedMembersNestedTypes2 (); } } @@ -177,6 +213,22 @@ static void Ldvirtftn () var _ = new Action (instance.AnnotatedMethod); } + [ExpectedWarning ("IL2111", nameof (MethodWithSingleAnnotatedParameter))] + [ExpectedWarning ("IL2026", "test")] + [ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))] + static void DynamicallyAccessedMembersAll1 () + { + typeof (AnnotatedMethodParameters).RequiresAll (); + } + + [ExpectedWarning ("IL2111", nameof (MethodWithSingleAnnotatedParameter))] + [ExpectedWarning ("IL2026", "test")] + [ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))] + static void DynamicallyAccessedMembersAll2 () + { + typeof (AnnotatedMethodParameters).RequiresAll (); + } + [UnconditionalSuppressMessage ("test", "IL2026")] public static void Test () { @@ -189,6 +241,8 @@ public static void Test () DynamicallyAccessedMembersSuppressedByRUC (); Ldftn (); Ldvirtftn (); + DynamicallyAccessedMembersAll1 (); + DynamicallyAccessedMembersAll2 (); } } @@ -395,6 +449,24 @@ static void DynamicallyAccessedMembersSuppressedByRUC () typeof (AnnotatedProperty).RequiresPublicProperties (); } + [ExpectedWarning ("IL2111", nameof (PropertyWithAnnotation) + ".set")] + [ExpectedWarning ("IL2026", "test")] + [ExpectedWarning ("IL2111", nameof (VirtualPropertyWithAnnotationGetterOnly) + ".get")] + [UnconditionalSuppressMessage ("Test", "IL2110", Justification = "Suppress warning about backing field of PropertyWithAnnotation")] + static void DynamicallyAccessedMembersAll1 () + { + typeof (AnnotatedProperty).RequiresAll (); + } + + [ExpectedWarning ("IL2111", nameof (PropertyWithAnnotation) + ".set")] + [ExpectedWarning ("IL2026", "test")] + [ExpectedWarning ("IL2111", nameof (VirtualPropertyWithAnnotationGetterOnly) + ".get")] + [UnconditionalSuppressMessage ("Test", "IL2110", Justification = "Suppress warning about backing field of PropertyWithAnnotation")] + static void DynamicallyAccessedMembersAll2 () + { + typeof (AnnotatedProperty).RequiresAll (); + } + [UnconditionalSuppressMessage ("test", "IL2026")] public static void Test () { @@ -410,6 +482,8 @@ public static void Test () DynamicDependencySuppressedByRUC (); DynamicallyAccessedMembers (); DynamicallyAccessedMembersSuppressedByRUC (); + DynamicallyAccessedMembersAll1 (); + DynamicallyAccessedMembersAll2 (); } } @@ -446,12 +520,19 @@ static void InstantiateGeneric (Type type = null) typeof (AnnotatedGenerics).GetMethod (nameof (GenericWithAnnotation)).MakeGenericMethod (type); } + // Like above, no warning expected + static void DynamicallyAccessedMembersAll () + { + typeof (AnnotatedGenerics).RequiresAll (); + } + public static void Test () { ReflectionOnly (); DynamicDependency (); DynamicallyAccessedMembers (); InstantiateGeneric (); + DynamicallyAccessedMembersAll (); } } @@ -499,6 +580,20 @@ public static void GenericMethodDynamicallyAccessedMembers () typeof (AnnotationOnGenerics).RequiresPublicMethods (); } + [ExpectedWarning ("IL2111", nameof (GenericMethodWithAnnotation))] + [ExpectedWarning ("IL2111", "GenericWithAnnotatedMethod", "AnnotatedMethod")] + static void DynamicallyAccessedMembersAll1 () + { + typeof (AnnotationOnGenerics).RequiresAll (); + } + + [ExpectedWarning ("IL2111", nameof (GenericMethodWithAnnotation))] + [ExpectedWarning ("IL2111", "GenericWithAnnotatedMethod", "AnnotatedMethod")] + static void DynamicallyAccessedMembersAll2 () + { + typeof (AnnotationOnGenerics).RequiresAll (); + } + public static void Test () { GenericTypeWithStaticMethodViaLdftn (); @@ -506,6 +601,8 @@ public static void Test () GenericMethodWithAnnotationDirectCall (); GenericMethodWithAnnotationViaLdftn (); GenericMethodDynamicallyAccessedMembers (); + DynamicallyAccessedMembersAll1 (); + DynamicallyAccessedMembersAll2 (); } } diff --git a/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs b/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs index 3cbdf4b0e4e3..1393d2361907 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs @@ -30,6 +30,8 @@ public static void Main () RequirePublicNestedTypes (annotatedPublicNestedTypes.GetType ()); RequireInterfaces (annotatedInterfaces.GetType ()); RequireAll (annotatedAll.GetType ()); + var t1 = typeof (DerivedFromAnnotatedAll1); + var t2 = typeof (DerivedFromAnnotatedAll2); RequirePublicMethods (annotatedRUCPublicMethods.GetType ()); // Instantiate this type just so its property getters are considered reachable @@ -91,6 +93,28 @@ Type t { } } + [Kept] + [KeptMember (".ctor()")] + [KeptBaseType (typeof (AnnotatedAll))] + // https://github.com/mono/linker/issues/2175 + [ExpectedWarning ("IL2113", "--RUC on AnnotatedAll.RUCMethod--")] + [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMField))] + [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMMethod))] + class DerivedFromAnnotatedAll1 : AnnotatedAll + { + } + + [Kept] + [KeptMember (".ctor()")] + [KeptBaseType (typeof (AnnotatedAll))] + // https://github.com/mono/linker/issues/2175 + [ExpectedWarning ("IL2113", "--RUC on AnnotatedAll.RUCMethod--")] + [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMField))] + [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMMethod))] + class DerivedFromAnnotatedAll2 : AnnotatedAll + { + } + [Kept] [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] From f97b935879ebf34f36a201fe6917bf7fcd00cd07 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 5 Aug 2021 20:16:09 +0000 Subject: [PATCH 3/5] PR feedback - Remove unnecessary helper --- src/linker/Linker.Steps/MarkStep.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 93297e78c73d..dc5faa71e0ba 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -311,26 +311,21 @@ protected bool IsFullyPreserved (TypeDefinition type) internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason) { - // Prevent cases where there's nothing on the stack (can happen when marking entire assemblies) - // In which case we would generate warnings with no source (hard to debug) - using var _ = _scopeStack.CurrentScope.Origin.MemberDefinition == null ? _scopeStack.PushScope (new MessageOrigin (type)) : null; - - MarkEntireTypeInternal (type, reason); - } - - void MarkEntireTypeInternal (TypeDefinition type, in DependencyInfo reason) - { #if DEBUG if (!_entireTypeReasons.Contains (reason.Kind)) throw new InternalErrorException ($"Unsupported type dependency '{reason.Kind}'."); #endif + // Prevent cases where there's nothing on the stack (can happen when marking entire assemblies) + // In which case we would generate warnings with no source (hard to debug) + using var _ = _scopeStack.CurrentScope.Origin.MemberDefinition == null ? _scopeStack.PushScope (new MessageOrigin (type)) : null; + if (!_entireTypesMarked.Add (type)) return; if (type.HasNestedTypes) { foreach (TypeDefinition nested in type.NestedTypes) - MarkEntireTypeInternal (nested, new DependencyInfo (DependencyKind.NestedType, type)); + MarkEntireType (nested, new DependencyInfo (DependencyKind.NestedType, type)); } Annotations.Mark (type, reason); From b1abc94646ee0c1c6b10d2696b01d0a067329057 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 9 Aug 2021 23:12:28 +0000 Subject: [PATCH 4/5] Avoid redundant DAM warnings for base members --- .../DynamicallyAccessedMembersBinder.cs | 76 +++++--- .../ReflectionMethodBodyScanner.cs | 16 +- .../TypeHierarchyReflectionWarnings.cs | 174 ++++++++++-------- .../Reflection/TypeHierarchySuppressions.cs | 11 -- 4 files changed, 157 insertions(+), 120 deletions(-) diff --git a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs index 4339af8a7924..820bb2be1ede 100644 --- a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs @@ -22,10 +22,12 @@ internal static class DynamicallyAccessedMembersBinder // Returns the members of the type bound by memberTypes. For DynamicallyAccessedMemberTypes.All, this returns all members of the type and its // nested types, including interface implementations, plus the same or any base types or implemented interfaces. // DynamicallyAccessedMemberTypes.PublicNestedTypes and NonPublicNestedTypes do the same for members of the selected nested types. - public static IEnumerable GetDynamicallyAccessedMembers (this TypeDefinition typeDefinition, LinkContext context, DynamicallyAccessedMemberTypes memberTypes) + public static IEnumerable GetDynamicallyAccessedMembers (this TypeDefinition typeDefinition, LinkContext context, DynamicallyAccessedMemberTypes memberTypes, bool declaredOnly = false) { + var declaredOnlyFlags = declaredOnly ? BindingFlags.DeclaredOnly : BindingFlags.Default; + if (memberTypes == DynamicallyAccessedMemberTypes.All) { - foreach (var m in typeDefinition.GetAllOnType (context)) + foreach (var m in typeDefinition.GetAllOnType (context, declaredOnly)) yield return m; yield break; } @@ -46,29 +48,29 @@ public static IEnumerable GetDynamicallyAccessedMembers } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicMethods)) { - foreach (var m in typeDefinition.GetMethodsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic)) + foreach (var m in typeDefinition.GetMethodsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic | declaredOnlyFlags)) yield return m; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicMethods)) { - foreach (var m in typeDefinition.GetMethodsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public)) + foreach (var m in typeDefinition.GetMethodsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public | declaredOnlyFlags)) yield return m; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicFields)) { - foreach (var f in typeDefinition.GetFieldsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic)) + foreach (var f in typeDefinition.GetFieldsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic | declaredOnlyFlags)) yield return f; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicFields)) { - foreach (var f in typeDefinition.GetFieldsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public)) + foreach (var f in typeDefinition.GetFieldsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public | declaredOnlyFlags)) yield return f; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)) { foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.NonPublic)) { yield return nested; - foreach (var m in nested.GetAllOnType (context)) + foreach (var m in nested.GetAllOnType (context, declaredOnly: false)) yield return m; } } @@ -76,33 +78,33 @@ public static IEnumerable GetDynamicallyAccessedMembers if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicNestedTypes)) { foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.Public)) { yield return nested; - foreach (var m in nested.GetAllOnType (context)) + foreach (var m in nested.GetAllOnType (context, declaredOnly: false)) yield return m; } } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicProperties)) { - foreach (var p in typeDefinition.GetPropertiesOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic)) + foreach (var p in typeDefinition.GetPropertiesOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic | declaredOnlyFlags)) yield return p; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicProperties)) { - foreach (var p in typeDefinition.GetPropertiesOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public)) + foreach (var p in typeDefinition.GetPropertiesOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public | declaredOnlyFlags)) yield return p; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicEvents)) { - foreach (var e in typeDefinition.GetEventsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic)) + foreach (var e in typeDefinition.GetEventsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.NonPublic | declaredOnlyFlags)) yield return e; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicEvents)) { - foreach (var e in typeDefinition.GetEventsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public)) + foreach (var e in typeDefinition.GetEventsOnTypeHierarchy (context, filter: null, bindingFlags: BindingFlags.Public | declaredOnlyFlags)) yield return e; } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypesOverlay.Interfaces)) { - foreach (var i in typeDefinition.GetAllInterfaceImplementations (context)) + foreach (var i in typeDefinition.GetAllInterfaceImplementations (context, declaredOnly)) yield return i; } } @@ -167,6 +169,9 @@ public static IEnumerable GetMethodsOnTypeHierarchy (this Type yield return method; } + if ((bindingFlags & BindingFlags.DeclaredOnly) == BindingFlags.DeclaredOnly) + yield break; + type = context.TryResolve (type.BaseType); onBaseType = true; } @@ -203,6 +208,9 @@ public static IEnumerable GetFieldsOnTypeHierarchy (this TypeDe yield return field; } + if ((bindingFlags & BindingFlags.DeclaredOnly) == BindingFlags.DeclaredOnly) + yield break; + type = context.TryResolve (type.BaseType); onBaseType = true; } @@ -268,6 +276,9 @@ public static IEnumerable GetPropertiesOnTypeHierarchy (this yield return property; } + if ((bindingFlags & BindingFlags.DeclaredOnly) == BindingFlags.DeclaredOnly) + yield break; + type = context.TryResolve (type.BaseType); onBaseType = true; } @@ -313,31 +324,39 @@ public static IEnumerable GetEventsOnTypeHierarchy (this TypeDe yield return @event; } + if ((bindingFlags & BindingFlags.DeclaredOnly) == BindingFlags.DeclaredOnly) + yield break; + type = context.TryResolve (type.BaseType); onBaseType = true; } } - public static IEnumerable GetAllInterfaceImplementations (this TypeDefinition type, LinkContext context) + public static IEnumerable GetAllInterfaceImplementations (this TypeDefinition type, LinkContext context, bool declaredOnly) { while (type != null) { foreach (var i in type.Interfaces) { yield return i; - TypeDefinition interfaceType = context.TryResolve (i.InterfaceType); - if (interfaceType != null) { - foreach (var innerInterface in interfaceType.GetAllInterfaceImplementations (context)) - yield return innerInterface; + if (!declaredOnly) { + TypeDefinition interfaceType = context.TryResolve (i.InterfaceType); + if (interfaceType != null) { + foreach (var innerInterface in interfaceType.GetAllInterfaceImplementations (context, declaredOnly: false)) + yield return innerInterface; + } } } + if (declaredOnly) + yield break; + type = context.TryResolve (type.BaseType); } } - public static IEnumerable GetAllOnType (this TypeDefinition type, LinkContext context) => GetAllOnType (type, context, new HashSet ()); + public static IEnumerable GetAllOnType (this TypeDefinition type, LinkContext context, bool declaredOnly) => GetAllOnType (type, context, declaredOnly, new HashSet ()); - static IEnumerable GetAllOnType (TypeDefinition type, LinkContext context, HashSet types) + static IEnumerable GetAllOnType (TypeDefinition type, LinkContext context, bool declaredOnly, HashSet types) { if (!types.Add (type)) yield break; @@ -345,22 +364,25 @@ static IEnumerable GetAllOnType (TypeDefinition type, Li if (type.HasNestedTypes) { foreach (var nested in type.NestedTypes) { yield return nested; - foreach (var m in GetAllOnType (nested, context, types)) + // Base types and interfaces of nested types are always included. + foreach (var m in GetAllOnType (nested, context, declaredOnly: false, types)) yield return m; } } - var baseType = context.TryResolve (type.BaseType); - if (baseType != null) { - foreach (var m in GetAllOnType (baseType, context, types)) - yield return m; + if (!declaredOnly) { + var baseType = context.TryResolve (type.BaseType); + if (baseType != null) { + foreach (var m in GetAllOnType (baseType, context, declaredOnly: false, types)) + yield return m; + } } - if (type.HasInterfaces) { + if (!declaredOnly && type.HasInterfaces) { foreach (var iface in type.Interfaces) { yield return iface; var interfaceType = context.Resolve (iface.InterfaceType); - foreach (var m in GetAllOnType (interfaceType, context, types)) + foreach (var m in GetAllOnType (interfaceType, context, declaredOnly: false, types)) yield return m; } } diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 76519a3d9c30..f367a3f4cd87 100644 --- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -127,7 +127,17 @@ public void ApplyDynamicallyAccessedMembersToType (ref ReflectionPatternContext // Handle cases where a type has no members but annotations are to be applied to derived type members reflectionPatternContext.RecordHandledPattern (); - MarkTypeForDynamicallyAccessedMembers (ref reflectionPatternContext, type, annotation, DependencyKind.DynamicallyAccessedMemberOnType); + // The annotations this type inherited from its base types should not produce + // warnings on the base members, since those are covered by warnings in the base type. + // For now, annotations that are inherited from base types but also declared explicitly on this type + // still warn about base members since the cache doesn't track enough information to tell the difference. + var declaredAnnotation = _context.Annotations.FlowAnnotations.GetTypeAnnotation (type); + var inheritedOnlyAnnotation = annotation & ~declaredAnnotation; + + // Warn about all members accessed by the annotation on this type (including members of base types/interfaces) + MarkTypeForDynamicallyAccessedMembers (ref reflectionPatternContext, type, declaredAnnotation, DependencyKind.DynamicallyAccessedMemberOnType, declaredOnly: false); + // Warn about members accessed by inherited annotations (not including members of base types/interfaces) + MarkTypeForDynamicallyAccessedMembers (ref reflectionPatternContext, type, inheritedOnlyAnnotation, DependencyKind.DynamicallyAccessedMemberOnType, declaredOnly: true); } ValueNode GetValueNodeForCustomAttributeArgument (CustomAttributeArgument argument) @@ -2285,9 +2295,9 @@ void RequireDynamicallyAccessedMembers (ref ReflectionPatternContext reflectionC static bool HasBindingFlag (BindingFlags? bindingFlags, BindingFlags? search) => bindingFlags != null && (bindingFlags & search) == search; - void MarkTypeForDynamicallyAccessedMembers (ref ReflectionPatternContext reflectionContext, TypeDefinition typeDefinition, DynamicallyAccessedMemberTypes requiredMemberTypes, DependencyKind dependencyKind) + void MarkTypeForDynamicallyAccessedMembers (ref ReflectionPatternContext reflectionContext, TypeDefinition typeDefinition, DynamicallyAccessedMemberTypes requiredMemberTypes, DependencyKind dependencyKind, bool declaredOnly = false) { - foreach (var member in typeDefinition.GetDynamicallyAccessedMembers (_context, requiredMemberTypes)) { + foreach (var member in typeDefinition.GetDynamicallyAccessedMembers (_context, requiredMemberTypes, declaredOnly)) { switch (member) { case MethodDefinition method: MarkMethod (ref reflectionContext, method, dependencyKind); diff --git a/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs b/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs index 1393d2361907..5bd06a7e7edd 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs @@ -17,22 +17,27 @@ public class TypeHierarchyReflectionWarnings { public static void Main () { - RequirePublicMethods (annotatedBase.GetType ()); + annotatedBase.GetType ().RequiresPublicMethods (); + var baseType = annotatedBaseSharedByNestedTypes.GetType (); + baseType.RequiresPublicNestedTypes (); + baseType.RequiresPublicMethods (); + var derivedType = typeof (DerivedWithNestedTypes); // Reference to the derived type should apply base annotations - var t = typeof (DerivedFromAnnotatedBase); - RequirePublicMethods (annotatedDerivedFromBase.GetType ()); - RequirePublicNestedTypes (annotatedPublicNestedTypes.GetType ()); - RequirePublicFields (derivedFromAnnotatedDerivedFromBase.GetType ()); - RequirePublicMethods (annotatedPublicMethods.GetType ()); - RequirePublicFields (annotatedPublicFields.GetType ()); - RequirePublicProperties (annotatedPublicProperties.GetType ()); - RequirePublicEvents (annotatedPublicEvents.GetType ()); - RequirePublicNestedTypes (annotatedPublicNestedTypes.GetType ()); - RequireInterfaces (annotatedInterfaces.GetType ()); - RequireAll (annotatedAll.GetType ()); - var t1 = typeof (DerivedFromAnnotatedAll1); - var t2 = typeof (DerivedFromAnnotatedAll2); - RequirePublicMethods (annotatedRUCPublicMethods.GetType ()); + var t1 = typeof (DerivedFromAnnotatedBase); + var t2 = typeof (AnnotatedDerivedFromAnnotatedBase); + annotatedDerivedFromBase.GetType ().RequiresPublicMethods (); + annotatedPublicNestedTypes.GetType ().RequiresPublicNestedTypes (); + derivedFromAnnotatedDerivedFromBase.GetType ().RequiresPublicFields (); + annotatedPublicMethods.GetType ().RequiresPublicMethods (); + annotatedPublicFields.GetType ().RequiresPublicFields (); + annotatedPublicProperties.GetType ().RequiresPublicProperties (); + annotatedPublicEvents.GetType ().RequiresPublicEvents (); + annotatedPublicNestedTypes.GetType ().RequiresPublicNestedTypes (); + annotatedInterfaces.GetType ().RequiresInterfaces (); + annotatedAll.GetType ().RequiresAll (); + var t3 = typeof (DerivedFromAnnotatedAll1); + var t4 = typeof (DerivedFromAnnotatedAll2); + annotatedRUCPublicMethods.GetType ().RequiresPublicMethods (); // Instantiate this type just so its property getters are considered reachable var b = new DerivedFromAnnotatedDerivedFromBase (); @@ -57,6 +62,8 @@ public static void Main () [Kept] static AnnotatedBase annotatedBase; [Kept] + static AnnotatedBaseSharedByNestedTypes annotatedBaseSharedByNestedTypes; + [Kept] static AnnotatedDerivedFromBase annotatedDerivedFromBase; [Kept] static AnnotatedPublicNestedTypes annotatedPublicNestedTypes; @@ -96,10 +103,6 @@ Type t [Kept] [KeptMember (".ctor()")] [KeptBaseType (typeof (AnnotatedAll))] - // https://github.com/mono/linker/issues/2175 - [ExpectedWarning ("IL2113", "--RUC on AnnotatedAll.RUCMethod--")] - [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMField))] - [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMMethod))] class DerivedFromAnnotatedAll1 : AnnotatedAll { } @@ -107,10 +110,6 @@ class DerivedFromAnnotatedAll1 : AnnotatedAll [Kept] [KeptMember (".ctor()")] [KeptBaseType (typeof (AnnotatedAll))] - // https://github.com/mono/linker/issues/2175 - [ExpectedWarning ("IL2113", "--RUC on AnnotatedAll.RUCMethod--")] - [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMField))] - [ExpectedWarning ("IL2115", nameof (AnnotatedAll.DAMMethod))] class DerivedFromAnnotatedAll2 : AnnotatedAll { } @@ -260,11 +259,14 @@ class AnnotatedBase [ExpectedWarning ("IL2112", "--RUC on AnnotatedBase--")] [RequiresUnreferencedCode ("--RUC on AnnotatedBase--")] public void RUCMethod () { } + + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicMethods)] + public string DAMField1; } [KeptBaseType (typeof (AnnotatedBase))] - // Warning about base member could go away with https://github.com/mono/linker/issues/2175 - [ExpectedWarning ("IL2113", "--RUC on AnnotatedBase--")] class DerivedFromAnnotatedBase : AnnotatedBase { [Kept] @@ -274,6 +276,29 @@ class DerivedFromAnnotatedBase : AnnotatedBase public void RUCMethod () { } } + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [KeptBaseType (typeof (AnnotatedBase))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.PublicFields)] + // In theory we could avoid warning for public methods on base types because that + // annotation is already inherited from the base type, but currently we warn on base + // members whenever a type has an explicit annotation. + [ExpectedWarning ("IL2113", nameof (AnnotatedBase), nameof (AnnotatedBase.RUCMethod))] + [ExpectedWarning ("IL2115", nameof (AnnotatedBase), nameof (AnnotatedBase.DAMField1))] + class AnnotatedDerivedFromAnnotatedBase : AnnotatedBase + { + [Kept] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [ExpectedWarning ("IL2112", "--RUC on AnnotatedDerivedFromAnnotatedBase--")] + [RequiresUnreferencedCode ("--RUC on AnnotatedDerivedFromAnnotatedBase--")] + public void DerivedRUCMethod () { } + + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [ExpectedWarning ("IL2114", nameof (AnnotatedDerivedFromAnnotatedBase), nameof (DAMField2))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicMethods)] + public string DAMField2; + } + [KeptMember (".ctor()")] class Base { @@ -338,13 +363,11 @@ public override void RUCVirtualMethod () { } [KeptMember (".ctor()")] [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] - // Warnings about base members could go away with https://github.com/mono/linker/issues/2175 + // The annotation from this type should warn about all public fields including those + // from base types, but the inherited PublicMethods annotation should not warn + // again about base methods. [ExpectedWarning ("IL2115", nameof (Base), nameof (DAMField1))] [ExpectedWarning ("IL2115", nameof (AnnotatedDerivedFromBase), nameof (DAMField2))] - [ExpectedWarning ("IL2113", "--RUCBaseMethod--")] - [ExpectedWarning ("IL2113", "--Base.RUCVirtualMethod--")] - [ExpectedWarning ("IL2113", "--RUC on AnnotatedDerivedFromBase--")] - [ExpectedWarning ("IL2115", nameof (Base), nameof (Base.DAMVirtualProperty) + ".get")] class DerivedFromAnnotatedDerivedFromBase : AnnotatedDerivedFromBase { [Kept] @@ -373,8 +396,19 @@ public override void RUCVirtualMethod () { } public override string DAMVirtualProperty { [Kept] get; } } + [KeptMember (".ctor()")] + public class BaseTypeOfNestedType + { + [Kept] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode ("--RUC on BaseTypeOfNestedType.RUCMethod--")] + public void RUCMethod () { } + } + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicNestedTypes)] + // Warnings about base types of nested types are shown at the (outer) type level. + [ExpectedWarning ("IL2113", nameof (BaseTypeOfNestedType), nameof (BaseTypeOfNestedType.RUCMethod))] class AnnotatedPublicNestedTypes { [KeptMember (".ctor()")] @@ -387,6 +421,12 @@ public class NestedType void RUCMethod () { } } + [KeptMember (".ctor()")] + [KeptBaseType (typeof (BaseTypeOfNestedType))] + public class NestedTypeWithBase : BaseTypeOfNestedType + { + } + [KeptMember (".ctor()")] [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] @@ -431,6 +471,31 @@ public class NestedRUCTypeWithDefaultConstructor } } + [KeptMember (".ctor()")] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.PublicNestedTypes)] + class AnnotatedBaseSharedByNestedTypes + { + [Kept] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [ExpectedWarning ("IL2112", "--RUC on AnnotatedBaseSharedByNestedTypes.RUCMethod--")] + [RequiresUnreferencedCode ("--RUC on AnnotatedBaseSharedByNestedTypes.RUCMethod--")] + public void RUCMethod () { } + } + + [KeptBaseType (typeof (AnnotatedBaseSharedByNestedTypes))] + // Nested types that share the outer class base type can produce warnings about base methods of the annotated type. + [ExpectedWarning ("IL2113", "--RUC on AnnotatedBaseSharedByNestedTypes.RUCMethod--")] + class DerivedWithNestedTypes : AnnotatedBaseSharedByNestedTypes + { + + [KeptMember (".ctor()")] + [KeptBaseType (typeof (AnnotatedBaseSharedByNestedTypes))] + public class NestedType : AnnotatedBaseSharedByNestedTypes + { + } + } + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] @@ -452,54 +517,5 @@ public void Method () { } [ExpectedWarning ("IL2112", "--AnnotatedRUCPublicMethods--")] public static void StaticMethod () { } } - - [Kept] - static void RequirePublicMethods ( - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] - Type type) - { } - - [Kept] - static void RequirePublicFields ( - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] - Type type) - { } - - [Kept] - static void RequirePublicProperties ( - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)] - Type type) - { } - - [Kept] - static void RequirePublicEvents ( - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicEvents)] - Type type) - { } - - [Kept] - static void RequireAll ( - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] - Type type) - { } - - [Kept] - static void RequirePublicNestedTypes ( - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicNestedTypes)] - Type type) - { } - - [Kept] - static void RequireInterfaces ( - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] - Type type) - { } } } diff --git a/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchySuppressions.cs b/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchySuppressions.cs index fe9a44c3b092..7318f1faf7c2 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchySuppressions.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchySuppressions.cs @@ -79,7 +79,6 @@ public void RUCMethod () { } [Kept] [KeptBaseType (typeof (Unsuppressed))] - [ExpectedWarning ("IL2113", "--RUC on Unsuppressed--")] class DerivedFromUnsuppressed1 : Unsuppressed { [Kept] @@ -91,7 +90,6 @@ public void DerivedRUCMethod () { } [Kept] [KeptBaseType (typeof (Unsuppressed))] - [ExpectedWarning ("IL2113", "--RUC on Unsuppressed--")] class DerivedFromUnsuppressed2 : Unsuppressed { [Kept] @@ -116,8 +114,6 @@ public void RUCMethod () { } [Kept] [KeptBaseType (typeof (Suppressed))] - // Base method should warn even though it was suppressed on the base - [ExpectedWarning ("IL2113", "--RUC on Suppressed--")] class DerivedFromSuppressed1 : Suppressed { [Kept] @@ -129,8 +125,6 @@ public void RUCDerivedMethod () { } [Kept] [KeptBaseType (typeof (Suppressed))] - // Base method should warn even though it was suppressed on the base - [ExpectedWarning ("IL2113", "--RUC on Suppressed--")] class DerivedFromSuppressed2 : Suppressed { [Kept] @@ -142,7 +136,6 @@ public void RUCDerivedMethod () { } [Kept] [KeptBaseType (typeof (Unsuppressed))] - [ExpectedWarning ("IL2113", "--RUC on Unsuppressed--")] class SuppressedOnDerived1 : Unsuppressed { [Kept] @@ -155,7 +148,6 @@ public void DerivedRUCMethod () { } [Kept] [KeptBaseType (typeof (Unsuppressed))] - [ExpectedWarning ("IL2113", "--RUC on Unsuppressed--")] class SuppressedOnDerived2 : Unsuppressed { [Kept] @@ -168,9 +160,6 @@ public void DerivedRUCMethod () { } [Kept] [KeptBaseType (typeof (Unsuppressed))] - [KeptAttributeAttribute (typeof (UnconditionalSuppressMessageAttribute))] - // Suppress warnings about base members - [UnconditionalSuppressMessage ("TrimAnalysis", "IL2113")] class SuppressedBaseWarningsOnDerived : Unsuppressed { [Kept] From 9a1aa771914c9dc6da6565c1bd632cf856a81708 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 10 Aug 2021 00:31:21 +0000 Subject: [PATCH 5/5] PR feedback - Resolve -> TryResolve - Avoid nested yield returns - Testcases with base instantiated over self for generic parameter with requirements --- .../DynamicallyAccessedMembersBinder.cs | 44 ++++++++++--------- .../DataFlow/GenericParameterDataFlow.cs | 20 +++++++++ 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs index 820bb2be1ede..85b597fe13bb 100644 --- a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs @@ -27,7 +27,9 @@ public static IEnumerable GetDynamicallyAccessedMembers var declaredOnlyFlags = declaredOnly ? BindingFlags.DeclaredOnly : BindingFlags.Default; if (memberTypes == DynamicallyAccessedMemberTypes.All) { - foreach (var m in typeDefinition.GetAllOnType (context, declaredOnly)) + var members = new List (); + typeDefinition.GetAllOnType (context, declaredOnly, members); + foreach (var m in members) yield return m; yield break; } @@ -70,7 +72,9 @@ public static IEnumerable GetDynamicallyAccessedMembers if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)) { foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.NonPublic)) { yield return nested; - foreach (var m in nested.GetAllOnType (context, declaredOnly: false)) + var members = new List (); + nested.GetAllOnType (context, declaredOnly: false, members); + foreach (var m in members) yield return m; } } @@ -78,7 +82,9 @@ public static IEnumerable GetDynamicallyAccessedMembers if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicNestedTypes)) { foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.Public)) { yield return nested; - foreach (var m in nested.GetAllOnType (context, declaredOnly: false)) + var members = new List (); + nested.GetAllOnType (context, declaredOnly: false, members); + foreach (var m in members) yield return m; } } @@ -354,57 +360,53 @@ public static IEnumerable GetAllInterfaceImplementation } } - public static IEnumerable GetAllOnType (this TypeDefinition type, LinkContext context, bool declaredOnly) => GetAllOnType (type, context, declaredOnly, new HashSet ()); + public static void GetAllOnType (this TypeDefinition type, LinkContext context, bool declaredOnly, List members) => GetAllOnType (type, context, declaredOnly, members, new HashSet ()); - static IEnumerable GetAllOnType (TypeDefinition type, LinkContext context, bool declaredOnly, HashSet types) + static void GetAllOnType (TypeDefinition type, LinkContext context, bool declaredOnly, List members, HashSet types) { if (!types.Add (type)) - yield break; + return; if (type.HasNestedTypes) { foreach (var nested in type.NestedTypes) { - yield return nested; + members.Add (nested); // Base types and interfaces of nested types are always included. - foreach (var m in GetAllOnType (nested, context, declaredOnly: false, types)) - yield return m; + GetAllOnType (nested, context, declaredOnly: false, members, types); } } if (!declaredOnly) { var baseType = context.TryResolve (type.BaseType); - if (baseType != null) { - foreach (var m in GetAllOnType (baseType, context, declaredOnly: false, types)) - yield return m; - } + if (baseType != null) + GetAllOnType (baseType, context, declaredOnly: false, members, types); } if (!declaredOnly && type.HasInterfaces) { foreach (var iface in type.Interfaces) { - yield return iface; - var interfaceType = context.Resolve (iface.InterfaceType); - foreach (var m in GetAllOnType (interfaceType, context, declaredOnly: false, types)) - yield return m; + members.Add (iface); + var interfaceType = context.TryResolve (iface.InterfaceType); + GetAllOnType (interfaceType, context, declaredOnly: false, members, types); } } if (type.HasFields) { foreach (var f in type.Fields) - yield return f; + members.Add (f); } if (type.HasMethods) { foreach (var m in type.Methods) - yield return m; + members.Add (m); } if (type.HasProperties) { foreach (var p in type.Properties) - yield return p; + members.Add (p); } if (type.HasEvents) { foreach (var e in type.Events) - yield return e; + members.Add (e); } } } diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs index 09de0a0155c5..1665c1abd26c 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs @@ -243,6 +243,7 @@ public static void TestNothing () static void TestBaseTypeGenericRequirements () { new DerivedTypeWithInstantiatedGenericOnBase (); + new DerivedTypeWithInstantiationOverSelfOnBase (); new DerivedTypeWithOpenGenericOnBase (); new DerivedTypeWithOpenGenericOnBaseWithRequirements (); } @@ -261,6 +262,15 @@ class DerivedTypeWithInstantiatedGenericOnBase : GenericBaseTypeWithRequirements { } + class GenericBaseTypeWithRequiresAll<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] T> + { + } + + [RecognizedReflectionAccessPattern] + class DerivedTypeWithInstantiationOverSelfOnBase : GenericBaseTypeWithRequirements + { + } + [UnrecognizedReflectionAccessPattern (typeof (GenericBaseTypeWithRequirements<>), "T", messageCode: "IL2091")] class DerivedTypeWithOpenGenericOnBase : GenericBaseTypeWithRequirements { @@ -277,6 +287,7 @@ class DerivedTypeWithOpenGenericOnBaseWithRequirements<[DynamicallyAccessedMembe static void TestInterfaceTypeGenericRequirements () { IGenericInterfaceTypeWithRequirements instance = new InterfaceImplementationTypeWithInstantiatedGenericOnBase (); + new InterfaceImplementationTypeWithInstantiationOverSelfOnBase (); new InterfaceImplementationTypeWithOpenGenericOnBase (); new InterfaceImplementationTypeWithOpenGenericOnBaseWithRequirements (); } @@ -290,6 +301,15 @@ class InterfaceImplementationTypeWithInstantiatedGenericOnBase : IGenericInterfa { } + interface IGenericInterfaceTypeWithRequiresAll<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] T> + { + } + + [RecognizedReflectionAccessPattern] + class InterfaceImplementationTypeWithInstantiationOverSelfOnBase : IGenericInterfaceTypeWithRequiresAll + { + } + [UnrecognizedReflectionAccessPattern (typeof (IGenericInterfaceTypeWithRequirements<>), "T", messageCode: "IL2091")] class InterfaceImplementationTypeWithOpenGenericOnBase : IGenericInterfaceTypeWithRequirements {