From f70429149d0f6b8249684116f267727a7d3b05c4 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Mon, 9 May 2022 14:36:40 -0500 Subject: [PATCH 01/29] Re-add static interface trimming with more testing --- docs/removal-behavior.md | 2 +- src/linker/Linker.Steps/MarkStep.cs | 12 + src/linker/Linker.Steps/SweepStep.cs | 28 + .../Assertions/KeptOverrideAttribute.cs | 27 + .../Assertions/RemovedOverrideAttribute.cs | 25 + .../InterfaceVariants.cs | 15 +- .../StaticAbstractInterfaceMethods.cs | 650 ++++++++++++++++++ .../TestCasesRunner/AssemblyChecker.cs | 41 +- 8 files changed, 782 insertions(+), 18 deletions(-) create mode 100644 test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs create mode 100644 test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs create mode 100644 test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs diff --git a/docs/removal-behavior.md b/docs/removal-behavior.md index 994b1d32ef66..1133c842ad2e 100644 --- a/docs/removal-behavior.md +++ b/docs/removal-behavior.md @@ -42,7 +42,7 @@ public class Program ### Method call on a constrained type parameter -On a call to a static abstract interface method that is accessed through a constrained type parameter, the interface method is rooted, as well as every implementation method on every type. +On a call to a static abstract interface method that is accessed through a constrained type parameter, the interface method is rooted, as well as every implementation method on every type that is rooted. Example: diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 3912ebc5752b..f925c4b92b49 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -574,6 +574,10 @@ void ProcessVirtualMethods () } } + /// + /// Does extra handling of marked types that have interfaces when it's necessary to know what types are marked or instantiated. + /// e.g. Marks the "implements interface" annotations and removes override annotations for static interface methods. + /// void ProcessMarkedTypesWithInterfaces () { // We may mark an interface type later on. Which means we need to reprocess any time with one or more interface implementations that have not been marked @@ -3043,8 +3047,16 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo } } + // Mark overridden methods except for static interface methods if (method.HasOverrides) { foreach (MethodReference ov in method.Overrides) { + // Method implementing a static interface method will have an override to it - note nonstatic methods usually don't unless they're explicit. + // Calling the implementation method directly has no impact on the interface, and as such it should not mark the interface or its method. + // Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round. + if (Context.Resolve (ov)?.IsStatic == true + && Context.Resolve (ov.DeclaringType)?.IsInterface == true) { + continue; + } MarkMethod (ov, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin); MarkExplicitInterfaceImplementation (method, ov); } diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index 24a2ccbce0c1..6f53ee2ec054 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -453,6 +453,8 @@ protected virtual void SweepMethods (Collection methods) SweepCustomAttributes (method.MethodReturnType); + SweepOverrides (method); + if (!method.HasParameters) continue; @@ -467,6 +469,32 @@ protected virtual void SweepMethods (Collection methods) } } + void SweepOverrides (MethodDefinition method) + { + for (int i = 0; i < method.Overrides.Count;) { + // We can't rely on the context resolution cache anymore, since it may remember methods which are already removed + // So call the direct Resolve here and avoid the cache. + // It can still happen that the Resolve will return a removed method (this happens if the Overrides collection stores actual + // MethodDefinition and not MethodReference, in which case the Resolve is just "return this" and doesn't check the existence of the method + // in any way). In such case the Declaring type of the method is null. And since it has been removed the override pointing + // to it should be removed as well. + // Resolve() may also return null if the method has been removed, in which case the override should be removed from the list + if (method.Overrides[i].Resolve () is not MethodDefinition ov || ShouldRemove (ov) && (ov.DeclaringType == null || !IgnoreScope (ov.DeclaringType.Scope))) + method.Overrides.RemoveAt (i); + else + i++; + } + } + + /// + /// Returns true if the assembly of the is not set to link (i.e. action=copy is set for that assembly) + /// + private bool IgnoreScope (IMetadataScope scope) + { + AssemblyDefinition? assembly = Context.Resolve (scope); + return assembly != null && Annotations.GetAction (assembly) != AssemblyAction.Link; + } + void SweepDebugInfo (Collection methods) { List? sweptScopes = null; diff --git a/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs new file mode 100644 index 000000000000..62bfd0c15794 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs @@ -0,0 +1,27 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace Mono.Linker.Tests.Cases.Expectations.Assertions +{ + /// + /// Used to ensure that a method should keep an 'override' annotation for a method in the supplied base type. + /// The absence of this attribute does not enforce that the override is removed -- this is different from other Kept attributes + /// To enforce the removal of an override, use . + /// Fails in tests if the method doesn't have the override method in the original or linked assembly. + /// + /// + [AttributeUsage (AttributeTargets.Method, AllowMultiple = true, Inherited = false)] + public class KeptOverrideAttribute : KeptAttribute + { + public Type TypeWithOverriddenMethodDeclaration; + + public KeptOverrideAttribute (Type typeWithOverriddenMethod) + { + if (typeWithOverriddenMethod == null) + throw new ArgumentNullException (nameof (typeWithOverriddenMethod)); + TypeWithOverriddenMethodDeclaration = typeWithOverriddenMethod; + } + } +} \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs new file mode 100644 index 000000000000..c15cf9a9e414 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs @@ -0,0 +1,25 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace Mono.Linker.Tests.Cases.Expectations.Assertions +{ + /// + /// Used to ensure that a method should remove an 'override' annotation for a method in the supplied base type. + /// Fails in tests if the method has the override method in the linked assembly, + /// or if the override is not found in the original assembly + /// + /// + [AttributeUsage (AttributeTargets.Method, AllowMultiple = true, Inherited = false)] + public class RemovedOverrideAttribute : BaseInAssemblyAttribute + { + public Type TypeWithOverriddenMethodDeclaration; + public RemovedOverrideAttribute (Type typeWithOverriddenMethod) + { + if (typeWithOverriddenMethod == null) + throw new ArgumentException ("Value cannot be null or empty.", nameof (typeWithOverriddenMethod)); + TypeWithOverriddenMethodDeclaration = typeWithOverriddenMethod; + } + } +} \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs index bb43569b614c..c7fb236a4497 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs @@ -71,41 +71,32 @@ internal class ImplementsUnusedStaticInterface // The interface methods themselves are not used, but the implementation of these methods is internal interface IStaticInterfaceMethodUnused { - // Can be removed with Static Interface trimming optimization - [Kept] static abstract void InterfaceUsedMethodNot (); } - // Can be removed with Static Interface Trimming - [Kept] internal interface IStaticInterfaceUnused { - // Can be removed with Static Interface Trimming - [Kept] static abstract void InterfaceAndMethodNoUsed (); } [Kept] - [KeptInterface (typeof (IStaticInterfaceUnused))] - [KeptInterface (typeof (IStaticInterfaceMethodUnused))] internal class InterfaceMethodUsedThroughImplementation : IStaticInterfaceMethodUnused, IStaticInterfaceUnused { [Kept] + [RemovedOverride (typeof (IStaticInterfaceMethodUnused))] public static void InterfaceUsedMethodNot () { } [Kept] + [RemovedOverride (typeof (IStaticInterfaceUnused))] public static void InterfaceAndMethodNoUsed () { } } [Kept] - [KeptInterface (typeof (IStaticInterfaceMethodUnused))] - [KeptInterface (typeof (IStaticInterfaceUnused))] + [KeptInterface (typeof (IStaticInterfaceMethodUnused))] // Bug internal class InterfaceMethodUnused : IStaticInterfaceMethodUnused, IStaticInterfaceUnused { - [Kept] public static void InterfaceUsedMethodNot () { } - [Kept] public static void InterfaceAndMethodNoUsed () { } } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs new file mode 100644 index 000000000000..6d7bf0d25bb3 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -0,0 +1,650 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods +{ + public class StaticAbstractInterfaceMethods + { + static void Main () + { + InterfaceMethodsUsedThroughConstrainedType.Test (); + InterfaceWithMethodsUsedEachWay.Test (); + InterfaceMethodUsedOnConcreteType.Test (); + InterfaceMethodsKeptThroughReflection.Test (); + StaticInterfaceInheritance.Test (); + GenericStaticInterface.Test (); + RecursiveGenericInterface.Test (); + } + + [Kept] + class InterfaceMethodsUsedThroughConstrainedType + { + [Kept] + internal interface IUsedThroughConstrainedType + { + [Kept] + static abstract int UsedThroughConstrainedType (); + } + + [Kept] + [KeptInterface (typeof (IUsedThroughConstrainedType))] + class UsesIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedType))] + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + [KeptInterface (typeof (IUsedThroughConstrainedType))] + class UnusedIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedType))] + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + static void CallMethodOnConstrainedType () where T : IUsedThroughConstrainedType + { + T.UsedThroughConstrainedType (); + } + + [Kept] + public static void Test () + { + CallMethodOnConstrainedType (); + + Type t = typeof (UnusedIUsedThroughConstrainedTypeMethods); + + ExplicitImplementation.Test (); + } + + [Kept] + public class ExplicitImplementation + { + [Kept] + [KeptInterface (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + class UsedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTypeExplicitImplementation + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + static int IUsedThroughConstrainedTypeExplicitImplementation.UsedThroughConstrainedType () => 0; + } + + [Kept] + [KeptInterface (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + class UnusedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTypeExplicitImplementation + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + static int IUsedThroughConstrainedTypeExplicitImplementation.UsedThroughConstrainedType () => 0; + } + + [Kept] + internal interface IUsedThroughConstrainedTypeExplicitImplementation + { + [Kept] + static abstract int UsedThroughConstrainedType (); + } + + [Kept] + static void CallTypeConstrainedMethod () where T : IUsedThroughConstrainedTypeExplicitImplementation + { + T.UsedThroughConstrainedType (); + } + + [Kept] + public static void Test () + { + CallTypeConstrainedMethod (); + + Type t = typeof (UnusedIUsedThroughConstrainedTypeExplicitMethods); + } + } + } + + [Kept] + class InterfaceMethodUsedOnConcreteType + { + [Kept] + class UsesIUsedOnConcreteTypeMethods : IUsedOnConcreteType + { + [Kept] + [RemovedOverride (typeof (IUsedOnConcreteType))] + public static int UsedOnConcreteType () => 0; + } + + [Kept] + class UnusedIUsedOnConcreteTypeMethods : IUsedOnConcreteType + { + public static int UsedOnConcreteType () => 0; + } + + internal interface IUsedOnConcreteType + { + static abstract int UsedOnConcreteType (); + } + + [Kept] + public static void Test () + { + UsesIUsedOnConcreteTypeMethods.UsedOnConcreteType (); + + Type t = typeof (UnusedIUsedOnConcreteTypeMethods); + } + } + + [Kept] + class InterfaceWithMethodsUsedEachWay + { + + [Kept] + internal interface IUsedEveryWay + { + [Kept] + static abstract int UsedThroughConstrainedType (); + + static abstract int UsedOnConcreteType (); + + [Kept] + static abstract int UsedThroughConstrainedTypeExplicit (); + } + + [Kept] + [KeptInterface (typeof (IUsedEveryWay))] + class UsedIUsedEveryWay : IUsedEveryWay + { + + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + static int IUsedEveryWay.UsedThroughConstrainedTypeExplicit () => 0; + + [Kept] + [RemovedOverride (typeof (IUsedEveryWay))] + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + [KeptInterface (typeof (IUsedEveryWay))] + class UnusedIUsedEveryWay : IUsedEveryWay + { + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + static int IUsedEveryWay.UsedThroughConstrainedTypeExplicit () => 0; + + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + static void CallTypeConstrainedMethods () where T : IUsedEveryWay + { + T.UsedThroughConstrainedType (); + T.UsedThroughConstrainedTypeExplicit (); + } + + [Kept] + public static void Test () + { + UsedIUsedEveryWay.UsedOnConcreteType (); + CallTypeConstrainedMethods (); + + Type t = typeof (UnusedIUsedEveryWay); + } + } + + [Kept] + internal class InterfaceMethodsKeptThroughReflection + { + [Kept] + internal interface IMethodsKeptThroughReflection + { + [Kept] + public static abstract int UnusedMethod (); + + [Kept] + public static abstract int UsedOnConcreteType (); + + [Kept] + public static abstract int UsedOnConstrainedType (); + } + + [Kept] + [KeptInterface (typeof (IMethodsKeptThroughReflection))] + class UsedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection + { + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UnusedMethod () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConstrainedType () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConcreteType () => 0; + } + + [Kept] + [KeptInterface (typeof (IMethodsKeptThroughReflection))] + class UnusedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection + { + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UnusedMethod () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConstrainedType () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConcreteType () => 0; + } + + [Kept] + public static void Test () + { + typeof (IMethodsKeptThroughReflection).RequiresPublicMethods (); + UsedMethodsKeptThroughtReflection.UsedOnConcreteType (); + UseMethodThroughTypeConstraint (); + + Type t = typeof (UnusedMethodsKeptThroughtReflection); + [Kept] + static void UseMethodThroughTypeConstraint () where T : IMethodsKeptThroughReflection + { + T.UsedOnConstrainedType (); + } + } + } + + class InterfaceHasStaticAndInstanceMethods + { + [Kept] + interface IStaticAndInstanceMethods + { + static abstract int StaticMethodCalledOnConcreteType (); + + [Kept] + static abstract int StaticMethodExplicitImpl (); + + [Kept] + int InstanceMethod (); + } + + [Kept] + [KeptInterface (typeof (IStaticAndInstanceMethods))] + class UsesAllMethods : IStaticAndInstanceMethods + { + [Kept] + [RemovedOverride (typeof (IStaticAndInstanceMethods))] + public static int StaticMethodCalledOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + public int InstanceMethod () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + static int IStaticAndInstanceMethods.StaticMethodExplicitImpl () => 0; + + [Kept] + public static void Test () + { + UsesAllMethods.StaticMethodCalledOnConcreteType (); + var x = new UsesAllMethods (); + x.InstanceMethod (); + CallExplicitImplMethod (); + + void CallExplicitImplMethod () where T : IStaticAndInstanceMethods + { + T.StaticMethodExplicitImpl (); + } + } + } + + [Kept] + [KeptInterface (typeof (IStaticAndInstanceMethods))] + class UnusedMethods : IStaticAndInstanceMethods + { + public static int StaticMethodCalledOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + static int IStaticAndInstanceMethods.StaticMethodExplicitImpl () => 0; + + public int InstanceMethod () => 0; + + [Kept] + public static void Test () { } + } + } + + [Kept] + class StaticInterfaceInheritance + { + [Kept] + interface IBase1 + { + static abstract int UsedOnConcreteType (); + + [Kept] + static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + static abstract int UsedOnConstrainedTypeExplicitImpl (); + static abstract int UnusedImplicitImpl (); + static abstract int UnusedExplicitImpl (); + } + + [Kept] + [KeptInterface (typeof (IBase1))] + interface IInheritsFromBase : IBase1 + { + static abstract int UsedOnConcreteType (); + static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + static abstract int UsedOnConstrainedTypeExplicitImpl (); + static abstract int UnusedImplicitImpl (); + static abstract int UnusedExplicitImpl (); + } + + [Kept] + interface IBase2 + { + static abstract int UsedOnConcreteType (); + + [Kept] + static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + static abstract int UsedOnConstrainedTypeExplicitImpl (); + static abstract int UnusedImplicitImpl (); + static abstract int UnusedExplicitImpl (); + } + + [Kept] + [KeptInterface (typeof (IBase1))] + [KeptInterface (typeof (IBase2))] + interface IInheritsFromMultipleBases : IBase1, IBase2, IUnusedInterface + { + static abstract int UsedOnConcreteType (); + static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + static abstract int UsedOnConstrainedTypeExplicitImpl (); + static abstract int UnusedImplicitImpl (); + static abstract int UnusedExplicitImpl (); + } + + interface IUnusedInterface + { + static abstract int UsedOnConcreteType (); + + static abstract int UnusedImplicitImpl (); + + static abstract int UnusedExplicitImpl (); + } + + [Kept] + [KeptInterface (typeof (IBase1))] + [KeptInterface (typeof (IInheritsFromBase))] + class ImplementsIInheritsFromBase : IInheritsFromBase + { + [Kept] + [RemovedOverride (typeof (IInheritsFromBase))] + [RemovedOverride (typeof (IBase1))] + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof(IBase1))] + [RemovedOverride (typeof (IInheritsFromBase))] + public static int UsedOnBaseOnlyConstrainedTypeImplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof(IInheritsFromBase))] + static int IInheritsFromBase.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof(IBase1))] + static int IBase1.UsedOnConstrainedTypeExplicitImpl () => 0; + + public static int UnusedImplicitImpl () =>0; + + static int IBase1.UnusedExplicitImpl () => 0; + + static int IInheritsFromBase.UnusedExplicitImpl () => 0; + + [Kept] + public static void Test () + { + ImplementsIInheritsFromBase.UsedOnConcreteType (); + CallBase1TypeConstrainedMethod (); + CallSingleInheritTypeConstrainedMethod (); + } + } + + [KeptInterface (typeof (IInheritsFromMultipleBases))] + [KeptInterface (typeof (IBase1))] + [KeptInterface (typeof (IBase2))] + // [RemovedInterface (typeof (IUnusedInterface))] + class ImplementsIInheritsFromTwoBases : IInheritsFromMultipleBases + { + [Kept] + [RemovedOverride (typeof (IInheritsFromMultipleBases))] + [RemovedOverride (typeof (IBase1))] + [RemovedOverride (typeof (IBase2))] + [RemovedOverride (typeof (IUnusedInterface))] + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof(IBase1))] + [KeptOverride (typeof(IBase2))] + [RemovedOverride (typeof (IInheritsFromMultipleBases))] + public static int UsedOnBaseOnlyConstrainedTypeImplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof(IBase1))] + static int IBase1.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof(IBase2))] + static int IBase2.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof(IInheritsFromMultipleBases))] + static int IInheritsFromMultipleBases.UsedOnConstrainedTypeExplicitImpl () => 0; + + public static int UnusedImplicitImpl () =>0; + + static int IBase1.UnusedExplicitImpl () => 0; + + static int IBase2.UnusedExplicitImpl () => 0; + + static int IInheritsFromMultipleBases.UnusedExplicitImpl () => 0; + + static int IUnusedInterface.UnusedExplicitImpl () => 0; + + [Kept] + public static void Test () + { + ImplementsIInheritsFromTwoBases.UsedOnConcreteType (); + CallBase1TypeConstrainedMethod (); + CallBase2TypeConstrainedMethod (); + CallDoubleInheritTypeConstrainedMethod (); + } + } + + [Kept] + static void CallBase1TypeConstrainedMethod () where T: IBase1 + { + T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + static void CallBase2TypeConstrainedMethod () where T: IBase2 + { + T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + static void CallSingleInheritTypeConstrainedMethod () where T: IInheritsFromBase + { + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + static void CallDoubleInheritTypeConstrainedMethod () where T: IInheritsFromMultipleBases + { + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + public static void Test () + { + ImplementsIInheritsFromBase.Test (); + ImplementsIInheritsFromTwoBases.Test (); + } + } + + [Kept] + class GenericStaticInterface + { + [Kept] + interface IGenericInterface + { + static abstract T GetT (); + [Kept] + static abstract T GetTExplicit (); + } + + [Kept] + [KeptInterface (typeof (IGenericInterface))] + public class ImplementsGenericInterface : IGenericInterface + { + [Kept] + [RemovedOverride (typeof (IGenericInterface))] + public static int GetT () => 0; + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static int IGenericInterface.GetTExplicit () => 0; + } + + [Kept] + [KeptInterface (typeof (IGenericInterface))] + class ImplementsGenericInterfaceUnused : IGenericInterface + { + public static int GetT () => 0; + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static int IGenericInterface.GetTExplicit () => 0; + } + + [Kept] + public static void Test () + { + ImplementsGenericInterface.GetT (); + CallExplicitMethod (); + Type t = typeof (ImplementsGenericInterfaceUnused); + + } + + [Kept] + static void CallExplicitMethod () where T : IGenericInterface + { + T.GetTExplicit (); + } + } + + [Kept] + class RecursiveGenericInterface + { + [Kept] + interface IGenericInterface where T : IGenericInterface + { + static abstract T GetT (); + [Kept] + static abstract T GetTExplicit (); + } + + [Kept] + [KeptInterface (typeof (IGenericInterface))] + class ImplementsIGenericInterfaceOfSelf : IGenericInterface + { + [Kept] + [RemovedOverride (typeof (IGenericInterface))] + public static ImplementsIGenericInterfaceOfSelf GetT () => throw new NotImplementedException (); + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static ImplementsIGenericInterfaceOfSelf IGenericInterface.GetTExplicit () + => throw new NotImplementedException (); + } + + [Kept] + [KeptInterface (typeof (IGenericInterface))] + class ImplementsIGenericInterfaceOfOther : IGenericInterface + { + [Kept] + [RemovedOverride (typeof (IGenericInterface))] + public static ImplementsIGenericInterfaceOfSelf GetT () => throw new NotImplementedException (); + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static ImplementsIGenericInterfaceOfSelf IGenericInterface.GetTExplicit () + => throw new NotImplementedException (); + } + + [Kept] + [KeptInterface (typeof (IGenericInterface))] + class ImplementsIGenericInterfaceOfSelfUnused : IGenericInterface + { + public static ImplementsIGenericInterfaceOfSelfUnused GetT () => throw new NotImplementedException (); + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static ImplementsIGenericInterfaceOfSelfUnused IGenericInterface.GetTExplicit () + => throw new NotImplementedException (); + } + + [Kept] + public static void Test () + { + ImplementsIGenericInterfaceOfSelf.GetT (); + ImplementsIGenericInterfaceOfOther.GetT (); + CallExplicitGetT (); + CallExplicitGetT (); + + Type t = typeof (ImplementsIGenericInterfaceOfSelfUnused); + } + + [Kept] + static void CallExplicitGetT () where T : IGenericInterface + { + T.GetTExplicit (); + } + } + } +} diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index aa346b38af19..ccd92ca5f73a 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -172,12 +172,13 @@ protected virtual void VerifyTypeDefinitionKept (TypeDefinition original, TypeDe linkedMembers.Remove (f.FullName); } - foreach (var m in original.Methods) { - if (verifiedEventMethods.Contains (m.FullName)) + foreach (var originalMethod in original.Methods) { + if (verifiedEventMethods.Contains (originalMethod.FullName)) continue; - var msign = m.GetSignature (); - VerifyMethod (m, linked?.Methods.FirstOrDefault (l => msign == l.GetSignature ())); - linkedMembers.Remove (m.FullName); + var methodSignature = originalMethod.GetSignature (); + var linkedMethod = linked?.Methods.FirstOrDefault (l => methodSignature == l.GetSignature ()); + VerifyMethod (originalMethod, linkedMethod); + linkedMembers.Remove (originalMethod.FullName); } } @@ -212,6 +213,35 @@ void VerifyInterfaces (TypeDefinition src, TypeDefinition linked) } } + void VerifyOverrides (MethodDefinition original, MethodDefinition linked) + { + if (linked is null) + return; + var expectedBaseTypesOverridden = new HashSet (original.CustomAttributes + .Where (ca => ca.AttributeType.Name == nameof (KeptOverrideAttribute)) + .Select (ca => (ca.ConstructorArguments[0].Value as TypeReference).FullName)); + var originalBaseTypesOverridden = new HashSet (original.Overrides.Select (ov => ov.DeclaringType.FullName)); + var linkedBaseTypesOverridden = new HashSet (linked.Overrides.Select (ov => ov.DeclaringType.FullName)); + foreach (var expectedBaseType in expectedBaseTypesOverridden) { + Assert.IsTrue (originalBaseTypesOverridden.Contains (expectedBaseType), + $"Method {linked.FullName} was expected to keep override {expectedBaseType}::{linked.Name}, " + + "but it wasn't in the unlinked assembly"); + Assert.IsTrue (linkedBaseTypesOverridden.Contains (expectedBaseType), + $"Method {linked.FullName} was expected to override {expectedBaseType}::{linked.Name}"); + } + + var expectedBaseTypesNotOverridden = new HashSet (original.CustomAttributes + .Where (ca => ca.AttributeType.Name == nameof (RemovedOverrideAttribute)) + .Select (ca => (ca.ConstructorArguments[0].Value as TypeReference).FullName)); + foreach (var expectedRemovedBaseType in expectedBaseTypesNotOverridden) { + Assert.IsTrue (originalBaseTypesOverridden.Contains (expectedRemovedBaseType), + $"Method {linked.FullName} was expected to remove override {expectedRemovedBaseType}::{linked.Name}, " + + $"but it wasn't in the unlinked assembly"); + Assert.IsFalse (linkedBaseTypesOverridden.Contains (expectedRemovedBaseType), + $"Method {linked.FullName} was expected to not override {expectedRemovedBaseType}::{linked.Name}"); + } + } + static string FormatBaseOrInterfaceAttributeValue (CustomAttribute attr) { if (attr.ConstructorArguments.Count == 1) @@ -373,6 +403,7 @@ protected virtual void VerifyMethodKept (MethodDefinition src, MethodDefinition VerifySecurityAttributes (src, linked); VerifyArrayInitializers (src, linked); VerifyMethodBody (src, linked); + VerifyOverrides (src, linked); } protected virtual void VerifyMethodBody (MethodDefinition src, MethodDefinition linked) From 68c530da147492be759935ed240c02c379e7c8e0 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 12 May 2022 16:21:11 -0500 Subject: [PATCH 02/29] Add static interface tests for library mode --- ...nterfaces.StaticInterfaceMethodsTests.g.cs | 25 + .../StaticAbstractInterfaceMethods.cs | 251 +++-- .../StaticAbstractInterfaceMethodsLibrary.cs | 897 ++++++++++++++++++ .../Libraries/RootLibrary.cs | 1 - 4 files changed, 1080 insertions(+), 94 deletions(-) create mode 100644 test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs create mode 100644 test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs diff --git a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs new file mode 100644 index 000000000000..bac4335d6064 --- /dev/null +++ b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs @@ -0,0 +1,25 @@ +using System; +using System.Threading.Tasks; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests.Inheritance.Interfaces +{ + public sealed partial class StaticInterfaceMethodsTests : LinkerTestBase + { + + protected override string TestSuiteName => "Inheritance.Interfaces.StaticInterfaceMethods"; + + [Fact] + public Task StaticAbstractInterfaceMethods () + { + return RunTest (allowMissingWarnings: true); + } + + [Fact] + public Task StaticAbstractInterfaceMethodsLibrary () + { + return RunTest (allowMissingWarnings: true); + } + + } +} \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs index 6d7bf0d25bb3..2cf612134597 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -14,30 +14,32 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods { public class StaticAbstractInterfaceMethods { - static void Main () + public static void Main () { InterfaceMethodsUsedThroughConstrainedType.Test (); InterfaceWithMethodsUsedEachWay.Test (); InterfaceMethodUsedOnConcreteType.Test (); InterfaceMethodsKeptThroughReflection.Test (); + InterfaceHasStaticAndInstanceMethods.Test (); StaticInterfaceInheritance.Test (); GenericStaticInterface.Test (); RecursiveGenericInterface.Test (); + UnusedInterfaces.Test (); } [Kept] - class InterfaceMethodsUsedThroughConstrainedType + public class InterfaceMethodsUsedThroughConstrainedType { [Kept] - internal interface IUsedThroughConstrainedType + public interface IUsedThroughConstrainedType { [Kept] - static abstract int UsedThroughConstrainedType (); + public static abstract int UsedThroughConstrainedType (); } [Kept] [KeptInterface (typeof (IUsedThroughConstrainedType))] - class UsesIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType + public class UsesIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType { [Kept] [KeptOverride (typeof (IUsedThroughConstrainedType))] @@ -46,7 +48,7 @@ class UsesIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType [Kept] [KeptInterface (typeof (IUsedThroughConstrainedType))] - class UnusedIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType + public class UnusedIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType { [Kept] [KeptOverride (typeof (IUsedThroughConstrainedType))] @@ -54,7 +56,7 @@ class UnusedIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType } [Kept] - static void CallMethodOnConstrainedType () where T : IUsedThroughConstrainedType + public static void CallMethodOnConstrainedType () where T : IUsedThroughConstrainedType { T.UsedThroughConstrainedType (); } @@ -74,7 +76,7 @@ public class ExplicitImplementation { [Kept] [KeptInterface (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] - class UsedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTypeExplicitImplementation + public class UsedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTypeExplicitImplementation { [Kept] [KeptOverride (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] @@ -83,7 +85,7 @@ class UsedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTy [Kept] [KeptInterface (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] - class UnusedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTypeExplicitImplementation + public class UnusedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTypeExplicitImplementation { [Kept] [KeptOverride (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] @@ -91,14 +93,14 @@ class UnusedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrained } [Kept] - internal interface IUsedThroughConstrainedTypeExplicitImplementation + public interface IUsedThroughConstrainedTypeExplicitImplementation { [Kept] - static abstract int UsedThroughConstrainedType (); + public static abstract int UsedThroughConstrainedType (); } [Kept] - static void CallTypeConstrainedMethod () where T : IUsedThroughConstrainedTypeExplicitImplementation + public static void CallTypeConstrainedMethod () where T : IUsedThroughConstrainedTypeExplicitImplementation { T.UsedThroughConstrainedType (); } @@ -114,10 +116,10 @@ public static void Test () } [Kept] - class InterfaceMethodUsedOnConcreteType + public class InterfaceMethodUsedOnConcreteType { [Kept] - class UsesIUsedOnConcreteTypeMethods : IUsedOnConcreteType + public class UsesIUsedOnConcreteTypeMethods : IUsedOnConcreteType { [Kept] [RemovedOverride (typeof (IUsedOnConcreteType))] @@ -125,14 +127,14 @@ class UsesIUsedOnConcreteTypeMethods : IUsedOnConcreteType } [Kept] - class UnusedIUsedOnConcreteTypeMethods : IUsedOnConcreteType + public class UnusedIUsedOnConcreteTypeMethods : IUsedOnConcreteType { public static int UsedOnConcreteType () => 0; } - internal interface IUsedOnConcreteType + public interface IUsedOnConcreteType { - static abstract int UsedOnConcreteType (); + public static abstract int UsedOnConcreteType (); } [Kept] @@ -145,24 +147,24 @@ public static void Test () } [Kept] - class InterfaceWithMethodsUsedEachWay + public class InterfaceWithMethodsUsedEachWay { [Kept] - internal interface IUsedEveryWay + public interface IUsedEveryWay { [Kept] - static abstract int UsedThroughConstrainedType (); + public static abstract int UsedThroughConstrainedType (); - static abstract int UsedOnConcreteType (); + public static abstract int UsedOnConcreteType (); [Kept] - static abstract int UsedThroughConstrainedTypeExplicit (); + public static abstract int UsedThroughConstrainedTypeExplicit (); } [Kept] [KeptInterface (typeof (IUsedEveryWay))] - class UsedIUsedEveryWay : IUsedEveryWay + public class UsedIUsedEveryWay : IUsedEveryWay { [Kept] @@ -180,7 +182,7 @@ class UsedIUsedEveryWay : IUsedEveryWay [Kept] [KeptInterface (typeof (IUsedEveryWay))] - class UnusedIUsedEveryWay : IUsedEveryWay + public class UnusedIUsedEveryWay : IUsedEveryWay { [Kept] [KeptOverride (typeof (IUsedEveryWay))] @@ -194,7 +196,7 @@ class UnusedIUsedEveryWay : IUsedEveryWay } [Kept] - static void CallTypeConstrainedMethods () where T : IUsedEveryWay + public static void CallTypeConstrainedMethods () where T : IUsedEveryWay { T.UsedThroughConstrainedType (); T.UsedThroughConstrainedTypeExplicit (); @@ -211,10 +213,10 @@ public static void Test () } [Kept] - internal class InterfaceMethodsKeptThroughReflection + public class InterfaceMethodsKeptThroughReflection { [Kept] - internal interface IMethodsKeptThroughReflection + public interface IMethodsKeptThroughReflection { [Kept] public static abstract int UnusedMethod (); @@ -228,7 +230,7 @@ internal interface IMethodsKeptThroughReflection [Kept] [KeptInterface (typeof (IMethodsKeptThroughReflection))] - class UsedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection + public class UsedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection { [Kept] [KeptOverride (typeof (IMethodsKeptThroughReflection))] @@ -245,7 +247,7 @@ class UsedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection [Kept] [KeptInterface (typeof (IMethodsKeptThroughReflection))] - class UnusedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection + public class UnusedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection { [Kept] [KeptOverride (typeof (IMethodsKeptThroughReflection))] @@ -268,6 +270,7 @@ public static void Test () UseMethodThroughTypeConstraint (); Type t = typeof (UnusedMethodsKeptThroughtReflection); + [Kept] static void UseMethodThroughTypeConstraint () where T : IMethodsKeptThroughReflection { @@ -276,30 +279,38 @@ static void UseMethodThroughTypeConstraint () where T : IMethodsKeptThroughRe } } - class InterfaceHasStaticAndInstanceMethods + [Kept] + public class InterfaceHasStaticAndInstanceMethods { [Kept] - interface IStaticAndInstanceMethods + public interface IStaticAndInstanceMethods { - static abstract int StaticMethodCalledOnConcreteType (); + public static abstract int StaticMethodCalledOnConcreteType (); [Kept] - static abstract int StaticMethodExplicitImpl (); + public static abstract int StaticMethodExplicitImpl (); [Kept] - int InstanceMethod (); + public int InstanceMethod (); } [Kept] + public static void CallExplicitImplMethod () where T : IStaticAndInstanceMethods + { + T.StaticMethodExplicitImpl (); + } + + [Kept] + [KeptMember (".ctor()")] [KeptInterface (typeof (IStaticAndInstanceMethods))] - class UsesAllMethods : IStaticAndInstanceMethods + public class UsesAllMethods : IStaticAndInstanceMethods { [Kept] [RemovedOverride (typeof (IStaticAndInstanceMethods))] public static int StaticMethodCalledOnConcreteType () => 0; [Kept] - [KeptOverride (typeof (IStaticAndInstanceMethods))] + // Non-static implementation methods don't explicitly override the interface method public int InstanceMethod () => 0; [Kept] @@ -311,19 +322,15 @@ public static void Test () { UsesAllMethods.StaticMethodCalledOnConcreteType (); var x = new UsesAllMethods (); - x.InstanceMethod (); + ((IStaticAndInstanceMethods)x).InstanceMethod (); CallExplicitImplMethod (); - - void CallExplicitImplMethod () where T : IStaticAndInstanceMethods - { - T.StaticMethodExplicitImpl (); - } } } [Kept] - [KeptInterface (typeof (IStaticAndInstanceMethods))] - class UnusedMethods : IStaticAndInstanceMethods + // Bug + //[KeptInterface (typeof (IStaticAndInstanceMethods))] + public class UnusedMethods : IStaticAndInstanceMethods { public static int StaticMethodCalledOnConcreteType () => 0; @@ -336,79 +343,86 @@ class UnusedMethods : IStaticAndInstanceMethods [Kept] public static void Test () { } } + + [Kept] + public static void Test () + { + UsesAllMethods.Test (); + UnusedMethods.Test (); + } } [Kept] - class StaticInterfaceInheritance + public class StaticInterfaceInheritance { [Kept] - interface IBase1 + public interface IBase1 { - static abstract int UsedOnConcreteType (); + public static abstract int UsedOnConcreteType (); [Kept] - static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); [Kept] - static abstract int UsedOnConstrainedTypeExplicitImpl (); - static abstract int UnusedImplicitImpl (); - static abstract int UnusedExplicitImpl (); + public static abstract int UsedOnConstrainedTypeExplicitImpl (); + public static abstract int UnusedImplicitImpl (); + public static abstract int UnusedExplicitImpl (); } [Kept] [KeptInterface (typeof (IBase1))] - interface IInheritsFromBase : IBase1 + public interface IInheritsFromBase : IBase1 { - static abstract int UsedOnConcreteType (); - static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + public static abstract int UsedOnConcreteType (); + public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); [Kept] - static abstract int UsedOnConstrainedTypeExplicitImpl (); - static abstract int UnusedImplicitImpl (); - static abstract int UnusedExplicitImpl (); + public static abstract int UsedOnConstrainedTypeExplicitImpl (); + public static abstract int UnusedImplicitImpl (); + public static abstract int UnusedExplicitImpl (); } [Kept] - interface IBase2 + public interface IBase2 { - static abstract int UsedOnConcreteType (); + public static abstract int UsedOnConcreteType (); [Kept] - static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); [Kept] - static abstract int UsedOnConstrainedTypeExplicitImpl (); - static abstract int UnusedImplicitImpl (); - static abstract int UnusedExplicitImpl (); + public static abstract int UsedOnConstrainedTypeExplicitImpl (); + public static abstract int UnusedImplicitImpl (); + public static abstract int UnusedExplicitImpl (); } [Kept] [KeptInterface (typeof (IBase1))] [KeptInterface (typeof (IBase2))] - interface IInheritsFromMultipleBases : IBase1, IBase2, IUnusedInterface + public interface IInheritsFromMultipleBases : IBase1, IBase2, IUnusedInterface { - static abstract int UsedOnConcreteType (); - static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + public static abstract int UsedOnConcreteType (); + public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); [Kept] - static abstract int UsedOnConstrainedTypeExplicitImpl (); - static abstract int UnusedImplicitImpl (); - static abstract int UnusedExplicitImpl (); + public static abstract int UsedOnConstrainedTypeExplicitImpl (); + public static abstract int UnusedImplicitImpl (); + public static abstract int UnusedExplicitImpl (); } - interface IUnusedInterface + public interface IUnusedInterface { - static abstract int UsedOnConcreteType (); + public static abstract int UsedOnConcreteType (); - static abstract int UnusedImplicitImpl (); + public static abstract int UnusedImplicitImpl (); - static abstract int UnusedExplicitImpl (); + public static abstract int UnusedExplicitImpl (); } [Kept] [KeptInterface (typeof (IBase1))] [KeptInterface (typeof (IInheritsFromBase))] - class ImplementsIInheritsFromBase : IInheritsFromBase + public class ImplementsIInheritsFromBase : IInheritsFromBase { [Kept] [RemovedOverride (typeof (IInheritsFromBase))] @@ -447,7 +461,7 @@ public static void Test () [KeptInterface (typeof (IBase1))] [KeptInterface (typeof (IBase2))] // [RemovedInterface (typeof (IUnusedInterface))] - class ImplementsIInheritsFromTwoBases : IInheritsFromMultipleBases + public class ImplementsIInheritsFromTwoBases : IInheritsFromMultipleBases { [Kept] [RemovedOverride (typeof (IInheritsFromMultipleBases))] @@ -495,27 +509,27 @@ public static void Test () } [Kept] - static void CallBase1TypeConstrainedMethod () where T: IBase1 + public static void CallBase1TypeConstrainedMethod () where T: IBase1 { T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); T.UsedOnConstrainedTypeExplicitImpl (); } [Kept] - static void CallBase2TypeConstrainedMethod () where T: IBase2 + public static void CallBase2TypeConstrainedMethod () where T: IBase2 { T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); T.UsedOnConstrainedTypeExplicitImpl (); } [Kept] - static void CallSingleInheritTypeConstrainedMethod () where T: IInheritsFromBase + public static void CallSingleInheritTypeConstrainedMethod () where T: IInheritsFromBase { T.UsedOnConstrainedTypeExplicitImpl (); } [Kept] - static void CallDoubleInheritTypeConstrainedMethod () where T: IInheritsFromMultipleBases + public static void CallDoubleInheritTypeConstrainedMethod () where T: IInheritsFromMultipleBases { T.UsedOnConstrainedTypeExplicitImpl (); } @@ -529,14 +543,14 @@ public static void Test () } [Kept] - class GenericStaticInterface + public class GenericStaticInterface { [Kept] - interface IGenericInterface + public interface IGenericInterface { - static abstract T GetT (); + public static abstract T GetT (); [Kept] - static abstract T GetTExplicit (); + public static abstract T GetTExplicit (); } [Kept] @@ -554,7 +568,7 @@ public class ImplementsGenericInterface : IGenericInterface [Kept] [KeptInterface (typeof (IGenericInterface))] - class ImplementsGenericInterfaceUnused : IGenericInterface + public class ImplementsGenericInterfaceUnused : IGenericInterface { public static int GetT () => 0; [Kept] @@ -572,26 +586,26 @@ public static void Test () } [Kept] - static void CallExplicitMethod () where T : IGenericInterface + public static void CallExplicitMethod () where T : IGenericInterface { T.GetTExplicit (); } } [Kept] - class RecursiveGenericInterface + public class RecursiveGenericInterface { [Kept] - interface IGenericInterface where T : IGenericInterface + public interface IGenericInterface where T : IGenericInterface { - static abstract T GetT (); + public static abstract T GetT (); [Kept] - static abstract T GetTExplicit (); + public static abstract T GetTExplicit (); } [Kept] [KeptInterface (typeof (IGenericInterface))] - class ImplementsIGenericInterfaceOfSelf : IGenericInterface + public class ImplementsIGenericInterfaceOfSelf : IGenericInterface { [Kept] [RemovedOverride (typeof (IGenericInterface))] @@ -605,7 +619,7 @@ static ImplementsIGenericInterfaceOfSelf IGenericInterface))] - class ImplementsIGenericInterfaceOfOther : IGenericInterface + public class ImplementsIGenericInterfaceOfOther : IGenericInterface { [Kept] [RemovedOverride (typeof (IGenericInterface))] @@ -619,7 +633,7 @@ static ImplementsIGenericInterfaceOfSelf IGenericInterface))] - class ImplementsIGenericInterfaceOfSelfUnused : IGenericInterface + public class ImplementsIGenericInterfaceOfSelfUnused : IGenericInterface { public static ImplementsIGenericInterfaceOfSelfUnused GetT () => throw new NotImplementedException (); @@ -641,10 +655,61 @@ public static void Test () } [Kept] - static void CallExplicitGetT () where T : IGenericInterface + public static void CallExplicitGetT () where T : IGenericInterface { T.GetTExplicit (); } } + + [Kept] + public class UnusedInterfaces + { + public interface IUnusedInterface + { + public int UnusedMethodImplicit (); + public int UnusedMethodExplicit (); + } + + [Kept] + public interface IUnusedMethods + { + public int UnusedMethodImplicit (); + public int UnusedMethodExplicit (); + } + + [Kept] + public class ImplementsUnusedInterface : IUnusedInterface + { + int IUnusedInterface.UnusedMethodExplicit () => 0; + + public int UnusedMethodImplicit () => 0; + } + + [Kept] + // In link mode, if we remove all methods from the interface, we should be able to remove the interface. We need it now since we don't remove the type constraint from UsesIUnusedMethods + [KeptInterface (typeof (IUnusedMethods))] + public class ImplementsIUnusedMethods : IUnusedMethods + { + int IUnusedMethods.UnusedMethodExplicit () => 0; + + public int UnusedMethodImplicit () => 0; + } + + [Kept] + // In link mode, if there are no constrained calls we should be able to remove the type constraint + public static void UsesIUnusedMethods () where T : IUnusedMethods { } + + [Kept] + public static void Test () + { + UsesIUnusedMethods (); + Type t = typeof (ImplementsUnusedInterface); + } + } + + public class ClassInheritance + { + + } } } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs new file mode 100644 index 000000000000..c931913671bb --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs @@ -0,0 +1,897 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods +{ + [SetupLinkerArgument ("-a", "test.exe", "library")] + public static class StaticAbstractInterfaceMethodsLibrary + { + public static void Main () + { + InterfaceMethodsUsedThroughConstrainedType.Test (); + InterfaceWithMethodsUsedEachWay.Test (); + InterfaceMethodUsedOnConcreteType.Test (); + InterfaceMethodsKeptThroughReflection.Test (); + StaticInterfaceInheritance.Test (); + GenericStaticInterface.Test (); + RecursiveGenericInterface.Test (); + UnusedInterfaces.Test (); + } + + [Kept] + public static class InterfaceMethodsUsedThroughConstrainedType + { + [Kept] + public interface IUsedThroughConstrainedType + { + [Kept] + static abstract int UsedThroughConstrainedType (); + } + + [Kept] + internal interface IUsedThroughConstrainedTypeInternal + { + static abstract int UsedThroughConstrainedType (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IUsedThroughConstrainedType))] + [KeptInterface (typeof (IUsedThroughConstrainedTypeInternal))] + public class UsesIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType, IUsedThroughConstrainedTypeInternal + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedType))] + [RemovedOverride (typeof (IUsedThroughConstrainedTypeInternal))] + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IUsedThroughConstrainedType))] + [KeptInterface (typeof (IUsedThroughConstrainedTypeInternal))] + public class UnusedIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedType, IUsedThroughConstrainedTypeInternal + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedType))] + [RemovedOverride (typeof (IUsedThroughConstrainedTypeInternal))] + public static int UsedThroughConstrainedType () => 0; + } + + // Should this be kept? + private class UnusedIUsedThroughConstrainedTypeMethodsPrivate : IUsedThroughConstrainedType, IUsedThroughConstrainedTypeInternal + { + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + public static void CallMethodOnConstrainedType () where T : IUsedThroughConstrainedType + { + T.UsedThroughConstrainedType (); + } + + [Kept] + public static void Test () + { + CallMethodOnConstrainedType (); + Type t = typeof (UnusedIUsedThroughConstrainedTypeMethods); + + ExplicitImplementation.Test (); + } + + [Kept] + public static class ExplicitImplementation + { + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + public class UsedIUsedThroughConstrainedTypeExplicitMethods : IUsedThroughConstrainedTypeExplicitImplementation + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + static int IUsedThroughConstrainedTypeExplicitImplementation.UsedThroughConstrainedType () => 0; + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + public class UnusedIUsedThroughConstrainedTypeExplicitMethods + : IUsedThroughConstrainedTypeExplicitImplementation + { + [Kept] + [KeptOverride (typeof (IUsedThroughConstrainedTypeExplicitImplementation))] + static int IUsedThroughConstrainedTypeExplicitImplementation.UsedThroughConstrainedType () => 0; + } + + [Kept] + public interface IUsedThroughConstrainedTypeExplicitImplementation + { + [Kept] + static abstract int UsedThroughConstrainedType (); + } + + [Kept] + public static void CallTypeConstrainedMethod () where T : IUsedThroughConstrainedTypeExplicitImplementation + { + T.UsedThroughConstrainedType (); + } + + [Kept] + public static void Test () + { + CallTypeConstrainedMethod (); + + Type t = typeof (UnusedIUsedThroughConstrainedTypeExplicitMethods); + } + } + } + + [Kept] + public static class InterfaceMethodUsedOnConcreteType + { + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IUsedOnConcreteType))] + [KeptInterface (typeof (IUsedOnConcreteTypeInternal))] + public class UsesIUsedOnConcreteTypeMethods : IUsedOnConcreteType, IUsedOnConcreteTypeInternal + { + [Kept] + [KeptOverride (typeof (IUsedOnConcreteType))] + [RemovedOverride (typeof (IUsedOnConcreteTypeInternal))] + public static int UsedOnConcreteType () => 0; + } + + [Kept] + public interface IUsedOnConcreteType + { + [Kept] + public static abstract int UsedOnConcreteType (); + } + + [Kept] + internal interface IUsedOnConcreteTypeInternal + { + static abstract int UsedOnConcreteType (); + } + + [Kept] + public static void Test () + { + UsesIUsedOnConcreteTypeMethods.UsedOnConcreteType (); + } + } + + [Kept] + public static class InterfaceWithMethodsUsedEachWay + { + + [Kept] + public interface IUsedEveryWay + { + [Kept] + public static abstract int UsedThroughConstrainedType (); + + [Kept] + public static abstract int UsedOnConcreteType (); + + [Kept] + public static abstract int UsedThroughConstrainedTypeExplicit (); + } + + [Kept] + internal interface IUsedEveryWayInternal + { + [Kept] + internal static abstract int UsedThroughConstrainedType (); + + internal static abstract int UsedOnConcreteType (); + + [Kept] + internal static abstract int UsedThroughConstrainedTypeExplicit (); + } + + [Kept] + internal interface IUnusedEveryWayInternal + { + internal static abstract int UsedThroughConstrainedType (); + + internal static abstract int UsedOnConcreteType (); + + internal static abstract int UsedThroughConstrainedTypeExplicit (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IUsedEveryWay))] + [KeptInterface (typeof (IUsedEveryWayInternal))] + [KeptInterface (typeof (IUnusedEveryWayInternal))] + public class UsedIUsedEveryWay : IUsedEveryWay, IUsedEveryWayInternal, IUnusedEveryWayInternal + { + + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + static int IUsedEveryWay.UsedThroughConstrainedTypeExplicit () => 0; + + [Kept] + [KeptOverride (typeof (IUsedEveryWayInternal))] + static int IUsedEveryWayInternal.UsedThroughConstrainedTypeExplicit () => 0; + + static int IUnusedEveryWayInternal.UsedThroughConstrainedTypeExplicit () => 0; + + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + [RemovedOverride (typeof (IUsedEveryWayInternal))] + [RemovedOverride (typeof (IUnusedEveryWayInternal))] + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + [KeptOverride (typeof (IUsedEveryWayInternal))] + [RemovedOverride (typeof (IUnusedEveryWayInternal))] + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + [KeptInterface (typeof (IUsedEveryWay))] + [KeptInterface (typeof (IUsedEveryWayInternal))] + [KeptInterface (typeof (IUnusedEveryWayInternal))] + internal class UnusedIUsedEveryWayInternal : IUsedEveryWay, IUsedEveryWayInternal, IUnusedEveryWayInternal + { + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + static int IUsedEveryWay.UsedThroughConstrainedTypeExplicit () => 0; + + [Kept] + [KeptOverride (typeof (IUsedEveryWayInternal))] + static int IUsedEveryWayInternal.UsedThroughConstrainedTypeExplicit () => 0; + + static int IUnusedEveryWayInternal.UsedThroughConstrainedTypeExplicit () => 0; + + [Kept] + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IUsedEveryWay))] + [KeptOverride (typeof (IUsedEveryWayInternal))] + public static int UsedThroughConstrainedType () => 0; + } + + [Kept] + public static void CallTypeConstrainedMethods () where T : IUsedEveryWay + { + T.UsedThroughConstrainedType (); + T.UsedThroughConstrainedTypeExplicit (); + } + + [Kept] + internal static void CallTypeConstrainedMethodsInternal () where T : IUsedEveryWayInternal + { + T.UsedThroughConstrainedType (); + T.UsedThroughConstrainedTypeExplicit (); + } + + [Kept] + public static void Test () + { + UsedIUsedEveryWay.UsedOnConcreteType (); + CallTypeConstrainedMethods (); + CallTypeConstrainedMethodsInternal (); + + Type t = typeof (UnusedIUsedEveryWayInternal); + } + } + + [Kept] + public static class InterfaceMethodsKeptThroughReflection + { + [Kept] + public interface IMethodsKeptThroughReflection + { + [Kept] + public static abstract int UnusedMethod (); + + [Kept] + public static abstract int UsedOnConcreteType (); + + [Kept] + public static abstract int UsedOnConstrainedType (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IMethodsKeptThroughReflection))] + public class UsedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection + { + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UnusedMethod () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConstrainedType () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConcreteType () => 0; + } + + [Kept] + [KeptInterface (typeof (IMethodsKeptThroughReflection))] + internal class UnusedMethodsKeptThroughtReflection : IMethodsKeptThroughReflection + { + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UnusedMethod () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConstrainedType () => 0; + + [Kept] + [KeptOverride (typeof (IMethodsKeptThroughReflection))] + public static int UsedOnConcreteType () => 0; + } + + [Kept] + public static void Test () + { + typeof (IMethodsKeptThroughReflection).RequiresPublicMethods (); + UsedMethodsKeptThroughtReflection.UsedOnConcreteType (); + UseMethodThroughTypeConstraint (); + + Type t = typeof (UnusedMethodsKeptThroughtReflection); + + [Kept] + static void UseMethodThroughTypeConstraint () where T : IMethodsKeptThroughReflection + { + T.UsedOnConstrainedType (); + } + } + } + + [Kept] + public static class InterfaceHasStaticAndInstanceMethods + { + [Kept] + public interface IStaticAndInstanceMethods + { + [Kept] + public static abstract int StaticMethodCalledOnConcreteType (); + + [Kept] + public static abstract int StaticMethodExplicitImpl (); + + [Kept] + public int InstanceMethod (); + } + + [Kept] + internal interface IStaticAndInstanceMethodsInternalUnused + { + static abstract int StaticMethodCalledOnConcreteType (); + + static abstract int StaticMethodExplicitImpl (); + + int InstanceMethod (); + } + + [Kept] + internal interface IStaticAndInstanceMethodsInternalUsed + { + static abstract int StaticMethodCalledOnConcreteType (); + + [Kept] + static abstract int StaticMethodExplicitImpl (); + + [Kept] + int InstanceMethod (); + } + + [Kept] + internal static void CallExplicitImplMethod () where T : IStaticAndInstanceMethods, new() + { + T.StaticMethodExplicitImpl (); + IStaticAndInstanceMethods x = new T (); + x.InstanceMethod (); + } + + [Kept] + internal static void CallExplicitImplMethodInternalUsed () where T : IStaticAndInstanceMethodsInternalUsed, new() + { + T.StaticMethodExplicitImpl (); + IStaticAndInstanceMethodsInternalUsed x = new T (); + x.InstanceMethod (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IStaticAndInstanceMethods))] + [KeptInterface (typeof (IStaticAndInstanceMethodsInternalUsed))] + [KeptInterface (typeof (IStaticAndInstanceMethodsInternalUnused))] + public class UsesAllMethods : IStaticAndInstanceMethods, IStaticAndInstanceMethodsInternalUnused, IStaticAndInstanceMethodsInternalUsed + { + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + [RemovedOverride (typeof (IStaticAndInstanceMethodsInternalUsed))] + [RemovedOverride (typeof (IStaticAndInstanceMethodsInternalUnused))] + public static int StaticMethodCalledOnConcreteType () => 0; + + [Kept] + // No .override / MethodImpl for implicit instance methods + public int InstanceMethod () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + static int IStaticAndInstanceMethods.StaticMethodExplicitImpl () => 0; + + static int IStaticAndInstanceMethodsInternalUnused.StaticMethodExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethodsInternalUsed))] + static int IStaticAndInstanceMethodsInternalUsed.StaticMethodExplicitImpl () => 0; + + [Kept] + public static void Test () + { + UsesAllMethods.StaticMethodCalledOnConcreteType (); + CallExplicitImplMethod (); + CallExplicitImplMethodInternalUsed (); + } + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IStaticAndInstanceMethods))] + [KeptInterface (typeof (IStaticAndInstanceMethodsInternalUsed))] + [KeptInterface (typeof (IStaticAndInstanceMethodsInternalUnused))] + public class UnusedMethods : IStaticAndInstanceMethods, IStaticAndInstanceMethodsInternalUnused, IStaticAndInstanceMethodsInternalUsed + { + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + [RemovedOverride (typeof (IStaticAndInstanceMethodsInternalUsed))] + [RemovedOverride (typeof (IStaticAndInstanceMethodsInternalUnused))] + public static int StaticMethodCalledOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + static int IStaticAndInstanceMethods.StaticMethodExplicitImpl () => 0; + + static int IStaticAndInstanceMethodsInternalUnused.StaticMethodExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethodsInternalUsed))] + static int IStaticAndInstanceMethodsInternalUsed.StaticMethodExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethods))] + int IStaticAndInstanceMethods.InstanceMethod () => 0; + + [Kept] + [KeptOverride (typeof (IStaticAndInstanceMethodsInternalUsed))] + int IStaticAndInstanceMethodsInternalUsed.InstanceMethod () => 0; + + int IStaticAndInstanceMethodsInternalUnused.InstanceMethod () => 0; + + [Kept] + public static void Test () { } + } + + [Kept] + public static void Test () + { + UsesAllMethods.Test (); + UnusedMethods.Test (); + } + } + + [Kept] + public static class StaticInterfaceInheritance + { + [Kept] + public interface IBase + { + [Kept] + public static abstract int UsedOnConcreteType (); + + [Kept] + public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + public static abstract int UsedOnConstrainedTypeExplicitImpl (); + + [Kept] + public static abstract int UnusedImplicitImpl (); + + [Kept] + public static abstract int UnusedExplicitImpl (); + } + + [Kept] + [KeptInterface (typeof (IBase))] + public interface IInheritsFromBase : IBase + { + [Kept] + public static abstract int UsedOnConcreteType (); + + [Kept] + public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + public static abstract int UsedOnConstrainedTypeExplicitImpl (); + + [Kept] + public static abstract int UnusedImplicitImpl (); + + [Kept] + public static abstract int UnusedExplicitImpl (); + } + + [Kept] + internal interface IBaseInternal + { + static abstract int UsedOnConcreteType (); + + [Kept] + static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + static abstract int UsedOnConstrainedTypeExplicitImpl (); + + static abstract int UnusedImplicitImpl (); + + static abstract int UnusedExplicitImpl (); + } + + [Kept] + [KeptInterface (typeof (IBase))] + [KeptInterface (typeof (IBaseInternal))] + [KeptInterface (typeof (IUnusedInterface))] + internal interface IInheritsFromMultipleBases : IBase, IBaseInternal, IUnusedInterface + { + static abstract int UsedOnConcreteType (); + static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + + [Kept] + static abstract int UsedOnConstrainedTypeExplicitImpl (); + static abstract int UnusedImplicitImpl (); + static abstract int UnusedExplicitImpl (); + } + + [Kept] + internal interface IUnusedInterface + { + static abstract int UsedOnConcreteType (); + + static abstract int UnusedImplicitImpl (); + + static abstract int UnusedExplicitImpl (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IBase))] + [KeptInterface (typeof (IInheritsFromBase))] + public class ImplementsIInheritsFromBase : IInheritsFromBase + { + [Kept] + [KeptOverride (typeof (IInheritsFromBase))] + [KeptOverride (typeof (IBase))] + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + [KeptOverride (typeof (IInheritsFromBase))] + public static int UsedOnBaseOnlyConstrainedTypeImplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IInheritsFromBase))] + static int IInheritsFromBase.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + static int IBase.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + [KeptOverride (typeof (IInheritsFromBase))] + public static int UnusedImplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + static int IBase.UnusedExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IInheritsFromBase))] + static int IInheritsFromBase.UnusedExplicitImpl () => 0; + + [Kept] + public static void Test () + { + ImplementsIInheritsFromBase.UsedOnConcreteType (); + CallBase1TypeConstrainedMethod (); + CallSingleInheritTypeConstrainedMethod (); + } + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IInheritsFromMultipleBases))] + [KeptInterface (typeof (IBase))] + [KeptInterface (typeof (IBaseInternal))] + [KeptInterface (typeof (IUnusedInterface))] + public class ImplementsIInheritsFromTwoBases : IInheritsFromMultipleBases + { + [Kept] + [RemovedOverride (typeof (IInheritsFromMultipleBases))] + [KeptOverride (typeof (IBase))] + [RemovedOverride (typeof (IBaseInternal))] + [RemovedOverride (typeof (IUnusedInterface))] + public static int UsedOnConcreteType () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + [KeptOverride (typeof (IBaseInternal))] + [RemovedOverride (typeof (IInheritsFromMultipleBases))] + public static int UsedOnBaseOnlyConstrainedTypeImplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + static int IBase.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IBaseInternal))] + static int IBaseInternal.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IInheritsFromMultipleBases))] + static int IInheritsFromMultipleBases.UsedOnConstrainedTypeExplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + public static int UnusedImplicitImpl () => 0; + + [Kept] + [KeptOverride (typeof (IBase))] + static int IBase.UnusedExplicitImpl () => 0; + + static int IBaseInternal.UnusedExplicitImpl () => 0; + + static int IInheritsFromMultipleBases.UnusedExplicitImpl () => 0; + + static int IUnusedInterface.UnusedExplicitImpl () => 0; + + [Kept] + public static void Test () + { + ImplementsIInheritsFromTwoBases.UsedOnConcreteType (); + CallBase1TypeConstrainedMethod (); + CallBase2TypeConstrainedMethod (); + CallDoubleInheritTypeConstrainedMethod (); + } + } + + [Kept] + public static void CallBase1TypeConstrainedMethod () where T : IBase + { + T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + internal static void CallBase2TypeConstrainedMethod () where T : IBaseInternal + { + T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + public static void CallSingleInheritTypeConstrainedMethod () where T : IInheritsFromBase + { + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + internal static void CallDoubleInheritTypeConstrainedMethod () where T : IInheritsFromMultipleBases + { + T.UsedOnConstrainedTypeExplicitImpl (); + } + + [Kept] + public static void Test () + { + ImplementsIInheritsFromBase.Test (); + ImplementsIInheritsFromTwoBases.Test (); + } + } + + [Kept] + public static class GenericStaticInterface + { + [Kept] + public interface IGenericInterface + { + [Kept] + public static abstract T GetT (); + [Kept] + public static abstract T GetTExplicit (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IGenericInterface))] + public class ImplementsGenericInterface : IGenericInterface + { + [Kept] + [KeptOverride (typeof (IGenericInterface))] + public static int GetT () => 0; + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static int IGenericInterface.GetTExplicit () => 0; + } + + [Kept] + [KeptInterface (typeof (IGenericInterface))] + internal class ImplementsGenericInterfaceUnused : IGenericInterface + { + [Kept] + [KeptOverride (typeof (IGenericInterface))] + public static int GetT () => 0; + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static int IGenericInterface.GetTExplicit () => 0; + } + + [Kept] + public static void Test () + { + ImplementsGenericInterface.GetT (); + CallExplicitMethod (); + Type t = typeof (ImplementsGenericInterfaceUnused); + } + + [Kept] + public static void CallExplicitMethod () where T : IGenericInterface + { + T.GetTExplicit (); + } + } + + [Kept] + public static class RecursiveGenericInterface + { + [Kept] + public interface IGenericInterface where T : IGenericInterface + { + [Kept] + public static abstract T GetT (); + [Kept] + public static abstract T GetTExplicit (); + } + + [Kept] + internal interface IGenericInterfaceInternal where T : IGenericInterfaceInternal + { + static abstract T GetT (); + + static abstract T GetTExplicit (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IGenericInterface))] + [KeptInterface (typeof (IGenericInterfaceInternal))] + public class ImplementsIGenericInterfaceOfSelf : IGenericInterface, IGenericInterfaceInternal + { + [Kept] + [KeptOverride (typeof (IGenericInterface))] + [RemovedOverride (typeof (IGenericInterfaceInternal))] + public static ImplementsIGenericInterfaceOfSelf GetT () => throw new NotImplementedException (); + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static ImplementsIGenericInterfaceOfSelf IGenericInterface.GetTExplicit () + => throw new NotImplementedException (); + + static ImplementsIGenericInterfaceOfSelf IGenericInterfaceInternal.GetTExplicit () + => throw new NotImplementedException (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IGenericInterface))] + [KeptInterface (typeof (IGenericInterfaceInternal))] + public class ImplementsIGenericInterfaceOfOther : IGenericInterface, IGenericInterfaceInternal + { + [Kept] + [KeptOverride (typeof (IGenericInterface))] + [RemovedOverride (typeof (IGenericInterfaceInternal))] + public static ImplementsIGenericInterfaceOfSelf GetT () => throw new NotImplementedException (); + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static ImplementsIGenericInterfaceOfSelf IGenericInterface.GetTExplicit () + => throw new NotImplementedException (); + + static ImplementsIGenericInterfaceOfSelf IGenericInterfaceInternal.GetTExplicit () + => throw new NotImplementedException (); + } + + [Kept] + [KeptInterface (typeof (IGenericInterface))] + [KeptInterface (typeof (IGenericInterfaceInternal))] + internal class ImplementsIGenericInterfaceOfSelfUnused : IGenericInterface, IGenericInterfaceInternal + { + [Kept] + [KeptOverride (typeof (IGenericInterface))] + public static ImplementsIGenericInterfaceOfSelfUnused GetT () => throw new NotImplementedException (); + + [Kept] + [KeptOverride (typeof (IGenericInterface))] + static ImplementsIGenericInterfaceOfSelfUnused IGenericInterface.GetTExplicit () + => throw new NotImplementedException (); + + static ImplementsIGenericInterfaceOfSelfUnused IGenericInterfaceInternal.GetTExplicit () + => throw new NotImplementedException (); + } + + [Kept] + public static void CallExplicitGetT () where T : IGenericInterface + { + T.GetTExplicit (); + } + + [Kept] + public static void Test () + { + ImplementsIGenericInterfaceOfSelf.GetT (); + ImplementsIGenericInterfaceOfOther.GetT (); + CallExplicitGetT (); + CallExplicitGetT (); + + Type t = typeof (ImplementsIGenericInterfaceOfSelfUnused); + } + } + + [Kept] + public static class UnusedInterfaces + { + [Kept] + internal interface IUnusedInterface + { + int UnusedMethodImplicit (); + int UnusedMethodExplicit (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof(IUnusedInterface))] + public class ImplementsUnusedInterface : IUnusedInterface + { + int IUnusedInterface.UnusedMethodExplicit () => 0; + + [Kept] + // Bug: We should be able to remove this override + //[RemovedOverride (typeof (IUnusedInterface))] + public int UnusedMethodImplicit () => 0; + } + + [Kept] + public static void Test () + { + Type t = typeof (ImplementsUnusedInterface); + } + } + } +} + diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index 1534573ea1bf..68e02b1ac6fb 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -366,7 +366,6 @@ internal interface IInternalInterface [Kept] internal interface IInternalStaticInterface { - [Kept] // https://github.com/dotnet/linker/issues/2733 static abstract void InternalStaticInterfaceMethod (); static abstract void ExplicitImplementationInternalStaticInterfaceMethod (); From 97cd617b2a4992ae1e1e062e7ebf05a17fcfab79 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 12 May 2022 16:22:33 -0500 Subject: [PATCH 03/29] Add sanity check for overrides Make sure that MethodImpls point to an interface that the type implements or a base type that is inherited from. --- .../TestCasesRunner/AssemblyChecker.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index ccd92ca5f73a..517c062185d6 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -240,6 +240,25 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) Assert.IsFalse (linkedBaseTypesOverridden.Contains (expectedRemovedBaseType), $"Method {linked.FullName} was expected to not override {expectedRemovedBaseType}::{linked.Name}"); } + + //foreach (var overriddenMethod in linked.Overrides) { + // if (overriddenMethod.Resolve () is not MethodDefinition overriddenDefinition) { + // Assert.Fail ($"Method {linked.GetDisplayName ()} overrides method {overriddenMethod} which does not exist"); + // } + // else if (overriddenDefinition.DeclaringType.IsInterface) { + // Assert.True (linked.DeclaringType.Interfaces.Select(i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), + // $"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}"); + // } else { + // TypeReference? baseType = linked.DeclaringType!; + // TypeReference overriddenType = overriddenMethod.DeclaringType; + // while (baseType is not null) { + // if (baseType.Equals (overriddenType)) + // break; + // if (baseType.Resolve ()?.BaseType is null) + // Assert.Fail ($"Method {linked} overrides method {overriddenMethod} from, but {linked.DeclaringType} does not inherit from type {overriddenMethod.DeclaringType}"); + // } + // } + //} } static string FormatBaseOrInterfaceAttributeValue (CustomAttribute attr) From 8190bf79f9f1713a06fd3bae9887c4ed272621e0 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 12 May 2022 16:22:33 -0500 Subject: [PATCH 04/29] Add sanity check for overrides Make sure that MethodImpls point to an interface that the type implements or a base type that is inherited from. --- .../TestCasesRunner/AssemblyChecker.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index ccd92ca5f73a..517c062185d6 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -240,6 +240,25 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) Assert.IsFalse (linkedBaseTypesOverridden.Contains (expectedRemovedBaseType), $"Method {linked.FullName} was expected to not override {expectedRemovedBaseType}::{linked.Name}"); } + + //foreach (var overriddenMethod in linked.Overrides) { + // if (overriddenMethod.Resolve () is not MethodDefinition overriddenDefinition) { + // Assert.Fail ($"Method {linked.GetDisplayName ()} overrides method {overriddenMethod} which does not exist"); + // } + // else if (overriddenDefinition.DeclaringType.IsInterface) { + // Assert.True (linked.DeclaringType.Interfaces.Select(i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), + // $"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}"); + // } else { + // TypeReference? baseType = linked.DeclaringType!; + // TypeReference overriddenType = overriddenMethod.DeclaringType; + // while (baseType is not null) { + // if (baseType.Equals (overriddenType)) + // break; + // if (baseType.Resolve ()?.BaseType is null) + // Assert.Fail ($"Method {linked} overrides method {overriddenMethod} from, but {linked.DeclaringType} does not inherit from type {overriddenMethod.DeclaringType}"); + // } + // } + //} } static string FormatBaseOrInterfaceAttributeValue (CustomAttribute attr) From 733bf902ccdd69020685c2fa13c106b5ef8be778 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 12 May 2022 16:55:16 -0500 Subject: [PATCH 05/29] Update comment on override removal in sweep step --- src/linker/Linker.Steps/SweepStep.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index 6f53ee2ec054..b9aa1311a2fe 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -474,12 +474,16 @@ void SweepOverrides (MethodDefinition method) for (int i = 0; i < method.Overrides.Count;) { // We can't rely on the context resolution cache anymore, since it may remember methods which are already removed // So call the direct Resolve here and avoid the cache. - // It can still happen that the Resolve will return a removed method (this happens if the Overrides collection stores actual - // MethodDefinition and not MethodReference, in which case the Resolve is just "return this" and doesn't check the existence of the method - // in any way). In such case the Declaring type of the method is null. And since it has been removed the override pointing - // to it should be removed as well. - // Resolve() may also return null if the method has been removed, in which case the override should be removed from the list - if (method.Overrides[i].Resolve () is not MethodDefinition ov || ShouldRemove (ov) && (ov.DeclaringType == null || !IgnoreScope (ov.DeclaringType.Scope))) + // We want to remove a method from the list of Overrides if: + // Resolve() is null. This can happen for a couple of reasons, but it indicates the method isn't in the final assembly. + // OR + // ShouldRemove(ov) returns false + // AND ov.DeclaringType is not null + // AND ov.DeclaringType is in a `link` scope (i.e. !IgnoreScope). + // If ShouldRemove(ov) returns false we are removing the method from the assembly if it is a `link` assembly. + // ov.DeclaringType may be null if `ov` is a MethodDefinition already and just returns `this` even though it shouldn't exist. + // If the override is not in a `link` scope, ShouldRemove may return true even though the method will not be removed. + if (method.Overrides[i].Resolve () is not MethodDefinition ov || ShouldRemove (ov) && ov.DeclaringType is not null && !IgnoreScope (ov.DeclaringType.Scope)) method.Overrides.RemoveAt (i); else i++; @@ -487,7 +491,7 @@ void SweepOverrides (MethodDefinition method) } /// - /// Returns true if the assembly of the is not set to link (i.e. action=copy is set for that assembly) + /// Returns true if the assembly of the is not set to link (e.g. action=copy is set for that assembly) /// private bool IgnoreScope (IMetadataScope scope) { From 47104d9a3b0a5739ea35a330e6e9739b5c6693fe Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 12 May 2022 16:59:35 -0500 Subject: [PATCH 06/29] Add attributes --- .../Assertions/KeptOverrideAttribute.cs | 27 +++++++++++++++++++ .../Assertions/RemovedOverrideAttribute.cs | 25 +++++++++++++++++ .../TestCasesRunner/AssemblyChecker.cs | 2 +- 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs create mode 100644 test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs diff --git a/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs new file mode 100644 index 000000000000..62bfd0c15794 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs @@ -0,0 +1,27 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace Mono.Linker.Tests.Cases.Expectations.Assertions +{ + /// + /// Used to ensure that a method should keep an 'override' annotation for a method in the supplied base type. + /// The absence of this attribute does not enforce that the override is removed -- this is different from other Kept attributes + /// To enforce the removal of an override, use . + /// Fails in tests if the method doesn't have the override method in the original or linked assembly. + /// + /// + [AttributeUsage (AttributeTargets.Method, AllowMultiple = true, Inherited = false)] + public class KeptOverrideAttribute : KeptAttribute + { + public Type TypeWithOverriddenMethodDeclaration; + + public KeptOverrideAttribute (Type typeWithOverriddenMethod) + { + if (typeWithOverriddenMethod == null) + throw new ArgumentNullException (nameof (typeWithOverriddenMethod)); + TypeWithOverriddenMethodDeclaration = typeWithOverriddenMethod; + } + } +} \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs new file mode 100644 index 000000000000..c15cf9a9e414 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs @@ -0,0 +1,25 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace Mono.Linker.Tests.Cases.Expectations.Assertions +{ + /// + /// Used to ensure that a method should remove an 'override' annotation for a method in the supplied base type. + /// Fails in tests if the method has the override method in the linked assembly, + /// or if the override is not found in the original assembly + /// + /// + [AttributeUsage (AttributeTargets.Method, AllowMultiple = true, Inherited = false)] + public class RemovedOverrideAttribute : BaseInAssemblyAttribute + { + public Type TypeWithOverriddenMethodDeclaration; + public RemovedOverrideAttribute (Type typeWithOverriddenMethod) + { + if (typeWithOverriddenMethod == null) + throw new ArgumentException ("Value cannot be null or empty.", nameof (typeWithOverriddenMethod)); + TypeWithOverriddenMethodDeclaration = typeWithOverriddenMethod; + } + } +} \ No newline at end of file diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index b20e9526af31..607dc2b46a86 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -248,7 +248,7 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) Assert.True (linked.DeclaringType.Interfaces.Select(i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), $"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}"); } else { - TypeReference? baseType = linked.DeclaringType!; + TypeReference baseType = linked.DeclaringType; TypeReference overriddenType = overriddenMethod.DeclaringType; while (baseType is not null) { if (baseType.Equals (overriddenType)) From c2f4fd117eba0ba44a5f59763d9187164ee4cd8e Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 12 May 2022 17:03:16 -0500 Subject: [PATCH 07/29] formatting --- test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index 607dc2b46a86..e6a9a6012a82 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -243,9 +243,8 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) foreach (var overriddenMethod in linked.Overrides) { if (overriddenMethod.Resolve () is not MethodDefinition overriddenDefinition) { Assert.Fail ($"Method {linked.GetDisplayName ()} overrides method {overriddenMethod} which does not exist"); - } - else if (overriddenDefinition.DeclaringType.IsInterface) { - Assert.True (linked.DeclaringType.Interfaces.Select(i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), + } else if (overriddenDefinition.DeclaringType.IsInterface) { + Assert.True (linked.DeclaringType.Interfaces.Select (i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), $"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}"); } else { TypeReference baseType = linked.DeclaringType; From 3d577ce3162e4364bab86d9db8b98cb2f58b028b Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 12 May 2022 17:08:29 -0500 Subject: [PATCH 08/29] formatting --- .../StaticAbstractInterfaceMethods.cs | 32 +++++++++---------- .../StaticAbstractInterfaceMethodsLibrary.cs | 2 +- .../TestCasesRunner/AssemblyChecker.cs | 5 ++- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs index 2cf612134597..44904fafbb24 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -322,7 +322,7 @@ public static void Test () { UsesAllMethods.StaticMethodCalledOnConcreteType (); var x = new UsesAllMethods (); - ((IStaticAndInstanceMethods)x).InstanceMethod (); + ((IStaticAndInstanceMethods) x).InstanceMethod (); CallExplicitImplMethod (); } } @@ -430,19 +430,19 @@ public class ImplementsIInheritsFromBase : IInheritsFromBase public static int UsedOnConcreteType () => 0; [Kept] - [KeptOverride (typeof(IBase1))] + [KeptOverride (typeof (IBase1))] [RemovedOverride (typeof (IInheritsFromBase))] public static int UsedOnBaseOnlyConstrainedTypeImplicitImpl () => 0; [Kept] - [KeptOverride (typeof(IInheritsFromBase))] + [KeptOverride (typeof (IInheritsFromBase))] static int IInheritsFromBase.UsedOnConstrainedTypeExplicitImpl () => 0; [Kept] - [KeptOverride (typeof(IBase1))] + [KeptOverride (typeof (IBase1))] static int IBase1.UsedOnConstrainedTypeExplicitImpl () => 0; - public static int UnusedImplicitImpl () =>0; + public static int UnusedImplicitImpl () => 0; static int IBase1.UnusedExplicitImpl () => 0; @@ -471,24 +471,24 @@ public class ImplementsIInheritsFromTwoBases : IInheritsFromMultipleBases public static int UsedOnConcreteType () => 0; [Kept] - [KeptOverride (typeof(IBase1))] - [KeptOverride (typeof(IBase2))] + [KeptOverride (typeof (IBase1))] + [KeptOverride (typeof (IBase2))] [RemovedOverride (typeof (IInheritsFromMultipleBases))] public static int UsedOnBaseOnlyConstrainedTypeImplicitImpl () => 0; [Kept] - [KeptOverride (typeof(IBase1))] + [KeptOverride (typeof (IBase1))] static int IBase1.UsedOnConstrainedTypeExplicitImpl () => 0; [Kept] - [KeptOverride (typeof(IBase2))] + [KeptOverride (typeof (IBase2))] static int IBase2.UsedOnConstrainedTypeExplicitImpl () => 0; [Kept] - [KeptOverride (typeof(IInheritsFromMultipleBases))] + [KeptOverride (typeof (IInheritsFromMultipleBases))] static int IInheritsFromMultipleBases.UsedOnConstrainedTypeExplicitImpl () => 0; - public static int UnusedImplicitImpl () =>0; + public static int UnusedImplicitImpl () => 0; static int IBase1.UnusedExplicitImpl () => 0; @@ -509,27 +509,27 @@ public static void Test () } [Kept] - public static void CallBase1TypeConstrainedMethod () where T: IBase1 + public static void CallBase1TypeConstrainedMethod () where T : IBase1 { T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); T.UsedOnConstrainedTypeExplicitImpl (); } [Kept] - public static void CallBase2TypeConstrainedMethod () where T: IBase2 + public static void CallBase2TypeConstrainedMethod () where T : IBase2 { T.UsedOnBaseOnlyConstrainedTypeImplicitImpl (); T.UsedOnConstrainedTypeExplicitImpl (); } [Kept] - public static void CallSingleInheritTypeConstrainedMethod () where T: IInheritsFromBase + public static void CallSingleInheritTypeConstrainedMethod () where T : IInheritsFromBase { T.UsedOnConstrainedTypeExplicitImpl (); } [Kept] - public static void CallDoubleInheritTypeConstrainedMethod () where T: IInheritsFromMultipleBases + public static void CallDoubleInheritTypeConstrainedMethod () where T : IInheritsFromMultipleBases { T.UsedOnConstrainedTypeExplicitImpl (); } @@ -538,7 +538,7 @@ public static void CallDoubleInheritTypeConstrainedMethod () where T: IInheri public static void Test () { ImplementsIInheritsFromBase.Test (); - ImplementsIInheritsFromTwoBases.Test (); + ImplementsIInheritsFromTwoBases.Test (); } } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs index c931913671bb..863f6c2ef08f 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs @@ -875,7 +875,7 @@ internal interface IUnusedInterface [Kept] [KeptMember (".ctor()")] - [KeptInterface (typeof(IUnusedInterface))] + [KeptInterface (typeof (IUnusedInterface))] public class ImplementsUnusedInterface : IUnusedInterface { int IUnusedInterface.UnusedMethodExplicit () => 0; diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index f3e1c7448ba4..8e75f50983fd 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -244,9 +244,8 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) foreach (var overriddenMethod in linked.Overrides) { if (overriddenMethod.Resolve () is not MethodDefinition overriddenDefinition) { Assert.Fail ($"Method {linked.GetDisplayName ()} overrides method {overriddenMethod} which does not exist"); - } - else if (overriddenDefinition.DeclaringType.IsInterface) { - Assert.True (linked.DeclaringType.Interfaces.Select(i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), + } else if (overriddenDefinition.DeclaringType.IsInterface) { + Assert.True (linked.DeclaringType.Interfaces.Select (i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), $"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}"); } else { TypeReference? baseType = linked.DeclaringType!; From cccca4c14a8ab0c7adaeeffac960e2e66cc22d85 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 13 May 2022 11:19:16 -0500 Subject: [PATCH 09/29] Modify override removal logic and update comment --- src/linker/Linker.Steps/SweepStep.cs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index b9aa1311a2fe..9fb3ffd5bae8 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -475,15 +475,17 @@ void SweepOverrides (MethodDefinition method) // We can't rely on the context resolution cache anymore, since it may remember methods which are already removed // So call the direct Resolve here and avoid the cache. // We want to remove a method from the list of Overrides if: - // Resolve() is null. This can happen for a couple of reasons, but it indicates the method isn't in the final assembly. + // Resolve() is null + // This can happen for a couple of reasons, but it indicates the method isn't in the final assembly. + // Resolve also may return a removed value if method.Overrides[i] is a MethodDefinition. In this case, Resolve short circuits and returns `this`. // OR - // ShouldRemove(ov) returns false - // AND ov.DeclaringType is not null - // AND ov.DeclaringType is in a `link` scope (i.e. !IgnoreScope). - // If ShouldRemove(ov) returns false we are removing the method from the assembly if it is a `link` assembly. - // ov.DeclaringType may be null if `ov` is a MethodDefinition already and just returns `this` even though it shouldn't exist. - // If the override is not in a `link` scope, ShouldRemove may return true even though the method will not be removed. - if (method.Overrides[i].Resolve () is not MethodDefinition ov || ShouldRemove (ov) && ov.DeclaringType is not null && !IgnoreScope (ov.DeclaringType.Scope)) + // ov.DeclaringType is null + // ov.DeclaringType may be null if Resolve short circuited and returned a removed method. In this case, we want to remove the override. + // OR + // ov is in a `link` scope and is unmarked + // We need to make sure the override is in a link scope. + // Only things in a link scope are marked, so ShouldRemove is only valid for items in a `link` scope. + if (method.Overrides[i].Resolve () is not MethodDefinition ov || ov.DeclaringType is null || (IsLinkScope (ov.DeclaringType.Scope) && ShouldRemove (ov))) method.Overrides.RemoveAt (i); else i++; @@ -493,10 +495,10 @@ void SweepOverrides (MethodDefinition method) /// /// Returns true if the assembly of the is not set to link (e.g. action=copy is set for that assembly) /// - private bool IgnoreScope (IMetadataScope scope) + private bool IsLinkScope (IMetadataScope scope) { AssemblyDefinition? assembly = Context.Resolve (scope); - return assembly != null && Annotations.GetAction (assembly) != AssemblyAction.Link; + return assembly != null && Annotations.GetAction (assembly) == AssemblyAction.Link; } void SweepDebugInfo (Collection methods) From c380027550aa51aa7ce5b25e7baec56f85c8aa7e Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 13 May 2022 15:28:39 -0500 Subject: [PATCH 10/29] Mark override on explicit implementation --- src/linker/Linker.Steps/MarkStep.cs | 4 +++- src/linker/Linker.Steps/SweepStep.cs | 2 +- src/linker/Linker/Annotations.cs | 3 +++ .../StaticAbstractInterfaceMethods.cs | 5 +++-- test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs | 7 ++++--- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 8defdb3f9dcb..b2aa9fd7ce76 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -3055,7 +3055,9 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo // Calling the implementation method directly has no impact on the interface, and as such it should not mark the interface or its method. // Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round. if (Context.Resolve (ov)?.IsStatic == true - && Context.Resolve (ov.DeclaringType)?.IsInterface == true) { + && Context.Resolve (ov.DeclaringType)?.IsInterface == true + // Public methods can be called directly on the type. If it non-public it is an explicit implementation and if the method is marked we need to mark the override. + && method.IsPublic) { continue; } MarkMethod (ov, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin); diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index 9fb3ffd5bae8..38f986dc1ef3 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -493,7 +493,7 @@ void SweepOverrides (MethodDefinition method) } /// - /// Returns true if the assembly of the is not set to link (e.g. action=copy is set for that assembly) + /// Returns true if the assembly of the is set to link /// private bool IsLinkScope (IMetadataScope scope) { diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs index 022e45926dfb..301734033869 100644 --- a/src/linker/Linker/Annotations.cs +++ b/src/linker/Linker/Annotations.cs @@ -436,6 +436,9 @@ public bool IsPublic (IMetadataTokenProvider provider) return public_api.Contains (provider); } + /// + /// Returns an IEnumerable of the methods that override this method. Note this is different than , which returns the MethodImpl's + /// public IEnumerable? GetOverrides (MethodDefinition method) { return TypeMapInfo.GetOverrides (method); diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs index 44904fafbb24..46a06744ab37 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -328,8 +328,7 @@ public static void Test () } [Kept] - // Bug - //[KeptInterface (typeof (IStaticAndInstanceMethods))] + [KeptInterface (typeof (IStaticAndInstanceMethods))] public class UnusedMethods : IStaticAndInstanceMethods { public static int StaticMethodCalledOnConcreteType () => 0; @@ -338,6 +337,8 @@ public class UnusedMethods : IStaticAndInstanceMethods [KeptOverride (typeof (IStaticAndInstanceMethods))] static int IStaticAndInstanceMethods.StaticMethodExplicitImpl () => 0; + // Bug: If .ctor is removed, we can remove unused instance methods + [Kept] public int InstanceMethod () => 0; [Kept] diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index 9a73ac92eafb..c4f6e81515b2 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -245,15 +245,16 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) if (overriddenMethod.Resolve () is not MethodDefinition overriddenDefinition) { Assert.Fail ($"Method {linked.GetDisplayName ()} overrides method {overriddenMethod} which does not exist"); } else if (overriddenDefinition.DeclaringType.IsInterface) { - Assert.True (linked.DeclaringType.Interfaces.Select (i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), + Assert.True (linked.DeclaringType.Interfaces.Select (i => i.InterfaceType.FullName).Contains (overriddenMethod.DeclaringType.FullName), $"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}"); } else { TypeReference baseType = linked.DeclaringType; TypeReference overriddenType = overriddenMethod.DeclaringType; while (baseType is not null) { - if (baseType.Equals (overriddenType)) + if (baseType.FullName == overriddenType.FullName) break; - if (baseType.Resolve ()?.BaseType is null) + baseType = baseType.Resolve ()?.BaseType; + if (baseType is null) Assert.Fail ($"Method {linked} overrides method {overriddenMethod} from, but {linked.DeclaringType} does not inherit from type {overriddenMethod.DeclaringType}"); } } From 48130478cc973db6a17ccd36462d67a5cb737a6f Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 13 May 2022 15:43:49 -0500 Subject: [PATCH 11/29] Tweak override removal comment --- src/linker/Linker.Steps/SweepStep.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index 38f986dc1ef3..37a8f18c7746 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -483,7 +483,7 @@ void SweepOverrides (MethodDefinition method) // ov.DeclaringType may be null if Resolve short circuited and returned a removed method. In this case, we want to remove the override. // OR // ov is in a `link` scope and is unmarked - // We need to make sure the override is in a link scope. + // ShouldRemove returns true if the method is unmarked, but we also We need to make sure the override is in a link scope. // Only things in a link scope are marked, so ShouldRemove is only valid for items in a `link` scope. if (method.Overrides[i].Resolve () is not MethodDefinition ov || ov.DeclaringType is null || (IsLinkScope (ov.DeclaringType.Scope) && ShouldRemove (ov))) method.Overrides.RemoveAt (i); From 332e79f3534f6bd36e039b83e83345b812c3bd75 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 13 May 2022 17:20:54 -0500 Subject: [PATCH 12/29] wip --- .../StaticAbstractInterfaceMethods.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs index 46a06744ab37..9f87951d6271 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using System.Threading.Tasks; using Mono.Linker.Tests.Cases.Expectations.Assertions; @@ -25,6 +26,7 @@ public static void Main () GenericStaticInterface.Test (); RecursiveGenericInterface.Test (); UnusedInterfaces.Test (); + ClassInheritance.Test (); } [Kept] @@ -708,8 +710,57 @@ public static void Test () } } + [Kept] public class ClassInheritance { + [Kept] + public interface IBase + { + [Kept] + static abstract int ExplicitlyImplemented (); + static abstract int ImplicitlyImplementedUsedOnType (); + static abstract int ImplicitlyImplementedUsedOnInterface (); + int GetInt (); + } + + [Kept] + [KeptInterface (typeof (IBase))] + public abstract class BaseKeptOnType : IBase + { + [Kept] + [KeptOverride (typeof (IBase))] + static int IBase.ExplicitlyImplemented () => 0; + + // Don't use at all + public static int ImplicitlyImplementedUsedOnType () => 0; + + public static int ImplicitlyImplementedUsedOnInterface () => 0; + public int GetInt () => 0; + } + + [Kept] + [KeptInterface (typeof (IBase))] + [KeptBaseType (typeof (BaseKeptOnType))] + public class InheritsFromBase : BaseKeptOnType, IBase + { + // Use on this type only + [Kept] + //[RemovedOverride (typeof(IBase))] + public static int ImplictlyImplementedUsedOnType () => 0; + } + + [Kept] + public static void CallIBaseMethod () where T : IBase + { + T.ExplicitlyImplemented (); + } + + [Kept] + public static void Test () + { + InheritsFromBase.ImplictlyImplementedUsedOnType (); + CallIBaseMethod (); + } } } From 7f1064da13e7b957c0f84a1f8ff155c10b34b75d Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Wed, 25 May 2022 12:33:56 -0500 Subject: [PATCH 13/29] Choose correct merge conflict --- test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index 1c6f8c592144..c87c8144a17b 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -245,15 +245,16 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) if (overriddenMethod.Resolve () is not MethodDefinition overriddenDefinition) { Assert.Fail ($"Method {linked.GetDisplayName ()} overrides method {overriddenMethod} which does not exist"); } else if (overriddenDefinition.DeclaringType.IsInterface) { - Assert.True (linked.DeclaringType.Interfaces.Select (i => i.InterfaceType).Contains (overriddenMethod.DeclaringType), + Assert.True (linked.DeclaringType.Interfaces.Select (i => i.InterfaceType.FullName).Contains (overriddenMethod.DeclaringType.FullName), $"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}"); } else { TypeReference baseType = linked.DeclaringType; TypeReference overriddenType = overriddenMethod.DeclaringType; while (baseType is not null) { - if (baseType.Equals (overriddenType)) + if (baseType.FullName == overriddenType) break; - if (baseType.Resolve ()?.BaseType is null) + baseType = baseType.Resolve ()?.BaseType; + if (baseType is null) Assert.Fail ($"Method {linked} overrides method {overriddenMethod} from, but {linked.DeclaringType} does not inherit from type {overriddenMethod.DeclaringType}"); } } From 15db395ebfb85b62cbbd2f0d6355f453d32878f2 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 26 May 2022 11:52:43 -0500 Subject: [PATCH 14/29] use FullName in comparison --- test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index c87c8144a17b..f7e6df6627c3 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -251,7 +251,7 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked) TypeReference baseType = linked.DeclaringType; TypeReference overriddenType = overriddenMethod.DeclaringType; while (baseType is not null) { - if (baseType.FullName == overriddenType) + if (baseType.FullName == overriddenType.FullName) break; baseType = baseType.Resolve ()?.BaseType; if (baseType is null) From 175169727cf6175bdd9246645061b33bdbd934d0 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Wed, 1 Jun 2022 10:48:18 -0500 Subject: [PATCH 15/29] wip --- ...eritance.Interfaces.StaticInterfaceMethodsTests.cs} | 10 +++++----- .../SubstitutionsTests.g.cs | 6 ++++++ .../UnreachableBlockTests.g.cs | 6 ++++++ .../StaticAbstractInterfaceMethods.cs | 2 +- .../StaticAbstractInterfaceMethodsLibrary.cs | 1 - 5 files changed, 18 insertions(+), 7 deletions(-) rename test/ILLink.RoslynAnalyzer.Tests/{generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs => Inheritance.Interfaces.StaticInterfaceMethodsTests.cs} (58%) diff --git a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs b/test/ILLink.RoslynAnalyzer.Tests/Inheritance.Interfaces.StaticInterfaceMethodsTests.cs similarity index 58% rename from test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs rename to test/ILLink.RoslynAnalyzer.Tests/Inheritance.Interfaces.StaticInterfaceMethodsTests.cs index bac4335d6064..f1149a2f9f23 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/Inheritance.Interfaces.StaticInterfaceMethodsTests.cs @@ -1,4 +1,6 @@ -using System; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + using System.Threading.Tasks; using Xunit; @@ -6,20 +8,18 @@ namespace ILLink.RoslynAnalyzer.Tests.Inheritance.Interfaces { public sealed partial class StaticInterfaceMethodsTests : LinkerTestBase { - protected override string TestSuiteName => "Inheritance.Interfaces.StaticInterfaceMethods"; [Fact] public Task StaticAbstractInterfaceMethods () { - return RunTest (allowMissingWarnings: true); + return RunTest (nameof (StaticAbstractInterfaceMethods)); } [Fact] public Task StaticAbstractInterfaceMethodsLibrary () { - return RunTest (allowMissingWarnings: true); + return RunTest (nameof (StaticAbstractInterfaceMethodsLibrary)); } - } } \ No newline at end of file diff --git a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SubstitutionsTests.g.cs b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SubstitutionsTests.g.cs index 91298ba525e5..38e3afa9a0f9 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SubstitutionsTests.g.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SubstitutionsTests.g.cs @@ -87,6 +87,12 @@ public Task StubBodyUnsafe () return RunTest (allowMissingWarnings: true); } + [Fact] + public Task StubBodyWithStaticCtor () + { + return RunTest (allowMissingWarnings: true); + } + [Fact] public Task StubBodyWithValue () { diff --git a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBlockTests.g.cs b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBlockTests.g.cs index cfcc52b00a70..6a3716c594b1 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBlockTests.g.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBlockTests.g.cs @@ -55,6 +55,12 @@ public Task MultiStageRemoval () return RunTest (allowMissingWarnings: true); } + [Fact] + public Task ReplacedJumpTarget () + { + return RunTest (allowMissingWarnings: true); + } + [Fact] public Task ReplacedReturns () { diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs index 9f87951d6271..055610847b31 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -745,7 +745,7 @@ public class InheritsFromBase : BaseKeptOnType, IBase { // Use on this type only [Kept] - //[RemovedOverride (typeof(IBase))] + //[RemovedOverride (typeof (IBase))] public static int ImplictlyImplementedUsedOnType () => 0; } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs index 863f6c2ef08f..1e32ea5964e1 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs @@ -67,7 +67,6 @@ public class UnusedIUsedThroughConstrainedTypeMethods : IUsedThroughConstrainedT public static int UsedThroughConstrainedType () => 0; } - // Should this be kept? private class UnusedIUsedThroughConstrainedTypeMethodsPrivate : IUsedThroughConstrainedType, IUsedThroughConstrainedTypeInternal { public static int UsedThroughConstrainedType () => 0; From b10cd5f3ea857e04efb78adf7fa97b814254a694 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 1 Jun 2022 14:24:18 -0500 Subject: [PATCH 16/29] Update docs/removal-behavior.md Co-authored-by: Sven Boemer --- docs/removal-behavior.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/removal-behavior.md b/docs/removal-behavior.md index 1133c842ad2e..e4e53be10501 100644 --- a/docs/removal-behavior.md +++ b/docs/removal-behavior.md @@ -42,7 +42,7 @@ public class Program ### Method call on a constrained type parameter -On a call to a static abstract interface method that is accessed through a constrained type parameter, the interface method is rooted, as well as every implementation method on every type that is rooted. +On a call to a static abstract interface method that is accessed through a constrained type parameter, the interface method is kept, as is every implementation method on every kept type. Example: From e3fbf1235c5bbf7fd43374266d08074b1600e581 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Wed, 1 Jun 2022 17:31:33 -0500 Subject: [PATCH 17/29] Add explanation for workaround to remove interface implementation --- .../Inheritance.Interfaces/InterfaceVariants.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs index c7fb236a4497..7593efd6b330 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs @@ -92,7 +92,6 @@ public static void InterfaceAndMethodNoUsed () { } } [Kept] - [KeptInterface (typeof (IStaticInterfaceMethodUnused))] // Bug internal class InterfaceMethodUnused : IStaticInterfaceMethodUnused, IStaticInterfaceUnused { public static void InterfaceUsedMethodNot () { } @@ -100,15 +99,23 @@ public static void InterfaceUsedMethodNot () { } public static void InterfaceAndMethodNoUsed () { } } + [Kept] + // This method keeps InterfaceMethodUnused without making it 'relevant to variant casting' like + // doing a typeof or type argument would do. If the type is relevant to variant casting, + // we will keep all interface implementations for interfaces that are kept + internal static void KeepInterfaceMethodUnused (InterfaceMethodUnused x) { } + [Kept] public static void Test () { InterfaceMethodUsedThroughImplementation.InterfaceUsedMethodNot (); InterfaceMethodUsedThroughImplementation.InterfaceAndMethodNoUsed (); - Type t; - t = typeof (IStaticInterfaceMethodUnused); - t = typeof (InterfaceMethodUnused); + // The interface has to be kept this way, because if both the type and the interface may + // appear on the stack then they would be marked as relevant to variant casting and the + // interface implementation would be kept. + Type t = typeof (IStaticInterfaceMethodUnused); + KeepAnother (null); } } From 7467d82e674714efc047ca270e213cf118fd8aca Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 2 Jun 2022 11:45:28 -0500 Subject: [PATCH 18/29] wip --- src/linker/Linker.Steps/MarkStep.cs | 18 ++++++++++++------ .../InterfaceVariants.cs | 3 ++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index c421960e37f6..393000a69cc2 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -3028,16 +3028,22 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo // Calling the implementation method directly has no impact on the interface, and as such it should not mark the interface or its method. // Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round. if (Context.Resolve (ov)?.IsStatic == true - && Context.Resolve (ov.DeclaringType)?.IsInterface == true - // Public methods can be called directly on the type. If it non-public it is an explicit implementation and if the method is marked we need to mark the override. - && method.IsPublic) { + && Context.Resolve (ov.DeclaringType)?.IsInterface == true ) continue; - } MarkMethod (ov, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin); MarkExplicitInterfaceImplementation (method, ov); } } + if (method.IsStatic && method.DeclaringType.IsInterface) { + //var overridingMethods = Annotations.GetOverrides (method); + //if (overridingMethods is not null) { + // foreach (var overrideInfo in overridingMethods) { + _virtual_methods.Add ((method, ScopeStack.CurrentScope)); + //} + //} + } + MarkMethodSpecialCustomAttributes (method); if (method.IsVirtual) _virtual_methods.Add ((method, ScopeStack.CurrentScope)); @@ -3124,9 +3130,9 @@ protected virtual IEnumerable GetRequiredMethodsForInstantiate } } - void MarkExplicitInterfaceImplementation (MethodDefinition method, MethodReference ov) + void MarkExplicitInterfaceImplementation (MethodDefinition method, MethodReference overriddenMethod) { - if (Context.Resolve (ov) is not MethodDefinition resolvedOverride) + if (Context.Resolve (overriddenMethod) is not MethodDefinition resolvedOverride) return; if (resolvedOverride.DeclaringType.IsInterface) { diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs index 7593efd6b330..724552853bec 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs @@ -3,6 +3,7 @@ using System; using System.Collections; +using System.Runtime.CompilerServices; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Helpers; using Mono.Linker.Tests.Cases.Expectations.Metadata; @@ -115,7 +116,7 @@ public static void Test () // appear on the stack then they would be marked as relevant to variant casting and the // interface implementation would be kept. Type t = typeof (IStaticInterfaceMethodUnused); - KeepAnother (null); + KeepInterfaceMethodUnused (null); } } From 4efa5f233d340abf665b37149f9df9f07b070174 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 2 Jun 2022 16:07:37 -0500 Subject: [PATCH 19/29] Mark static interface implmentations in ProcessOverride --- src/linker/Linker.Steps/MarkStep.cs | 12 +++++------- src/linker/Linker/OverrideInformation.cs | 6 ++++++ .../Inheritance.Interfaces/InterfaceVariants.cs | 1 - .../StaticAbstractInterfaceMethods.cs | 4 ++-- .../StaticAbstractInterfaceMethodsLibrary.cs | 11 +++++------ 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 393000a69cc2..96ee0231add0 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -730,6 +730,9 @@ void ProcessOverride (OverrideInformation overrideInformation) MarkMethod (method, new DependencyInfo (DependencyKind.Override, @base), ScopeStack.CurrentScope.Origin); } + if (overrideInformation.IsStaticInterfaceMethodPair) + MarkExplicitInterfaceImplementation (method, @base); + if (method.IsVirtual) ProcessVirtualMethod (method); } @@ -740,7 +743,7 @@ bool IsInterfaceOverrideThatDoesNotNeedMarked (OverrideInformation overrideInfor return false; // This is a static interface method and these checks should all be true - if (overrideInformation.Override.IsStatic && overrideInformation.Base.IsStatic && overrideInformation.Base.IsAbstract && !overrideInformation.Override.IsVirtual) + if (overrideInformation.IsStaticInterfaceMethodPair) return false; if (overrideInformation.MatchingInterfaceImplementation != null) @@ -3036,12 +3039,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo } if (method.IsStatic && method.DeclaringType.IsInterface) { - //var overridingMethods = Annotations.GetOverrides (method); - //if (overridingMethods is not null) { - // foreach (var overrideInfo in overridingMethods) { _virtual_methods.Add ((method, ScopeStack.CurrentScope)); - //} - //} } MarkMethodSpecialCustomAttributes (method); @@ -3382,7 +3380,7 @@ void MarkInterfacesNeededByBodyStack (MethodBody body) { // If a type could be on the stack in the body and an interface it implements could be on the stack on the body // then we need to mark that interface implementation. When this occurs it is not safe to remove the interface implementation from the type - // even if the type is never instantiated + // even if the type is never instantiated. (ex. `Type1 x = null; IFoo y = (IFoo)x;`) var implementations = new InterfacesOnStackScanner (Context).GetReferencedInterfaces (body); if (implementations == null) return; diff --git a/src/linker/Linker/OverrideInformation.cs b/src/linker/Linker/OverrideInformation.cs index 00c06f5f005b..b1bc7f9f2c96 100644 --- a/src/linker/Linker/OverrideInformation.cs +++ b/src/linker/Linker/OverrideInformation.cs @@ -43,5 +43,11 @@ public TypeDefinition? InterfaceType { return Base.DeclaringType; } } + + public bool IsStaticInterfaceMethodPair { + get => IsOverrideOfInterfaceMember + && Base.IsAbstract && Base.IsStatic + && Override.IsStatic; + } } } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs index 724552853bec..1293c04732cd 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs @@ -3,7 +3,6 @@ using System; using System.Collections; -using System.Runtime.CompilerServices; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Helpers; using Mono.Linker.Tests.Cases.Expectations.Metadata; diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs index 055610847b31..c724faa51534 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -744,9 +744,9 @@ public abstract class BaseKeptOnType : IBase public class InheritsFromBase : BaseKeptOnType, IBase { // Use on this type only + // This doesn't override IBase.ImplicitlyImplementedUsedOnType [Kept] - //[RemovedOverride (typeof (IBase))] - public static int ImplictlyImplementedUsedOnType () => 0; + public new static int ImplictlyImplementedUsedOnType () => 0; } [Kept] diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs index 1e32ea5964e1..1e2c7fcf1daa 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethodsLibrary.cs @@ -868,8 +868,8 @@ public static class UnusedInterfaces [Kept] internal interface IUnusedInterface { - int UnusedMethodImplicit (); - int UnusedMethodExplicit (); + static abstract int UnusedMethodImplicit (); + static abstract int UnusedMethodExplicit (); } [Kept] @@ -877,12 +877,11 @@ internal interface IUnusedInterface [KeptInterface (typeof (IUnusedInterface))] public class ImplementsUnusedInterface : IUnusedInterface { - int IUnusedInterface.UnusedMethodExplicit () => 0; + static int IUnusedInterface.UnusedMethodExplicit () => 0; [Kept] - // Bug: We should be able to remove this override - //[RemovedOverride (typeof (IUnusedInterface))] - public int UnusedMethodImplicit () => 0; + [RemovedOverride (typeof (IUnusedInterface))] + public static int UnusedMethodImplicit () => 0; } [Kept] From c3a26caaff0a042037c557d12360465ee9f04690 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 2 Jun 2022 16:52:43 -0500 Subject: [PATCH 20/29] Format and update comment --- src/linker/Linker.Steps/MarkStep.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 96ee0231add0..6a0cab2eb2ca 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -3025,13 +3025,14 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo } // Mark overridden methods except for static interface methods + // This will not mark implicit interface methods because they do not have a MethodImpl and aren't in the .Overrides if (method.HasOverrides) { foreach (MethodReference ov in method.Overrides) { // Method implementing a static interface method will have an override to it - note nonstatic methods usually don't unless they're explicit. // Calling the implementation method directly has no impact on the interface, and as such it should not mark the interface or its method. // Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round. if (Context.Resolve (ov)?.IsStatic == true - && Context.Resolve (ov.DeclaringType)?.IsInterface == true ) + && Context.Resolve (ov.DeclaringType)?.IsInterface == true) continue; MarkMethod (ov, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin); MarkExplicitInterfaceImplementation (method, ov); From 62e3cc8399079397784a7100f3184ac173e6a820 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 3 Jun 2022 10:21:44 -0500 Subject: [PATCH 21/29] Use separate list for static interface methods --- src/linker/Linker.Steps/MarkStep.cs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 6a0cab2eb2ca..fe95b8efc886 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -60,6 +60,7 @@ protected LinkContext Context { protected Queue<(MethodDefinition, DependencyInfo, MessageOrigin)> _methods; protected List<(MethodDefinition, MarkScopeStack.Scope)> _virtual_methods; + protected List<(MethodDefinition, MarkScopeStack.Scope)> _static_interface_methods; protected Queue _assemblyLevelAttributes; readonly List _ivt_attributes; protected Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> _lateMarkedAttributes; @@ -219,6 +220,7 @@ public MarkStep () { _methods = new Queue<(MethodDefinition, DependencyInfo, MessageOrigin)> (); _virtual_methods = new List<(MethodDefinition, MarkScopeStack.Scope)> (); + _static_interface_methods = new List<(MethodDefinition, MarkScopeStack.Scope)> (); _assemblyLevelAttributes = new Queue (); _ivt_attributes = new List (); _lateMarkedAttributes = new Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> (); @@ -470,6 +472,7 @@ bool ProcessPrimaryQueue () while (!QueueIsEmpty ()) { ProcessQueue (); ProcessVirtualMethods (); + ProcessStaticInterfaceMethods (); ProcessMarkedTypesWithInterfaces (); ProcessDynamicCastableImplementationInterfaces (); ProcessPendingBodies (); @@ -575,9 +578,22 @@ void ProcessVirtualMethods () } } + void ProcessStaticInterfaceMethods () + { + foreach ((MethodDefinition method, MarkScopeStack.Scope scope) in _static_interface_methods) { + using (ScopeStack.PushScope (scope)) { + var overrides = Annotations.GetOverrides (method); + if (overrides != null) { + foreach (OverrideInformation @override in overrides) + ProcessOverride (@override); + } + } + } + } + /// /// Does extra handling of marked types that have interfaces when it's necessary to know what types are marked or instantiated. - /// e.g. Marks the "implements interface" annotations and removes override annotations for static interface methods. + /// Right now it only marks the "implements interface" annotations and removes override annotations for static interface methods. /// void ProcessMarkedTypesWithInterfaces () { @@ -3024,7 +3040,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo } } - // Mark overridden methods except for static interface methods + // Mark overridden methods and interface implementations except for static interface methods // This will not mark implicit interface methods because they do not have a MethodImpl and aren't in the .Overrides if (method.HasOverrides) { foreach (MethodReference ov in method.Overrides) { @@ -3039,14 +3055,13 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo } } - if (method.IsStatic && method.DeclaringType.IsInterface) { - _virtual_methods.Add ((method, ScopeStack.CurrentScope)); - } - MarkMethodSpecialCustomAttributes (method); if (method.IsVirtual) _virtual_methods.Add ((method, ScopeStack.CurrentScope)); + if (method.IsStatic && method.DeclaringType.IsInterface) + _static_interface_methods.Add ((method, ScopeStack.CurrentScope)); + MarkNewCodeDependencies (method); MarkBaseMethods (method); From 43597920c35515f855086b8f376fa3cf65344e7a Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 3 Jun 2022 14:01:24 -0500 Subject: [PATCH 22/29] Add clarifying comments --- src/linker/Linker.Steps/MarkStep.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index fe95b8efc886..45ecf7d7342f 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -578,6 +578,9 @@ void ProcessVirtualMethods () } } + /// + /// Handles marking implementations of static interface methods and the interface implementations of types that implement a static interface method. + /// void ProcessStaticInterfaceMethods () { foreach ((MethodDefinition method, MarkScopeStack.Scope scope) in _static_interface_methods) { @@ -712,6 +715,9 @@ void ProcessVirtualMethod (MethodDefinition method) } } + /// + /// Handles marking overriding methods if the type with the overriding method is instantiated or if the base method is a static abstract interface method + /// void ProcessOverride (OverrideInformation overrideInformation) { var method = overrideInformation.Override; @@ -727,14 +733,14 @@ void ProcessOverride (OverrideInformation overrideInformation) var isInstantiated = Annotations.IsInstantiated (method.DeclaringType); - // We don't need to mark overrides until it is possible that the type could be instantiated + // We don't need to mark overrides until it is possible that the type could be instantiated or the method is a static interface method // Note : The base type is interface check should be removed once we have base type sweeping if (IsInterfaceOverrideThatDoesNotNeedMarked (overrideInformation, isInstantiated)) return; - // Interface static veitual methods will be abstract and will also by pass this check to get marked - if (!isInstantiated && !@base.IsAbstract && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, method)) - return; + //// Interface static veitual methods will be abstract and will also by pass this check to get marked + //if (!isInstantiated && !@base.IsAbstract && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, method)) + // return; // Only track instantiations if override removal is enabled and the type is instantiated. // If it's disabled, all overrides are kept, so there's no instantiation site to blame. @@ -746,6 +752,8 @@ void ProcessOverride (OverrideInformation overrideInformation) MarkMethod (method, new DependencyInfo (DependencyKind.Override, @base), ScopeStack.CurrentScope.Origin); } + // We need to mark the interface implementation for static interface methods + // Explicit interface method implementations already mark the interface implementation in ProcessMethod if (overrideInformation.IsStaticInterfaceMethodPair) MarkExplicitInterfaceImplementation (method, @base); From 060345eeab6ce58aa5af33773b3012466fb87538 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 3 Jun 2022 15:49:41 -0500 Subject: [PATCH 23/29] Roll early exit case into function --- src/linker/Linker.Steps/MarkStep.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 45ecf7d7342f..3b93e22fabf4 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -738,10 +738,6 @@ void ProcessOverride (OverrideInformation overrideInformation) if (IsInterfaceOverrideThatDoesNotNeedMarked (overrideInformation, isInstantiated)) return; - //// Interface static veitual methods will be abstract and will also by pass this check to get marked - //if (!isInstantiated && !@base.IsAbstract && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, method)) - // return; - // Only track instantiations if override removal is enabled and the type is instantiated. // If it's disabled, all overrides are kept, so there's no instantiation site to blame. if (Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, method) && isInstantiated) { @@ -763,10 +759,14 @@ void ProcessOverride (OverrideInformation overrideInformation) bool IsInterfaceOverrideThatDoesNotNeedMarked (OverrideInformation overrideInformation, bool isInstantiated) { + // If it's a non-static interface method and the type isn't instantiated, we don't need to mark the override + if (!isInstantiated && !overrideInformation.Base.IsAbstract + && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, overrideInformation.Override)) + return true; + if (!overrideInformation.IsOverrideOfInterfaceMember || isInstantiated) return false; - // This is a static interface method and these checks should all be true if (overrideInformation.IsStaticInterfaceMethodPair) return false; From a050fdf57bb47756d875fa02ecf210cb89e5472d Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Tue, 7 Jun 2022 11:01:06 -0500 Subject: [PATCH 24/29] Move check back outside of interface override method --- src/linker/Linker.Steps/MarkStep.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 3b93e22fabf4..0fa4b8b5102c 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -736,6 +736,10 @@ void ProcessOverride (OverrideInformation overrideInformation) // We don't need to mark overrides until it is possible that the type could be instantiated or the method is a static interface method // Note : The base type is interface check should be removed once we have base type sweeping if (IsInterfaceOverrideThatDoesNotNeedMarked (overrideInformation, isInstantiated)) + return; + + // Interface static veitual methods will be abstract and will also by pass this check to get marked + if (!isInstantiated && !@base.IsAbstract && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, method)) return; // Only track instantiations if override removal is enabled and the type is instantiated. @@ -759,11 +763,6 @@ void ProcessOverride (OverrideInformation overrideInformation) bool IsInterfaceOverrideThatDoesNotNeedMarked (OverrideInformation overrideInformation, bool isInstantiated) { - // If it's a non-static interface method and the type isn't instantiated, we don't need to mark the override - if (!isInstantiated && !overrideInformation.Base.IsAbstract - && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, overrideInformation.Override)) - return true; - if (!overrideInformation.IsOverrideOfInterfaceMember || isInstantiated) return false; From 95df5415cc18b20ad7bc44ec8f4d733e434ad5f6 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Tue, 7 Jun 2022 11:55:20 -0500 Subject: [PATCH 25/29] formatting --- src/linker/Linker.Steps/MarkStep.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 0fa4b8b5102c..3c2db3f2cada 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -736,7 +736,7 @@ void ProcessOverride (OverrideInformation overrideInformation) // We don't need to mark overrides until it is possible that the type could be instantiated or the method is a static interface method // Note : The base type is interface check should be removed once we have base type sweeping if (IsInterfaceOverrideThatDoesNotNeedMarked (overrideInformation, isInstantiated)) - return; + return; // Interface static veitual methods will be abstract and will also by pass this check to get marked if (!isInstantiated && !@base.IsAbstract && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, method)) From 2b7a7cc166d6180a0482557056119bf6a3481a80 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Tue, 7 Jun 2022 16:22:19 -0500 Subject: [PATCH 26/29] Use Override pair to store logic for IsStaticInterfaceMethodPair --- src/linker/Linker.Steps/MarkStep.cs | 10 +++++----- src/linker/Linker/OverrideInformation.cs | 19 ++++++++++--------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 3c2db3f2cada..0c0ab78b2ea2 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -3050,15 +3050,15 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo // Mark overridden methods and interface implementations except for static interface methods // This will not mark implicit interface methods because they do not have a MethodImpl and aren't in the .Overrides if (method.HasOverrides) { - foreach (MethodReference ov in method.Overrides) { + foreach (MethodReference @base in method.Overrides) { // Method implementing a static interface method will have an override to it - note nonstatic methods usually don't unless they're explicit. // Calling the implementation method directly has no impact on the interface, and as such it should not mark the interface or its method. // Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round. - if (Context.Resolve (ov)?.IsStatic == true - && Context.Resolve (ov.DeclaringType)?.IsInterface == true) + if (Context.Resolve (@base) is MethodDefinition baseDefinition + && new OverrideInformation.OverridePair (baseDefinition, method).IsStaticInterfaceMethodPair ()) continue; - MarkMethod (ov, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin); - MarkExplicitInterfaceImplementation (method, ov); + MarkMethod (@base, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin); + MarkExplicitInterfaceImplementation (method, @base); } } diff --git a/src/linker/Linker/OverrideInformation.cs b/src/linker/Linker/OverrideInformation.cs index b1bc7f9f2c96..5034d1e465f2 100644 --- a/src/linker/Linker/OverrideInformation.cs +++ b/src/linker/Linker/OverrideInformation.cs @@ -10,17 +10,22 @@ namespace Mono.Linker public class OverrideInformation { readonly ITryResolveMetadata resolver; + readonly OverridePair _pair; public OverrideInformation (MethodDefinition @base, MethodDefinition @override, ITryResolveMetadata resolver, InterfaceImplementation? matchingInterfaceImplementation = null) { - Base = @base; - Override = @override; + _pair = new OverridePair (@base, @override); MatchingInterfaceImplementation = matchingInterfaceImplementation; this.resolver = resolver; } - public MethodDefinition Base { get; } - public MethodDefinition Override { get; } + public record struct OverridePair(MethodDefinition Base, MethodDefinition Override) + { + public bool IsStaticInterfaceMethodPair () => Base.DeclaringType.IsInterface && Base.IsStatic && Override.IsStatic; + } + + public MethodDefinition Base { get => _pair.Base; } + public MethodDefinition Override { get => _pair.Override; } public InterfaceImplementation? MatchingInterfaceImplementation { get; } public bool IsOverrideOfInterfaceMember { @@ -44,10 +49,6 @@ public TypeDefinition? InterfaceType { } } - public bool IsStaticInterfaceMethodPair { - get => IsOverrideOfInterfaceMember - && Base.IsAbstract && Base.IsStatic - && Override.IsStatic; - } + public bool IsStaticInterfaceMethodPair => _pair.IsStaticInterfaceMethodPair (); } } From c00e1d1b1dc0fbdbb4a53b4a6943596bceec50b0 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Wed, 8 Jun 2022 12:07:24 -0500 Subject: [PATCH 27/29] Formatting --- src/linker/Linker/OverrideInformation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linker/Linker/OverrideInformation.cs b/src/linker/Linker/OverrideInformation.cs index 5034d1e465f2..c85aecd6b207 100644 --- a/src/linker/Linker/OverrideInformation.cs +++ b/src/linker/Linker/OverrideInformation.cs @@ -19,7 +19,7 @@ public OverrideInformation (MethodDefinition @base, MethodDefinition @override, this.resolver = resolver; } - public record struct OverridePair(MethodDefinition Base, MethodDefinition Override) + public record struct OverridePair (MethodDefinition Base, MethodDefinition Override) { public bool IsStaticInterfaceMethodPair () => Base.DeclaringType.IsInterface && Base.IsStatic && Override.IsStatic; } From 358b01075d724c34d4f5d18228a0cdf45ef5fb39 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Mon, 13 Jun 2022 16:10:16 -0400 Subject: [PATCH 28/29] Move interfaceImpl marking out of ProcessOverride --- src/linker/Linker.Steps/MarkStep.cs | 11 ++--- .../StaticAbstractInterfaceMethods.cs | 49 ++++++++++++++----- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 0c0ab78b2ea2..c422a4c7bfcb 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -587,8 +587,12 @@ void ProcessStaticInterfaceMethods () using (ScopeStack.PushScope (scope)) { var overrides = Annotations.GetOverrides (method); if (overrides != null) { - foreach (OverrideInformation @override in overrides) + foreach (OverrideInformation @override in overrides) { ProcessOverride (@override); + // We need to mark the interface implementation for static interface methods + // Explicit interface method implementations already mark the interface implementation in ProcessMethod + MarkExplicitInterfaceImplementation (@override.Override, @override.Base); + } } } } @@ -752,11 +756,6 @@ void ProcessOverride (OverrideInformation overrideInformation) MarkMethod (method, new DependencyInfo (DependencyKind.Override, @base), ScopeStack.CurrentScope.Origin); } - // We need to mark the interface implementation for static interface methods - // Explicit interface method implementations already mark the interface implementation in ProcessMethod - if (overrideInformation.IsStaticInterfaceMethodPair) - MarkExplicitInterfaceImplementation (method, @base); - if (method.IsVirtual) ProcessVirtualMethod (method); } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs index c724faa51534..463adfc510e2 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/StaticAbstractInterfaceMethods.cs @@ -27,6 +27,7 @@ public static void Main () RecursiveGenericInterface.Test (); UnusedInterfaces.Test (); ClassInheritance.Test (); + ProcessOverrideAfterMarkedBase.Test (); } [Kept] @@ -376,13 +377,13 @@ public interface IBase1 [KeptInterface (typeof (IBase1))] public interface IInheritsFromBase : IBase1 { - public static abstract int UsedOnConcreteType (); - public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + public static new abstract int UsedOnConcreteType (); + public static new abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); [Kept] - public static abstract int UsedOnConstrainedTypeExplicitImpl (); - public static abstract int UnusedImplicitImpl (); - public static abstract int UnusedExplicitImpl (); + public static new abstract int UsedOnConstrainedTypeExplicitImpl (); + public static new abstract int UnusedImplicitImpl (); + public static new abstract int UnusedExplicitImpl (); } [Kept] @@ -404,13 +405,13 @@ public interface IBase2 [KeptInterface (typeof (IBase2))] public interface IInheritsFromMultipleBases : IBase1, IBase2, IUnusedInterface { - public static abstract int UsedOnConcreteType (); - public static abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); + public static new abstract int UsedOnConcreteType (); + public static new abstract int UsedOnBaseOnlyConstrainedTypeImplicitImpl (); [Kept] - public static abstract int UsedOnConstrainedTypeExplicitImpl (); - public static abstract int UnusedImplicitImpl (); - public static abstract int UnusedExplicitImpl (); + public static new abstract int UsedOnConstrainedTypeExplicitImpl (); + public static new abstract int UnusedImplicitImpl (); + public static new abstract int UnusedExplicitImpl (); } public interface IUnusedInterface @@ -746,7 +747,7 @@ public class InheritsFromBase : BaseKeptOnType, IBase // Use on this type only // This doesn't override IBase.ImplicitlyImplementedUsedOnType [Kept] - public new static int ImplictlyImplementedUsedOnType () => 0; + public static int ImplictlyImplementedUsedOnType () => 0; } [Kept] @@ -761,7 +762,33 @@ public static void Test () InheritsFromBase.ImplictlyImplementedUsedOnType (); CallIBaseMethod (); } + } + [Kept] + public static class ProcessOverrideAfterMarkedBase + { + [Kept] + interface IFoo + { + [Kept] + static abstract int Method (); + } + + [Kept] + [KeptInterface (typeof (IFoo))] + class Foo : IFoo + { + [Kept] + [KeptOverride (typeof (IFoo))] + public static int Method () => 0; + } + + [Kept] + public static void Test () + { + typeof (Foo).RequiresPublicMethods (); + typeof (IFoo).RequiresPublicMethods (); + } } } } From 432375d24714689c02f0f2595ea646479ab01baf Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Tue, 14 Jun 2022 11:19:59 -0400 Subject: [PATCH 29/29] PR Feedback --- src/linker/Linker.Steps/MarkStep.cs | 4 ++-- src/linker/Linker/OverrideInformation.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 748ee49f058b..751b944b9ff6 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -743,7 +743,7 @@ void ProcessOverride (OverrideInformation overrideInformation) if (IsInterfaceOverrideThatDoesNotNeedMarked (overrideInformation, isInstantiated)) return; - // Interface static veitual methods will be abstract and will also by pass this check to get marked + // Interface static virtual methods will be abstract and will also bypass this check to get marked if (!isInstantiated && !@base.IsAbstract && Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, method)) return; @@ -3102,7 +3102,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo if (method.IsVirtual) _virtual_methods.Add ((method, ScopeStack.CurrentScope)); - if (method.IsStatic && method.DeclaringType.IsInterface) + if (method.IsStatic && method.IsAbstract && method.DeclaringType.IsInterface) _static_interface_methods.Add ((method, ScopeStack.CurrentScope)); MarkNewCodeDependencies (method); diff --git a/src/linker/Linker/OverrideInformation.cs b/src/linker/Linker/OverrideInformation.cs index c85aecd6b207..10bca183cb3e 100644 --- a/src/linker/Linker/OverrideInformation.cs +++ b/src/linker/Linker/OverrideInformation.cs @@ -19,7 +19,7 @@ public OverrideInformation (MethodDefinition @base, MethodDefinition @override, this.resolver = resolver; } - public record struct OverridePair (MethodDefinition Base, MethodDefinition Override) + public readonly record struct OverridePair (MethodDefinition Base, MethodDefinition Override) { public bool IsStaticInterfaceMethodPair () => Base.DeclaringType.IsInterface && Base.IsStatic && Override.IsStatic; }