[ILLink] Add interface implementing type to OverrideInformation (#98274)

Adds the type that implements the interface to the OverrideInformation class to be able to properly handle cases where the base provides the interface method or the type relies on a default interface method. Adds the InterfaceImplementor class that is basically a tuple of (TypeDefinition typeWithInterface, InterfaceImplementation ImplementedInterface) to do this. Also cleans some unnecessary code in OverrideInformation and ensures that OverrideInformation will have an InterfaceImplementor if the base method is an interface method.

Author: jtschuster

src/tools/illink/illink.sln

@@ -223,6 +223,7 @@ Global
 		SolutionGuid = {E43A3901-42B0-48CA-BB36-5CD40A99A6EE}
 	EndGlobalSection
 	GlobalSection(SharedMSBuildProjectFiles) = preSolution
+		test\Trimming.Tests.Shared\Trimming.Tests.Shared.projitems*{400a1561-b6b6-482d-9e4c-3ddaede5bd07}*SharedItemsImports = 5
 		src\ILLink.Shared\ILLink.Shared.projitems*{dd28e2b1-057b-4b4d-a04d-b2ebd9e76e46}*SharedItemsImports = 5
 		src\ILLink.Shared\ILLink.Shared.projitems*{f1a44a78-34ee-408b-8285-9a26f0e7d4f2}*SharedItemsImports = 5
 		src\ILLink.Shared\ILLink.Shared.projitems*{ff598e93-8e9e-4091-9f50-61a7572663ae}*SharedItemsImports = 13

src/tools/illink/src/linker/CompatibilitySuppressions.xml

@@ -253,6 +253,10 @@
     <DiagnosticId>CP0001</DiagnosticId>
     <Target>T:Mono.Linker.ILogger</Target>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0001</DiagnosticId>
+    <Target>T:Mono.Linker.InterfaceImplementor</Target>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0001</DiagnosticId>
     <Target>T:Mono.Linker.InternalErrorException</Target>
@@ -1481,10 +1485,6 @@
     <DiagnosticId>CP0002</DiagnosticId>
     <Target>M:Mono.Linker.OverrideInformation.get_IsOverrideOfInterfaceMember</Target>
   </Suppression>
-  <Suppression>
-    <DiagnosticId>CP0002</DiagnosticId>
-    <Target>M:Mono.Linker.OverrideInformation.get_IsStaticInterfaceMethodPair</Target>
-  </Suppression>
   <Suppression>
     <DiagnosticId>CP0002</DiagnosticId>
     <Target>M:Mono.Linker.Steps.BaseStep.get_MarkingHelpers</Target>

src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

@@ -701,17 +701,16 @@ void ProcessVirtualMethod (MethodDefinition method)
 				var defaultImplementations = Annotations.GetDefaultInterfaceImplementations (method);
 				if (defaultImplementations is not null) {
 					foreach (var dimInfo in defaultImplementations) {
-						ProcessDefaultImplementation (dimInfo.ImplementingType, dimInfo.InterfaceImpl, dimInfo.DefaultInterfaceMethod);
+						ProcessDefaultImplementation (dimInfo);
 
-						var ov = new OverrideInformation (method, dimInfo.DefaultInterfaceMethod, Context, dimInfo.InterfaceImpl);
-						if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov, dimInfo.ImplementingType))
-							MarkMethod (ov.Override, new DependencyInfo (DependencyKind.Override, ov.Base), ScopeStack.CurrentScope.Origin);
+						if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (dimInfo))
+							MarkMethod (dimInfo.Override, new DependencyInfo (DependencyKind.Override, dimInfo.Base), ScopeStack.CurrentScope.Origin);
 					}
 				}
 				var overridingMethods = Annotations.GetOverrides (method);
 				if (overridingMethods is not null) {
-					foreach (var ov in overridingMethods) {
-						if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov, ov.Override.DeclaringType))
+					foreach (OverrideInformation ov in overridingMethods) {
+						if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov))
 							MarkMethod (ov.Override, new DependencyInfo (DependencyKind.Override, ov.Base), ScopeStack.CurrentScope.Origin);
 					}
 				}
@@ -819,13 +818,14 @@ bool RequiresInterfaceRecursively (TypeDefinition typeToExamine, TypeDefinition
 			return false;
 		}
 
-		void ProcessDefaultImplementation (TypeDefinition typeWithDefaultImplementedInterfaceMethod, InterfaceImplementation implementation, MethodDefinition implementationMethod)
+		void ProcessDefaultImplementation (OverrideInformation ov)
 		{
-			if ((!implementationMethod.IsStatic && !Annotations.IsInstantiated (typeWithDefaultImplementedInterfaceMethod))
-				|| implementationMethod.IsStatic && !Annotations.IsRelevantToVariantCasting (typeWithDefaultImplementedInterfaceMethod))
+			Debug.Assert (ov.IsOverrideOfInterfaceMember);
+			if ((!ov.Override.IsStatic && !Annotations.IsInstantiated (ov.InterfaceImplementor.Implementor))
+				|| ov.Override.IsStatic && !Annotations.IsRelevantToVariantCasting (ov.InterfaceImplementor.Implementor))
 				return;
 
-			MarkInterfaceImplementation (implementation);
+			MarkInterfaceImplementation (ov.InterfaceImplementor.InterfaceImplementation);
 		}
 
 		void MarkMarshalSpec (IMarshalInfoProvider spec, in DependencyInfo reason)
@@ -2549,11 +2549,11 @@ bool IsMethodNeededByTypeDueToPreservedScope (MethodDefinition method)
 		/// <summary>
 		/// Returns true if the override method is required due to the interface that the base method is declared on. See doc at <see href="docs/methods-kept-by-interface.md"/> for explanation of logic.
 		/// </summary>
-		bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformation overrideInformation, TypeDefinition typeThatImplsInterface)
+		bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformation overrideInformation)
 		{
 			var @base = overrideInformation.Base;
 			var method = overrideInformation.Override;
-			Debug.Assert (@base.DeclaringType.IsInterface);
+			Debug.Assert (overrideInformation.IsOverrideOfInterfaceMember);
 			if (@base is null || method is null || @base.DeclaringType is null)
 				return false;
 
@@ -2562,7 +2562,7 @@ bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformat
 
 			// If the interface implementation is not marked, do not mark the implementation method
 			// A type that doesn't implement the interface isn't required to have methods that implement the interface.
-			InterfaceImplementation? iface = overrideInformation.MatchingInterfaceImplementation;
+			InterfaceImplementation? iface = overrideInformation.InterfaceImplementor.InterfaceImplementation;
 			if (!((iface is not null && Annotations.IsMarked (iface))
 				|| IsInterfaceImplementationMarkedRecursively (method.DeclaringType, @base.DeclaringType)))
 				return false;
@@ -2580,12 +2580,12 @@ bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformat
 			// If the method is static and the implementing type is relevant to variant casting, mark the implementation method.
 			// A static method may only be called through a constrained call if the type is relevant to variant casting.
 			if (@base.IsStatic)
-				return Annotations.IsRelevantToVariantCasting (typeThatImplsInterface)
+				return Annotations.IsRelevantToVariantCasting (overrideInformation.InterfaceImplementor.Implementor)
 					|| IgnoreScope (@base.DeclaringType.Scope);
 
 			// If the implementing type is marked as instantiated, mark the implementation method.
 			// If the type is not instantiated, do not mark the implementation method
-			return Annotations.IsInstantiated (typeThatImplsInterface);
+			return Annotations.IsInstantiated (overrideInformation.InterfaceImplementor.Implementor);
 		}
 
 		static bool IsSpecialSerializationConstructor (MethodDefinition method)
@@ -3256,7 +3256,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
 					// Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round.
 					if (!markAllOverrides &&
 						Context.Resolve (@base) is MethodDefinition baseDefinition
-						&& new OverrideInformation.OverridePair (baseDefinition, method).IsStaticInterfaceMethodPair ())
+						&& baseDefinition.DeclaringType.IsInterface && baseDefinition.IsStatic && method.IsStatic)
 						continue;
 					MarkMethod (@base, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin);
 					MarkExplicitInterfaceImplementation (method, @base);

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

@@ -462,7 +462,7 @@ public bool IsPublic (IMetadataTokenProvider provider)
 		/// DefaultInterfaceMethod is the method that implements <paramref name="method"/>.
 		/// </summary>
 		/// <param name="method">The interface method to find default implementations for</param>
-		public IEnumerable<(TypeDefinition ImplementingType, InterfaceImplementation InterfaceImpl, MethodDefinition DefaultInterfaceMethod)>? GetDefaultInterfaceImplementations (MethodDefinition method)
+		public IEnumerable<OverrideInformation>? GetDefaultInterfaceImplementations (MethodDefinition method)
 		{
 			return TypeMapInfo.GetDefaultInterfaceImplementations (method);
 		}

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

@@ -0,0 +1,59 @@
+// 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;
+using System.Collections.Generic;
+using System.Diagnostics;
+using Mono.Cecil;
+
+namespace Mono.Linker
+{
+	public class InterfaceImplementor
+	{
+		/// <summary>
+		/// The type that implements <see cref="InterfaceImplementor.InterfaceType"/>.
+		/// </summary>
+		public TypeDefinition Implementor { get; }
+		/// <summary>
+		/// The .interfaceimpl on <see cref="InterfaceImplementor.Implementor"/>that points to <see cref="InterfaceImplementor.InterfaceType"/>
+		/// </summary>
+		public InterfaceImplementation InterfaceImplementation { get; }
+		/// <summary>
+		/// The type of the interface that is implemented by <see cref="InterfaceImplementor.Implementor"/>
+		/// </summary>
+		public TypeDefinition InterfaceType { get; }
+
+		public InterfaceImplementor (TypeDefinition implementor, InterfaceImplementation interfaceImplementation, TypeDefinition interfaceType, IMetadataResolver resolver)
+		{
+			Implementor = implementor;
+			InterfaceImplementation = interfaceImplementation;
+			InterfaceType = interfaceType;
+			Debug.Assert(resolver.Resolve (interfaceImplementation.InterfaceType) == interfaceType);
+		}
+
+		public static InterfaceImplementor Create(TypeDefinition implementor, TypeDefinition interfaceType, IMetadataResolver resolver)
+		{
+			foreach(InterfaceImplementation iface in implementor.Interfaces) {
+				if (resolver.Resolve(iface.InterfaceType) == interfaceType) {
+					return new InterfaceImplementor(implementor, iface, interfaceType, resolver);
+				}
+			}
+
+			Queue<TypeDefinition> ifacesToCheck = new ();
+			ifacesToCheck.Enqueue(implementor);
+			while (ifacesToCheck.Count > 0) {
+				var currentIface = ifacesToCheck.Dequeue ();
+
+				foreach(InterfaceImplementation ifaceImpl in currentIface.Interfaces) {
+					var iface = resolver.Resolve (ifaceImpl.InterfaceType);
+					if (iface == interfaceType) {
+						return new InterfaceImplementor(implementor, ifaceImpl, interfaceType, resolver);
+					}
+					ifacesToCheck.Enqueue (iface);
+				}
+			}
+			throw new InvalidOperationException ($"Type '{implementor.FullName}' does not implement interface '{interfaceType.FullName}' directly or through any interfaces");
+		}
+	}
+}

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

@@ -3,71 +3,39 @@
 
 using System.Diagnostics;
 using Mono.Cecil;
+using System.Diagnostics.CodeAnalysis;
 
 namespace Mono.Linker
 {
 	[DebuggerDisplay ("{Override}")]
 	public class OverrideInformation
 	{
-		readonly ITryResolveMetadata resolver;
-		readonly OverridePair _pair;
-		private InterfaceImplementation? _matchingInterfaceImplementation;
+		public MethodDefinition Base { get; }
 
-		public OverrideInformation (MethodDefinition @base, MethodDefinition @override, ITryResolveMetadata resolver, InterfaceImplementation? matchingInterfaceImplementation = null)
-		{
-			_pair = new OverridePair (@base, @override);
-			_matchingInterfaceImplementation = matchingInterfaceImplementation;
-			this.resolver = resolver;
-		}
-		public readonly record struct OverridePair (MethodDefinition Base, MethodDefinition Override)
-		{
-			public bool IsStaticInterfaceMethodPair () => Base.DeclaringType.IsInterface && Base.IsStatic && Override.IsStatic;
-			public InterfaceImplementation? GetMatchingInterfaceImplementation (ITryResolveMetadata resolver)
-			{
-				if (!Base.DeclaringType.IsInterface)
-					return null;
-				var interfaceType = Base.DeclaringType;
-				foreach (var @interface in Override.DeclaringType.Interfaces) {
-					if (resolver.TryResolve (@interface.InterfaceType)?.Equals (interfaceType) == true) {
-						return @interface;
-					}
-				}
-				return null;
-			}
-		}
+		public MethodDefinition Override { get; }
 
-		public MethodDefinition Base { get => _pair.Base; }
-		public MethodDefinition Override { get => _pair.Override; }
-		public InterfaceImplementation? MatchingInterfaceImplementation {
-			get {
-				if (_matchingInterfaceImplementation is not null)
-					return _matchingInterfaceImplementation;
-				_matchingInterfaceImplementation = _pair.GetMatchingInterfaceImplementation (resolver);
-				return _matchingInterfaceImplementation;
-			}
-		}
+		internal InterfaceImplementor? InterfaceImplementor { get; }
 
-		public bool IsOverrideOfInterfaceMember {
-			get {
-				if (MatchingInterfaceImplementation != null)
-					return true;
-
-				return Base.DeclaringType.IsInterface;
-			}
+		internal OverrideInformation (MethodDefinition @base, MethodDefinition @override, InterfaceImplementor? interfaceImplementor = null)
+		{
+			Base = @base;
+			Override = @override;
+			InterfaceImplementor = interfaceImplementor;
+			// Ensure we have an interface implementation if the base method is from an interface and the override method is on a class
+			Debug.Assert(@base.DeclaringType.IsInterface && interfaceImplementor != null
+						|| !@base.DeclaringType.IsInterface && interfaceImplementor == null);
+			// Ensure the interfaceImplementor is for the interface we expect
+			Debug.Assert (@base.DeclaringType.IsInterface ? interfaceImplementor!.InterfaceType == @base.DeclaringType : true);
 		}
 
-		public TypeDefinition? InterfaceType {
-			get {
-				if (!IsOverrideOfInterfaceMember)
-					return null;
+		public InterfaceImplementation? MatchingInterfaceImplementation
+			=> InterfaceImplementor?.InterfaceImplementation;
 
-				if (MatchingInterfaceImplementation != null)
-					return resolver.TryResolve (MatchingInterfaceImplementation.InterfaceType);
-
-				return Base.DeclaringType;
-			}
-		}
+		public TypeDefinition? InterfaceType
+			=> InterfaceImplementor?.InterfaceType;
 
-		public bool IsStaticInterfaceMethodPair => _pair.IsStaticInterfaceMethodPair ();
+		[MemberNotNullWhen (true, nameof (InterfaceImplementor), nameof (MatchingInterfaceImplementation))]
+		public bool IsOverrideOfInterfaceMember
+			=> InterfaceImplementor != null;
 	}
 }