Improvements to method overrides (SamboyCoding#436)

* Improvements to method overrides
* Add `BaseMethod` property
* Improve lookup for overrides
* Add some barebones testing

* Revision

* Add more tests

* Remove break statement

Author: ds5678

Cpp2IL.Core.Tests/AccessibilityExtensionsTests.cs

@@ -9,7 +9,7 @@ public class AccessibilityExtensionsTests
     [Test]
     public void AccessibilityTests()
     {
-        var appContext = GameLoader.LoadSimpleGame();
+        var appContext = TestGameLoader.LoadSimple2019Game();
         var mscorlib = appContext.AssembliesByName["mscorlib"];
         var coreModule = appContext.AssembliesByName["UnityEngine.CoreModule"];
 

Cpp2IL.Core.Tests/DllOutputTests.cs

@@ -15,7 +15,7 @@ public class DllOutputTests
     [Test]
     public void AllAssembliesBuild()
     {
-        var appContext = GameLoader.LoadSimpleGame();
+        var appContext = TestGameLoader.LoadSimple2019Game();
 
         var assemblies = new AsmResolverDllOutputFormatDefault().BuildAssemblies(appContext);
 
@@ -35,7 +35,7 @@ public void AllAssembliesBuild()
     [Test]
     public void MscorlibIsItsOwnCorLib()
     {
-        var appContext = GameLoader.LoadSimpleGame();
+        var appContext = TestGameLoader.LoadSimple2019Game();
 
         var assemblies = new AsmResolverDllOutputFormatEmpty().BuildAssemblies(appContext);
 
@@ -47,7 +47,7 @@ public void MscorlibIsItsOwnCorLib()
     [Test]
     public void MscorlibHasNoAssemblyReferences()
     {
-        var appContext = GameLoader.LoadSimpleGame();
+        var appContext = TestGameLoader.LoadSimple2019Game();
 
         var assemblies = new AsmResolverDllOutputFormatEmpty().BuildAssemblies(appContext);
 
@@ -59,7 +59,7 @@ public void MscorlibHasNoAssemblyReferences()
     [Test]
     public void MscorlibHasNoTypeReferences()
     {
-        var appContext = GameLoader.LoadSimpleGame();
+        var appContext = TestGameLoader.LoadSimple2019Game();
 
         var assemblies = new AsmResolverDllOutputFormatEmpty().BuildAssemblies(appContext);
 

Cpp2IL.Core.Tests/GameLoader.cs

@@ -0,0 +1,42 @@
+using System.Linq;
+using Cpp2IL.Core.Model.Contexts;
+
+namespace Cpp2IL.Core.Tests;
+
+public class MethodOverridesTests
+{
+    [Test]
+    public void OverridesTests()
+    {
+        var appContext = TestGameLoader.LoadSimple2019Game();
+        var mscorlib = appContext.AssembliesByName["mscorlib"];
+
+        var @enum = mscorlib.GetTypeByFullName("System.Enum")!;
+        var list = mscorlib.GetTypeByFullName("System.Collections.Generic.List`1")!;
+        var iList = mscorlib.GetTypeByFullName("System.Collections.IList")!;
+        var ordinalComparer = mscorlib.GetTypeByFullName("System.OrdinalComparer")!;
+        using (Assert.EnterMultipleScope())
+        {
+            // Simple override
+            Assert.That(@enum.GetMethod("ToString", 0).BaseMethod, Is.Not.Null);
+            Assert.That(@enum.GetMethod("ToString", 0).Overrides.Count(), Is.EqualTo(1));
+
+            // Simple interface implementation
+            Assert.That(list.GetMethod("get_Count").BaseMethod, Is.Null);
+            Assert.That(list.GetMethod("get_Count").Overrides.Count(), Is.EqualTo(3)); // ICollection, ICollection<T>, IReadOnlyCollection<T>
+
+            // Explicit interface implementation
+            Assert.That(list.GetMethod("System.Collections.Generic.ICollection<T>.get_IsReadOnly").Overrides.Count(), Is.EqualTo(1));
+
+            // No override
+            Assert.That(list.GetMethod("EnsureCapacity").Overrides.Count(), Is.EqualTo(0));
+
+            // Check that the base method can be found even if higher up in the inheritance chain.
+            // OrdinalComparer inherits from StringComparer, but StringComparer doesn't override GetHashCode.
+            Assert.That(ordinalComparer.GetMethod("GetHashCode", 0).BaseMethod?.DeclaringType?.FullName, Is.EqualTo("System.Object"));
+
+            // Interface methods should not override anything
+            Assert.That(iList.Methods.Select(m => m.Overrides.Count()), Is.All.EqualTo(0));
+        }
+    }
+}

Cpp2IL.Core.Tests/MethodOverridesTests.cs

@@ -11,7 +11,7 @@ public class PrimitiveTests
     [Test]
     public void PrimitiveTypesAreCorLibTypeSignature()
     {
-        var appContext = GameLoader.LoadSimpleGame();
+        var appContext = TestGameLoader.LoadSimple2019Game();
 
         var assemblies = new AsmResolverDllOutputFormatEmpty().BuildAssemblies(appContext);
 

Cpp2IL.Core.Tests/PrimitiveTests.cs

@@ -1,5 +1,5 @@
-using System.Diagnostics.CodeAnalysis;
 using AssetRipper.Primitives;
+using Cpp2IL.Core.Model.Contexts;
 
 namespace Cpp2IL.Core.Tests;
 
@@ -15,15 +15,17 @@ private static void EnsureInit()
         _initialized = true;
     }
 
-    public static void LoadSimple2019Game()
+    public static ApplicationAnalysisContext LoadSimple2019Game()
     {
         EnsureInit();
         Cpp2IlApi.InitializeLibCpp2Il(Paths.Simple2019Game.GameAssembly, Paths.Simple2019Game.Metadata, new UnityVersion(2019, 4, 34, UnityVersionType.Final, 1));
+        return Cpp2IlApi.CurrentAppContext!;
     }
 
-    public static void LoadSimple2022Game()
+    public static ApplicationAnalysisContext LoadSimple2022Game()
     {
         EnsureInit();
         Cpp2IlApi.InitializeLibCpp2Il(Paths.Simple2022Game.GameAssembly, Paths.Simple2022Game.Metadata, new UnityVersion(2022, 3, 35, UnityVersionType.Final, 1));
+        return Cpp2IlApi.CurrentAppContext!;
     }
 }

Cpp2IL.Core.Tests/TestGameLoader.cs

@@ -0,0 +1,17 @@
+using System.Linq;
+using Cpp2IL.Core.Model.Contexts;
+
+namespace Cpp2IL.Core.Tests;
+
+internal static class TypeAnalysisContextExtensions
+{
+    public static MethodAnalysisContext GetMethod(this TypeAnalysisContext type, string methodName)
+    {
+        return type.Methods.Single(m => m.Name == methodName);
+    }
+
+    public static MethodAnalysisContext GetMethod(this TypeAnalysisContext type, string methodName, int parameterCount)
+    {
+        return type.Methods.Single(m => m.Name == methodName && m.ParameterCount == parameterCount);
+    }
+}

Cpp2IL.Core.Tests/TypeAnalysisContextExtensions.cs

@@ -88,6 +88,8 @@ public class MethodAnalysisContext : HasCustomAttributesAndName, IMethodInfoProv
 
     protected Memory<byte>? rawMethodBody;
 
+    public MethodAnalysisContext? BaseMethod => Overrides.FirstOrDefault(m => m.DeclaringType?.IsInterface is false);
+
     /// <summary>
     /// The set of methods which this method overrides.
     /// </summary>
@@ -108,22 +110,29 @@ public virtual IEnumerable<MethodAnalysisContext> Overrides
 
             return GetOverriddenMethods(declaringTypeDefinition, vtable);
 
-            static void GetParentTypeAndSlot(Il2CppTypeDefinition declaringTypeDefinition, int vtableIndex, out Il2CppTypeReflectionData? parentType, out int slot)
+            bool TryGetMethodForSlot(TypeAnalysisContext declaringType, int slot, [NotNullWhen(true)] out MethodAnalysisContext? method)
             {
-                var interfaceOffsets = declaringTypeDefinition.InterfaceOffsets;
-                for (var i = interfaceOffsets.Length - 1; i >= 0; i--)
+                if (declaringType is GenericInstanceTypeAnalysisContext genericInstanceType)
+                {
+                    var genericMethod = genericInstanceType.GenericType.Methods.FirstOrDefault(m => m.Slot == slot);
+                    if (genericMethod is not null)
+                    {
+                        method = new ConcreteGenericMethodAnalysisContext(genericMethod, genericInstanceType.GenericArguments.ToArray(), []);
+                        return true;
+                    }
+                }
+                else
                 {
-                    var interfaceOffset = interfaceOffsets[i];
-                    if (vtableIndex >= interfaceOffset.offset)
+                    var baseMethod = declaringType.Methods.FirstOrDefault(m => m.Slot == slot);
+                    if (baseMethod is not null)
                     {
-                        slot = vtableIndex - interfaceOffsets[i].offset;
-                        parentType = interfaceOffset.Type;
-                        return;
+                        method = baseMethod;
+                        return true;
                     }
                 }
 
-                parentType = declaringTypeDefinition.BaseType;
-                slot = vtableIndex;
+                method = null;
+                return false;
             }
 
             IEnumerable<MethodAnalysisContext> GetOverriddenMethods(Il2CppTypeDefinition declaringTypeDefinition, MetadataUsage?[] vtable)
@@ -137,23 +146,29 @@ IEnumerable<MethodAnalysisContext> GetOverriddenMethods(Il2CppTypeDefinition dec
                     if (vtableEntry.AsMethod() != Definition)
                         continue;
 
-                    GetParentTypeAndSlot(declaringTypeDefinition, i, out var parentType, out var slot);
-
-                    var parentTypeContext = parentType?.ToContext(CustomAttributeAssembly);
-                    if (parentTypeContext == null)
-                        continue;
-
-                    if (parentTypeContext is GenericInstanceTypeAnalysisContext genericInstanceType)
+                    // Normal inheritance
+                    var baseType = DeclaringType?.BaseType;
+                    while (baseType is not null)
                     {
-                        var parentMethod = genericInstanceType.GenericType.Methods.FirstOrDefault(m => m.Slot == slot);
-                        if (parentMethod is not null)
-                            yield return new ConcreteGenericMethodAnalysisContext(parentMethod, genericInstanceType.GenericArguments.ToArray(), []);
+                        if (TryGetMethodForSlot(baseType, i, out var method))
+                        {
+                            yield return method;
+                            break; // We only want direct overrides, not the entire inheritance chain.
+                        }
+                        baseType = baseType.BaseType;
                     }
-                    else
+
+                    // Interface inheritance
+                    foreach (var interfaceOffset in declaringTypeDefinition.InterfaceOffsets)
                     {
-                        var parentMethod = parentTypeContext.Methods.FirstOrDefault(m => m.Slot == slot);
-                        if (parentMethod is not null)
-                            yield return parentMethod;
+                        if (i >= interfaceOffset.offset)
+                        {
+                            var interfaceTypeContext = interfaceOffset.Type.ToContext(CustomAttributeAssembly);
+                            if (interfaceTypeContext != null && TryGetMethodForSlot(interfaceTypeContext, i - interfaceOffset.offset, out var method))
+                            {
+                                yield return method;
+                            }
+                        }
                     }
                 }
             }