Trim static interfaces

src/linker/Linker.Steps/MarkStep.cs

@@ -573,6 +573,10 @@ void ProcessVirtualMethods ()
 			}
 		}
 
+		/// <summary>
+		/// 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.
+		/// </summary>
 		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
@@ -582,6 +586,9 @@ void ProcessMarkedTypesWithInterfaces ()
 			var typesWithInterfaces = _typesWithInterfaces.ToArray ();
 
 			foreach ((var type, var scope) in typesWithInterfaces) {
+				// Regardless of the optimizations or whether or not the type is instantiated, we should
+				//	remove unneeded override annotations for static interface method implementations
+				RemoveStaticInterfaceOverrideAnnotations (type);
 				// Exception, types that have not been flagged as instantiated yet.  These types may not need their interfaces even if the
 				// interface type is marked
 				// UnusedInterfaces optimization is turned off mark all interface implementations
@@ -2265,6 +2272,26 @@ void MarkInterfaceImplementations (TypeDefinition type)
 			}
 		}
 
+		/// <summary>
+		/// Removes the 'override' annotation for implementations of static interface methods if the interface method is not marked.
+		/// </summary>
+		void RemoveStaticInterfaceOverrideAnnotations (TypeDefinition type)
+		{
+			foreach (var method in type.Methods) {
+				if (!Annotations.IsMarked (method))
+					continue;
+				// Modify overrides in place
+				for (int i = 0; i < method.Overrides.Count;) {
+					if (Context.Resolve (method.Overrides[i].DeclaringType)?.IsInterface == true
+						&& Context.Resolve (method.Overrides[i])?.IsStatic == true
+						&& !Annotations.IsMarked (method.Overrides[i]))
+						method.Overrides.RemoveAt (i);
+					else
+						i++;
+				}
+			}
+		}
+
 		void MarkGenericParameterProvider (IGenericParameterProvider provider)
 		{
 			if (!provider.HasGenericParameters)
@@ -3049,8 +3076,15 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
 				}
 			}
 
+			// Mark overrides except for static interface methods
 			if (method.HasOverrides) {
 				foreach (MethodReference ov in method.Overrides) {
+					// Don't mark overrides for static interface methods.
+					// Since they can only be called on a concrete type and not the interface, these methods can safely be removed in some cases
+					if (Context.Resolve (ov)?.IsStatic == true
+						&& Context.Resolve (ov.DeclaringType)?.IsInterface == true) {
+						continue;
+					}
 					MarkMethod (ov, new DependencyInfo (DependencyKind.MethodImplOverride, method));
 					MarkExplicitInterfaceImplementation (method, ov);
 				}

test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBlockTests.g.cs

@@ -37,6 +37,12 @@ public Task InstanceMethodSubstitutions ()
 			return RunTest (allowMissingWarnings: true);
 		}
 
+		[Fact]
+		public Task MethodArgumentPropagation ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
 		[Fact]
 		public Task MethodWithParametersSubstitutions ()
 		{
@@ -55,6 +61,12 @@ public Task ReplacedReturns ()
 			return RunTest (allowMissingWarnings: true);
 		}
 
+		[Fact]
+		public Task ResultInliningNotPossible ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
 		[Fact]
 		public Task SimpleConditionalProperty ()
 		{

test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs

@@ -0,0 +1,24 @@
+// 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
+{
+	[AttributeUsage (AttributeTargets.Method, AllowMultiple = true, Inherited = false)]
+	/// <Summary>
+	/// Used to ensure that a method should keep an 'override' annotation for a method in the supplied base type
+	/// Fails in tests if the method doesn't have the override method in the original or linked assembly
+	/// </Summary>
+	public class KeptOverrideAttribute : KeptAttribute
+	{
+		public Type TypeWithOverriddenMethodDeclaration;
+
+		public KeptOverrideAttribute (Type typeWithOverriddenMethod)
+		{
+			if (typeWithOverriddenMethod == null)
+				throw new ArgumentNullException (nameof (typeWithOverriddenMethod));
+			TypeWithOverriddenMethodDeclaration = typeWithOverriddenMethod;
+		}
+	}
+}

test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs

@@ -0,0 +1,24 @@
+// 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
+{
+	/// <Summary>
+	/// 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
+	/// </Summary>
+	[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;
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs

@@ -20,29 +20,63 @@ public static void Main ()
 			t = typeof (UninstantiatedPublicClassWithImplicitlyImplementedInterface);
 			t = typeof (UninstantiatedPublicClassWithPrivateInterface);
 
-			UninstantiatedPublicClassWithInterface.InternalStaticInterfaceMethodUsed ();
-			InstantiatedClassWithInterfaces.InternalStaticInterfaceMethodUsed ();
-
+			UninstantiatedPublicClassWithInterface.InternalStaticInterfaceMethodUsedThroughImplementation ();
+			InstantiatedClassWithInterfaces.InternalStaticInterfaceMethodUsedThroughImplementation ();
+			GenericMethodThatCallsInternalStaticInterfaceMethod
+				<ImplementsInternalStaticInterfaceWithInterfaceDefinitionUsedThroughGeneric> ();
+			typeof (InterfaceVariants).GetMethod ("GenericMethodThatCallsInternalStaticInterfaceMethod")
+				.MakeGenericMethod (typeof (ImplementsInternalStaticInterfaceWithInterfaceDefinitionNotUsedThroughGeneric))
+				.Invoke (null, null);
 			// Use all public interfaces - they're marked as public only to denote them as "used"
 			typeof (IPublicInterface).RequiresPublicMethods ();
 			typeof (IPublicStaticInterface).RequiresPublicMethods ();
 
 			var a = new InstantiatedClassWithInterfaces ();
 		}
 
+		[Kept]
+		internal static void GenericMethodThatCallsInternalStaticInterfaceMethod<T> () where T : IInternalStaticInterfaceWithInterfaceDefinitionUsed
+		{
+			T.InternalStaticInterfaceUsedThroughInterface ();
+		}
+
+		[Kept]
+		[KeptInterface (typeof (IInternalStaticInterfaceWithInterfaceDefinitionUsed))]
+		internal class ImplementsInternalStaticInterfaceWithInterfaceDefinitionUsedThroughGeneric : IInternalStaticInterfaceWithInterfaceDefinitionUsed
+		{
+			[Kept]
+			[KeptOverride (typeof (IInternalStaticInterfaceWithInterfaceDefinitionUsed))]
+			public static void InternalStaticInterfaceUsedThroughInterface ()
+			{
+			}
+			public static void UnusedMethod () { }
+		}
+
+		[Kept]
+		[KeptInterface (typeof (IInternalStaticInterfaceWithInterfaceDefinitionUsed))]
+		internal class ImplementsInternalStaticInterfaceWithInterfaceDefinitionNotUsedThroughGeneric : IInternalStaticInterfaceWithInterfaceDefinitionUsed
+		{
+			[Kept]
+			[KeptOverride (typeof (IInternalStaticInterfaceWithInterfaceDefinitionUsed))]
+			public static void InternalStaticInterfaceUsedThroughInterface ()
+			{
+			}
+
+			public static void UnusedMethod () { }
+		}
+
 		[Kept]
 		[KeptInterface (typeof (IEnumerator))]
 		[KeptInterface (typeof (IPublicInterface))]
 		[KeptInterface (typeof (IPublicStaticInterface))]
-		[KeptInterface (typeof (IInternalStaticInterfaceWithUsedMethod))] // https://github.com/dotnet/linker/issues/2733
 		[KeptInterface (typeof (ICopyLibraryInterface))]
 		[KeptInterface (typeof (ICopyLibraryStaticInterface))]
 		public class UninstantiatedPublicClassWithInterface :
 			IPublicInterface,
 			IPublicStaticInterface,
 			IInternalInterface,
 			IInternalStaticInterface,
-			IInternalStaticInterfaceWithUsedMethod,
+			IInternalStaticInterfaceWithUsedMethodImplementation,
 			IEnumerator,
 			ICopyLibraryInterface,
 			ICopyLibraryStaticInterface
@@ -70,7 +104,8 @@ public static void InternalStaticInterfaceMethod () { }
 			static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { }
 
 			[Kept]
-			public static void InternalStaticInterfaceMethodUsed () { }
+			[RemovedOverride (typeof (IInternalStaticInterfaceWithUsedMethodImplementation))]
+			public static void InternalStaticInterfaceMethodUsedThroughImplementation () { }
 
 			[Kept]
 			[ExpectBodyModified]
@@ -122,15 +157,14 @@ public string ToString (string format, IFormatProvider formatProvider)
 		[KeptInterface (typeof (IEnumerator))]
 		[KeptInterface (typeof (IPublicInterface))]
 		[KeptInterface (typeof (IPublicStaticInterface))]
-		[KeptInterface (typeof (IInternalStaticInterfaceWithUsedMethod))] // https://github.com/dotnet/linker/issues/2733
 		[KeptInterface (typeof (ICopyLibraryInterface))]
 		[KeptInterface (typeof (ICopyLibraryStaticInterface))]
 		public class InstantiatedClassWithInterfaces :
 			IPublicInterface,
 			IPublicStaticInterface,
 			IInternalInterface,
 			IInternalStaticInterface,
-			IInternalStaticInterfaceWithUsedMethod,
+			IInternalStaticInterfaceWithUsedMethodImplementation,
 			IEnumerator,
 			ICopyLibraryInterface,
 			ICopyLibraryStaticInterface
@@ -159,7 +193,8 @@ public static void InternalStaticInterfaceMethod () { }
 			static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { }
 
 			[Kept]
-			public static void InternalStaticInterfaceMethodUsed () { }
+			[RemovedOverride (typeof (IInternalStaticInterfaceWithUsedMethodImplementation))]
+			public static void InternalStaticInterfaceMethodUsedThroughImplementation () { }
 
 			[Kept]
 			bool IEnumerator.MoveNext () { throw new PlatformNotSupportedException (); }
@@ -225,13 +260,20 @@ internal interface IInternalStaticInterface
 			static abstract void ExplicitImplementationInternalStaticInterfaceMethod ();
 		}
 
-		// The interface methods themselves are not used, but the implentation of these methods is
-		// https://github.com/dotnet/linker/issues/2733
+		// The interface methods themselves are not used, but the implementation of these methods is
+		internal interface IInternalStaticInterfaceWithUsedMethodImplementation
+		{
+			static abstract void InternalStaticInterfaceMethodUsedThroughImplementation ();
+		}
+
+		// The interface methods themselves are used through the interface
 		[Kept]
-		internal interface IInternalStaticInterfaceWithUsedMethod
+		internal interface IInternalStaticInterfaceWithInterfaceDefinitionUsed
 		{
-			[Kept] // https://github.com/dotnet/linker/issues/2733
-			static abstract void InternalStaticInterfaceMethodUsed ();
+			[Kept]
+			static abstract void InternalStaticInterfaceUsedThroughInterface ();
+
+			static abstract void UnusedMethod ();
 		}
 
 		private interface IPrivateInterface

test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs

@@ -217,6 +217,7 @@ public void InternalInterfaceMethod () { }
 			void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { }
 
 			[Kept]
+			[RemovedOverride (typeof (IInternalStaticInterface))]
 			public static void InternalStaticInterfaceMethod () { }
 
 			static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { }
@@ -300,6 +301,7 @@ public void InternalInterfaceMethod () { }
 			void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { }
 
 			[Kept]
+			[RemovedOverride (typeof (IInternalStaticInterface))]
 			public static void InternalStaticInterfaceMethod () { }
 
 			static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { }
@@ -366,7 +368,6 @@ internal interface IInternalInterface
 		[Kept]
 		internal interface IInternalStaticInterface
 		{
-			[Kept] // https://github.com/dotnet/linker/issues/2733
 			static abstract void InternalStaticInterfaceMethod ();
 
 			static abstract void ExplicitImplementationInternalStaticInterfaceMethod ();

test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs

@@ -176,7 +176,9 @@ protected virtual void VerifyTypeDefinitionKept (TypeDefinition original, TypeDe
 				if (verifiedEventMethods.Contains (m.FullName))
 					continue;
 				var msign = m.GetSignature ();
-				VerifyMethod (m, linked?.Methods.FirstOrDefault (l => msign == l.GetSignature ()));
+				var linkedMethod = linked?.Methods.FirstOrDefault (l => msign == l.GetSignature ());
+				VerifyMethod (m, linkedMethod);
+				VerifyOverrides (m, linkedMethod);
 				linkedMembers.Remove (m.FullName);
 			}
 		}
@@ -212,6 +214,35 @@ void VerifyInterfaces (TypeDefinition src, TypeDefinition linked)
 			}
 		}
 
+		void VerifyOverrides (MethodDefinition original, MethodDefinition linked)
+		{
+			if (linked is null)
+				return;
+			var expectedBaseTypesOverridden = new HashSet<string> (original.CustomAttributes
+				.Where (ca => ca.AttributeType.Name == nameof (KeptOverrideAttribute))
+				.Select (ca => (ca.ConstructorArguments[0].Value as TypeDefinition).FullName));
+			var originalBaseTypesOverridden = new HashSet<string> (original.Overrides.Select (ov => ov.DeclaringType.FullName));
+			var linkedBaseTypesOverridden = new HashSet<string> (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<string> (original.CustomAttributes
+				.Where (ca => ca.AttributeType.Name == nameof (RemovedOverrideAttribute))
+				.Select (ca => (ca.ConstructorArguments[0].Value as TypeDefinition).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)