Fix issue with being able to MakeGenericMethod methods on generic types (#89615)
authorMichal Strehovský <MichalStrehovsky@users.noreply.github.com>
Fri, 28 Jul 2023 07:10:59 +0000 (16:10 +0900)
committerGitHub <noreply@github.com>
Fri, 28 Jul 2023 07:10:59 +0000 (00:10 -0700)
The guarantees that one can always `MakeGeneric` instances over reference types should extend to generic methods on generic types.

We were previously not able to guarantee this, but #83438 made this possible.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs

index 34c8c43..e5784e0 100644 (file)
@@ -487,13 +487,28 @@ namespace ILCompiler
                         continue;
 
                     // Generic methods need to be instantiated over something.
+                    // We try to make up a canonical instantiation if possible to match the
+                    // general expectation that if one has a type, and a method on the uninstantiated type,
+                    // one can GetMemberWithSameMetadataDefinitionAs and MakeGenericMethod the result
+                    // over reference types. We promise reference type instantiations are possible.
+                    MethodDesc reflectedMethod;
                     if (method.HasInstantiation)
-                        continue;
+                    {
+                        Instantiation inst = TypeExtensions.GetInstantiationThatMeetsConstraints(method.Instantiation, allowCanon: true);
+                        if (inst.IsNull)
+                            continue;
+
+                        reflectedMethod = method.MakeInstantiatedMethod(inst);
+                    }
+                    else
+                    {
+                        reflectedMethod = method;
+                    }
 
                     dependencies ??= new CombinedDependencyList();
                     dependencies.Add(new DependencyNodeCore<NodeFactory>.CombinedDependencyListEntry(
-                        factory.ReflectedMethod(method),
-                        factory.ReflectedMethod(method.GetTypicalMethodDefinition()),
+                        factory.ReflectedMethod(reflectedMethod),
+                        factory.ReflectedMethod(reflectedMethod.GetTypicalMethodDefinition()),
                         "Methods have same reflectability"));
                 }
             }
index eea7cc8..b2ad549 100644 (file)
@@ -61,6 +61,7 @@ internal static class ReflectionTest
         TestVTableOfNullableUnderlyingTypes.Run();
         TestInterfaceLists.Run();
         TestMethodConsistency.Run();
+        TestGenericMethodOnGenericType.Run();
         TestIsValueTypeWithoutTypeHandle.Run();
         TestMdArrayLoad.Run();
         TestByRefTypeLoad.Run();
@@ -2271,6 +2272,40 @@ internal static class ReflectionTest
         }
     }
 
+    class TestGenericMethodOnGenericType
+    {
+        class Gen<T, U> { }
+
+        struct Atom1 { }
+        struct Atom2 { }
+
+        class GenericType<T>
+        {
+            public static Gen<T, U> GenericMethod1<U>() => new Gen<T, U>();
+            public static Gen<T, U> GenericMethod2<U>() => new Gen<T, U>();
+        }
+
+        static MethodInfo s_genericMethod2 = typeof(GenericType<>).GetMethod("GenericMethod2");
+
+        static object Make<T>(Type t)
+        {
+            if (t.IsValueType) throw null;
+            // AOT safe because we just checked t is not a valuetype
+            return typeof(GenericType<T>).GetMethod("GenericMethod1").MakeGenericMethod(t).Invoke(null, null);
+        }
+
+        public static void Run()
+        {
+            Make<Atom1>(typeof(object));
+
+            // This is supposed to be all AOT safe because we're instantiating over a reference type
+            // Ideally there would also be no AOT warning.
+            ((MethodInfo)(typeof(GenericType<Atom2>).GetMemberWithSameMetadataDefinitionAs(s_genericMethod2)))
+                .MakeGenericMethod(typeof(object))
+                .Invoke(null, null);
+        }
+    }
+
     class TestIsValueTypeWithoutTypeHandle
     {
         [Nothing(