ILLink: Allow interface methods to not have newslot (dotnet#93662)

Fixes dotnet#93008

This comes from F#, where interface method defined in F# doesn't have `newslot` flag in its metadata. Trimmer gets confused and doesn't recognize such method as a "base" method for its implementation in the type which implements the interface. As a result, the implementation method is removed from the program because there doesn't seem to be a use for it anywhere.

The fix is to basically ignore the `newslot` flag for interface instance methods. The existing behavior must be kept for static interface methods though - as a redefinition of a method with the `new` C# keyword will produce an interface method definition without the `newslot` flag as well.

The change adds a new test and modifies the AOT/analyzer infra to run the test as well.

src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestDatabase.cs

@@ -34,6 +34,11 @@ public static IEnumerable<object[]> InlineArrays ()
 			return TestNamesBySuiteName();
 		}
 
+        public static IEnumerable<object[]> Inheritance_Interaces()
+        {
+            return TestNamesBySuiteName("Inheritance.Interfaces");
+        }
+
         public static IEnumerable<object[]> Libraries()
         {
             return TestNamesBySuiteName();

src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestSuites.cs

@@ -37,6 +37,20 @@ public void InlineArrays(string t)
 			Run(t);
 		}
 
+        [Theory]
+        [MemberData(nameof(TestDatabase.Inheritance_Interaces), MemberType = typeof(TestDatabase))]
+        public void Inheritance_Interfaces(string t)
+        {
+            switch (t) {
+            case ".InterfaceWithoutNewSlot":
+                Run (t);
+                break;
+            default:
+                // Skip the rest for now
+                break;
+            }
+        }
+
         [Theory]
         [MemberData(nameof(TestDatabase.Libraries), MemberType = typeof(TestDatabase))]
         public void Libraries(string t)

src/tools/illink/src/linker/Linker/TypeMapInfo.cs

@@ -149,9 +149,12 @@ void MapInterfaceMethodsInTypeHierarchy (TypeDefinition type)
 					// we shouldn't need to run the below logic. This results in ILLink potentially
 					// keeping more methods than needed.
 
+					// Static methods on interfaces must be implemented only via explicit method-impl record
+					// not by a signature match. So there's no point in running the logic below for static methods.
+
 					if (!resolvedInterfaceMethod.IsVirtual
 						|| resolvedInterfaceMethod.IsFinal
-						|| !resolvedInterfaceMethod.IsNewSlot)
+						|| resolvedInterfaceMethod.IsStatic)
 						continue;
 
 					// Try to find an implementation with a name/sig match on the current type

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.InterfacesTests.g.cs

@@ -27,5 +27,11 @@ public Task InterfaceVariants ()
 			return RunTest (allowMissingWarnings: true);
 		}
 
+		[Fact]
+		public Task InterfaceWithoutNewSlot ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
 	}
 }

src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/Dependencies/InterfaceWithoutNewSlot.il

@@ -0,0 +1,53 @@
+// 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.
+
+.assembly extern mscorlib { }
+
+.assembly 'library' { }
+
+.class interface public auto ansi abstract IInterfaceWithoutNewSlot
+{
+	.method public hidebysig abstract virtual 
+		instance void AbstractNoNewSlot () cil managed 
+	{
+	}
+
+	.method public hidebysig abstract virtual 
+		instance void AbstractNoNewSlotUnused () cil managed 
+	{
+	}
+
+	.method public hidebysig newslot abstract virtual 
+		instance void AbstractNewSlot () cil managed 
+	{
+	}
+
+	.method public hidebysig newslot abstract virtual 
+		instance void AbstractNewSlotUnused () cil managed 
+	{
+	}
+
+	.method public hidebysig virtual 
+		instance void ImplementedNoNewSlot () cil managed 
+	{
+		ret;
+	}
+
+	.method public hidebysig virtual 
+		instance void ImplementedNoNewSlotUnused () cil managed 
+	{
+		ret;
+	}
+
+	.method public hidebysig newslot virtual 
+		instance void ImplementedNewSlot () cil managed 
+	{
+		ret;
+	}
+
+	.method public hidebysig newslot virtual 
+		instance void ImplementedNewSlotUnused () cil managed 
+	{
+		ret;
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceWithoutNewSlot.cs

@@ -0,0 +1,79 @@
+// 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.Metadata;
+
+namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces
+{
+	// Tests the case where the interface method doesn't have 'newslot' flag set
+	[TestCaseRequirements (TestRunCharacteristics.SupportsDefaultInterfaceMethods, "Requires support for default interface methods")]
+	[Define ("IL_ASSEMBLY_AVAILABLE")]
+	[SetupCompileBefore ("library.dll", new[] { "Dependencies/InterfaceWithoutNewSlot.il" })]
+	class InterfaceWithoutNewSlot
+	{
+		public static void Main ()
+		{
+			ImplementationType.Test ();
+		}
+
+#if !IL_ASSEMBLY_AVAILABLE
+		// Declaration for the build of the test suite
+		// When compiled for actual run the IL version is used instead
+		// This is because there's no way to express the newslot/no-newslot in C#
+		interface IInterfaceWithoutNewSlot
+		{
+			void AbstractNoNewSlot ();
+			void AbstractNoNewSlotUnused ();
+			void AbstractNewSlot ();
+			void AbstractNewSlotUnused ();
+			void ImplementedNoNewSlot () { }
+			void ImplementedNoNewSlotUnused () { }
+			void ImplementedNewSlot () { }
+			void ImplementedNewSlotUnused () { }
+		}
+#endif
+
+		[Kept]
+		[KeptInterface (typeof (IInterfaceWithoutNewSlot))]
+		[KeptMember (".ctor()")]
+		class ImplementationType : IInterfaceWithoutNewSlot
+		{
+			[Kept]
+			public void AbstractNoNewSlot () { }
+
+			public void AbstractNoNewSlotUnused () { }
+
+			[Kept]
+			public void AbstractNewSlot () { }
+
+			public void AbstractNewSlotUnused () { }
+
+			// This is not considered an implementation of the interface method
+			// CoreCLR doesn't treat it that way either.
+			public void ImplementedNoNewSlot () { }
+
+			public void ImplementedNoNewSlotUnused () { }
+
+			[Kept]
+			public void ImplementedNewSlot () { }
+
+			public void ImplementedNewSlotUnused () { }
+
+			[Kept]
+			public static void Test ()
+			{
+				IInterfaceWithoutNewSlot instance = new ImplementationType ();
+				instance.AbstractNoNewSlot ();
+				instance.AbstractNewSlot ();
+				instance.ImplementedNoNewSlot ();
+				instance.ImplementedNewSlot ();
+			}
+		}
+	}
+}