Clean up and add comments

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

@@ -3722,7 +3722,8 @@ protected virtual void MarkInstruction (Instruction instruction, MethodDefinitio
 					ScopeStack.UpdateCurrentScopeInstructionOffset (instruction.Offset);
 					if (markForReflectionAccess) {
 						MarkMethodVisibleToReflection (methodReference, new DependencyInfo (dependencyKind, method), ScopeStack.CurrentScope.Origin);
-					} else {
+					}
+					else {
 						MarkMethod (methodReference, new DependencyInfo (dependencyKind, method), ScopeStack.CurrentScope.Origin);
 					}
 					break;

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

@@ -2,13 +2,9 @@
 // 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.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
-using System.Runtime.InteropServices;
-using System.Transactions;
 using Mono.Cecil;
 
 namespace Mono.Linker
@@ -25,30 +21,44 @@ public class InterfaceImplementor
 		/// </summary>
 		public TypeDefinition? InterfaceType { get; }
 
-		public ImmutableArray<ImplNode> InterfaceImplementationNode { get; }
-
+		/// <summary>
+		/// A <see cref="TypeReference"/> to the <see cref="InterfaceType"/> with the generic parameters substituted.
+		/// </summary>
 		public TypeReference InflatedInterface { get; }
 
-		public InterfaceImplementor (TypeDefinition implementor, TypeDefinition? interfaceType, TypeReference inflatedInterface, ImmutableArray<ImplNode> implNode, LinkContext context)
+		/// <summary>
+		/// The graphs of <see cref="InterfaceImplementation"/>s that make <see cref="Implementor"/> implement the <see cref="InflatedInterface"/>.
+		/// There can be many ways a type implements an interface if an explicit interface implementation is given for an interface that is also implemented recursively due to another interface implementation.
+		/// It will be in the following order:
+		///  1. Explicit interface implementation on <see cref="Implementor"/>
+		///  2. Explicit interface implementation on a base type of <see cref="Implementor"/>
+		///  3. Recursive interface implementations on an explicitly implemented interface on <see cref="Implementor"/> or it's base types
+		/// </summary>
+		public readonly ImmutableArray<InterfaceImplementationNode> InterfaceImplementationNodes;
+
+		public InterfaceImplementor (TypeDefinition implementor, TypeDefinition? interfaceType, TypeReference inflatedInterface, ImmutableArray<InterfaceImplementationNode> implNode, LinkContext context)
 		{
 			Implementor = implementor;
 			InterfaceType = interfaceType;
-			InterfaceImplementationNode = implNode;
+			InterfaceImplementationNodes = implNode;
 			InflatedInterface = inflatedInterface;
 			Debug.Assert (context.Resolve (inflatedInterface) == interfaceType);
 			Debug.Assert (implNode.Length != 0);
 			Debug.Assert (implNode.All (i => interfaceType == context.Resolve (i.GetLast ().InterfaceType)));
 		}
 
+		/// <summary>
+		/// An Enumerable over the most direct <see cref="InterfaceImplementation"/> chain to the <see cref="InflatedInterface"/>
+		/// </summary>
 		public ShortestInterfaceImplementationChainEnumerator MostDirectInterfaceImplementationPath => new (this);
 
 		public struct ShortestInterfaceImplementationChainEnumerator
 		{
-			ImplNode _current;
+			InterfaceImplementationNode _current;
 			bool _hasMoved;
 			public ShortestInterfaceImplementationChainEnumerator (InterfaceImplementor implementor)
 			{
-				_current = implementor.InterfaceImplementationNode[0];
+				_current = implementor.InterfaceImplementationNodes[0];
 				_hasMoved = false;
 			}
 			public ShortestInterfaceImplementationChainEnumerator GetEnumerator() => this;
@@ -67,29 +77,50 @@ public bool MoveNext()
 			}
 		}
 
-		public void MarkShortestImplementation(AnnotationStore annotations, in DependencyInfo reason, in MessageOrigin origin)
-		{
-			InterfaceImplementationNode[0].MarkShortestImplementation (annotations, reason, origin);
-		}
-
+		/// <summary>
+		/// Returns true if the most direct implementation of <see cref="InflatedInterface"/> is marked. <see cref="Implementor"/> may still have a different recursive implementation marked.
+		/// </summary>
 		public bool IsMostDirectImplementationMarked(AnnotationStore annotations)
 		{
-			return InterfaceImplementationNode[0].IsMostDirectImplementationMarked (annotations);
+			return InterfaceImplementationNodes[0].IsMostDirectImplementationMarked (annotations);
 		}
 
+		/// <summary>
+		/// Returns true if <see cref="Implementor"/> implements <see cref="InflatedInterface"/> via any of the possible interface implementation chains.
+		/// </summary>
 		public bool IsMarked(AnnotationStore annotations)
 		{
-			foreach(var i in InterfaceImplementationNode) {
+			foreach(var i in InterfaceImplementationNodes) {
 				if (i.IsMarked(annotations))
 					return true;
 			}
 			return false;
 		}
 	}
 
-	public sealed record ImplNode (InterfaceImplementation InterfaceImplementation, TypeDefinition InterfaceImplementationProvider, ImmutableArray<ImplNode> Next) : IComparable<ImplNode>
+	/// <summary>
+	/// Represents a node in the graph of a type implementing an interface.
+	/// </summary>
+	public sealed class InterfaceImplementationNode : IComparable<InterfaceImplementationNode>
 	{
-		int _length = -1;
+		/// <summary>
+		/// The <see cref="Mono.Cecil.InterfaceImplementation"/> that is on <see cref="InterfaceImplementationProvider"/> that is part of the chain of interface implementations.
+		/// </summary>
+		public InterfaceImplementation InterfaceImplementation { get; }
+
+		/// <summary>
+		/// The type that has <see cref="InterfaceImplementation"/> in its <see cref="TypeDefinition.Interfaces"/>.
+		/// </summary>
+		public TypeDefinition InterfaceImplementationProvider { get; }
+
+		/// <summary>
+		/// The <see cref="InterfaceImplementationNode"/>s that are on the type pointed to by <see cref="InterfaceImplementation"/> that lead to the interface type.
+		/// </summary>
+		public ImmutableArray<InterfaceImplementationNode> Next { get; }
+
+		/// <summary>
+		/// The number of interface implementations on the most direct way the interface is implemented from <see cref="InterfaceImplementationProvider"/>
+		/// </summary>
 		public int Length {
 			get {
 				if (_length != -1)
@@ -99,15 +130,13 @@ public int Length {
 				return _length = Next[0].Length + 1;
 			}
 		}
+		int _length = -1;
 
-		public void MarkShortestImplementation (AnnotationStore annotations, in DependencyInfo reason, in MessageOrigin origin)
+		public InterfaceImplementationNode(InterfaceImplementation interfaceImplementation, TypeDefinition interfaceImplementationProvider, ImmutableArray<InterfaceImplementationNode> next)
 		{
-			ImplNode curr = this;
-			annotations.Mark (curr.InterfaceImplementation, reason, origin);
-			while (curr.Next.Length > 0) {
-				curr = curr.Next[0];
-				annotations.Mark (curr.InterfaceImplementation, reason, origin);
-			}
+			InterfaceImplementation = interfaceImplementation;
+			InterfaceImplementationProvider = interfaceImplementationProvider;
+			Next = next;
 		}
 
 		public bool IsMostDirectImplementationMarked(AnnotationStore annotations)
@@ -143,6 +172,6 @@ public InterfaceImplementation GetLast ()
 			return curr.InterfaceImplementation;
 		}
 
-		public int CompareTo (ImplNode? other) => this.Length - other!.Length;
+		int IComparable<InterfaceImplementationNode>.CompareTo (InterfaceImplementationNode? other) => this.Length - other!.Length;
 	}
 }

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

@@ -4,8 +4,6 @@
 using System.Diagnostics;
 using Mono.Cecil;
 using System.Diagnostics.CodeAnalysis;
-using System.Linq;
-using System.Transactions;
 
 namespace Mono.Linker
 {

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

@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
-using System.Collections.Generic;
 using Mono.Cecil;
 
 namespace Mono.Linker

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

@@ -155,26 +155,29 @@ void MapInterfacesOnType (TypeDefinition type)
 			if (_interfaces.ContainsKey (type))
 				return;
 
-			// Map from inflated interface type => list of every way to recursively implement that interface
-			Dictionary<TypeReference, SortedSet<ImplNode>> waysToImplementIface = new (InflatedInterfaceComparer);
+			// Map from inflated interface type => list of every way to recursively implement that interface in 'type declaration order' according to ECMA 335 12.2
+			Dictionary<TypeReference, List<InterfaceImplementationNode>> waysToImplementIface = new (InflatedInterfaceComparer);
 
-			// Get all interfaces directly implemented by this type
+			// Get all explicit interfaces of this type
 			foreach (var directIface in type.Interfaces) {
-				ImplNode directlyImplementedNode = new ImplNode (directIface, type, []);
+				InterfaceImplementationNode directlyImplementedNode = new InterfaceImplementationNode (directIface, type, []);
 				TypeReference inflatedDirectIface = directIface.InterfaceType.TryInflateFrom (type, context)!;
 				waysToImplementIface.AddToList (inflatedDirectIface, directlyImplementedNode);
 			}
 
-			// Add interfaces on base type with the same implementation chain
+			// Add interfaces on base type
 			if (type.BaseType is { } baseType && context.Resolve (baseType) is { } baseDef) {
 				MapInterfacesOnType (baseDef);
 				var baseInterfaces = _interfaces[baseDef];
 				foreach (var item in baseInterfaces) {
-					foreach (var node in item.InterfaceImplementationNode) {
-						waysToImplementIface.AddToList (item.InflatedInterface, node);
+					var inflatedInterface = item.InflatedInterface.TryInflateFrom (type.BaseType, context);
+					Debug.Assert (inflatedInterface is not null);
+					foreach (var node in item.InterfaceImplementationNodes) {
+						waysToImplementIface.AddToList (inflatedInterface, node);
 					}
 				}
 			}
+
 			// Recursive interfaces next to preserve Inherit/Implement tree order
 			foreach (var directIface in type.Interfaces) {
 				// If we can't resolve the interface type we can't find recursive interfaces
@@ -186,7 +189,7 @@ void MapInterfacesOnType (TypeDefinition type)
 				TypeReference inflatedDirectIface = directIface.InterfaceType.TryInflateFrom (type, context)!;
 				var recursiveInterfaces = _interfaces[ifaceDirectlyOnType];
 				foreach (var recursiveInterface in recursiveInterfaces) {
-					var implToRecursiveIfaceChain = new ImplNode (directIface, type, recursiveInterface.InterfaceImplementationNode);
+					var implToRecursiveIfaceChain = new InterfaceImplementationNode (directIface, type, recursiveInterface.InterfaceImplementationNodes);
 					// Inflate the generic arguments up to the terminal interfaceImpl to get the inflated interface type implemented by this type
 					TypeReference inflatedRecursiveInterface = inflatedDirectIface;
 					foreach (var interfaceImpl in recursiveInterface.MostDirectInterfaceImplementationPath) {
@@ -348,6 +351,9 @@ void AnnotateMethods (MethodDefinition @base, MethodDefinition @override, Interf
 			return context.TryResolve (type)?.BaseType;
 		}
 
+		/// <summary>
+		/// Compares two TypeReferences to interface types and determines if they are equivalent references, taking into account generic arguments and element types.
+		/// </summary>
 		public bool InterfaceTypeEquals (TypeReference? type, TypeReference? other)
 		{
 			Debug.Assert (type is not null && other is not null);

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

@@ -116,7 +116,6 @@ void parseArrayDimensions (ArrayType at)
 			}
 		}
 
-
 		public static TypeReference? GetInflatedDeclaringType (this TypeReference type, ITryResolveMetadata resolver)
 		{
 			if (type.IsGenericParameter || type.IsByReference || type.IsPointer)

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

@@ -15,6 +15,12 @@ public Task CanDisableUnusedInterfaces ()
 			return RunTest (allowMissingWarnings: true);
 		}
 
+		[Fact]
+		public Task GenericInterfaceImplementedMultipleTimes ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
 		[Fact]
 		public Task InterfaceOnUninstantiatedTypeRemoved ()
 		{

src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/OnReferenceType/BaseProvidesInterfaceMember/GenericInterfaceWithMethodManyVariations.cs

@@ -2,7 +2,6 @@
 
 namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.BaseProvidesInterfaceMember
 {
-	[SkipILVerify]
 	public class GenericInterfaceWithMethodManyVariations
 	{
 		public static void Main ()

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

@@ -29,7 +29,7 @@
     } // end of class I000
 
     .class interface nested public auto ansi abstract beforefieldinit I010
-        implements Library/I00
+        implements Library/I01
     {
     } // end of class I010
 

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

@@ -9,7 +9,7 @@
 using Mono.Linker.Tests.Cases.Expectations.Assertions;
 using Mono.Linker.Tests.Cases.Expectations.Metadata;
 
-namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces
+namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.RecursiveInterfaces
 {
 	[SetupLinkerArgument ("--skip-unresolved", "true")]
 	[TestCaseRequirements (TestRunCharacteristics.SupportsDefaultInterfaceMethods, "Requires support for default interface methods")]

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

@@ -9,7 +9,7 @@
 using Mono.Linker.Tests.Cases.Expectations.Assertions;
 using Mono.Linker.Tests.Cases.Expectations.Metadata;
 
-namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces
+namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.RecursiveInterfaces
 {
 	[SetupLinkerArgument ("--skip-unresolved", "true")]
 	[TestCaseRequirements (TestRunCharacteristics.SupportsDefaultInterfaceMethods, "Requires support for default interface methods")]

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

@@ -8,23 +8,21 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.RecursiveInterfaces
 {
 	/// <summary>
 	/// This tests that when a type implements an interface recursively (via implementations on implemented interfaces),
-	/// the shortest chain of interface implementations required to keep the implementation is marked
-	/// MyFoo => I000 => I00 => I0 (3 interfaceImpl long chain)
-	/// MyFoo => I0100 => I010 => I01 => I0 (4 interfaceImpl long chain)
+	/// the interface implementations kept are in type declaration order according to ECMA-335 12.2
 	/// </summary>
 	[TestCaseRequirements (TestRunCharacteristics.SupportsDefaultInterfaceMethods, "Requires support for default interface methods")]
 	[Define ("IL_ASSEMBLY_AVAILABLE")]
 	[SetupCompileBefore ("library.dll", new[] { "Dependencies/RecursiveInterfaceTwoImplementationPaths.il" })]
 	[SkipILVerify]
 #if IL_ASSEMBLY_AVAILABLE
 	[KeptTypeInAssembly ("library.dll", typeof(Library.MyClass))]
-	[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Library.MyClass), "library.dll", typeof (Library.I000))]
-	[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Library.I000), "library.dll", typeof (Library.I00))]
-	[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Library.I00), "library.dll", typeof (Library.I0))]
-	[RemovedTypeInAssembly("library.dll", typeof(Library.I01))]
-	[RemovedTypeInAssembly("library.dll", typeof(Library.I010))]
-	[RemovedTypeInAssembly("library.dll", typeof(Library.I0100))]
-	[RemovedInterfaceOnTypeInAssembly("library.dll", typeof (Library.MyClass), "library.dll", typeof (Library.I0100))]
+	[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Library.MyClass), "library.dll", typeof (Library.I0100))]
+	[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Library.I0100), "library.dll", typeof (Library.I010))]
+	[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Library.I010), "library.dll", typeof (Library.I01))]
+	[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Library.I01), "library.dll", typeof (Library.I0))]
+	[RemovedTypeInAssembly("library.dll", typeof(Library.I00))]
+	[RemovedTypeInAssembly("library.dll", typeof(Library.I000))]
+	[RemovedInterfaceOnTypeInAssembly("library.dll", typeof (Library.MyClass), "library.dll", typeof (Library.I000))]
 #endif
 	public class RecursiveInterfaceKept
 	{