Ensure consistent reflectability for generic methods

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDictionaryNode.cs

@@ -7,6 +7,8 @@
 using Internal.Text;
 using Internal.TypeSystem;
 
+using CombinedDependencyList = System.Collections.Generic.List<ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.CombinedDependencyListEntry>;
+
 namespace ILCompiler.DependencyAnalysis
 {
     /// <summary>
@@ -196,6 +198,13 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
         protected override TypeSystemContext Context => _owningMethod.Context;
         public override TypeSystemEntity OwningEntity => _owningMethod;
         public MethodDesc OwningMethod => _owningMethod;
+        public override bool HasConditionalStaticDependencies => true;
+        public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory)
+        {
+            CombinedDependencyList list = null;
+            factory.MetadataManager.GetConditionalDependenciesDueToMethodGenericDictionary(ref list, factory, _owningMethod);
+            return list ?? (IEnumerable<CombinedDependencyListEntry>)System.Array.Empty<CombinedDependencyListEntry>();
+        }
 
         protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
         {

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs

@@ -221,6 +221,9 @@ protected virtual void Graph_NewMarkedNode(DependencyNodeCore<NodeFactory> obj)
             if (dictionaryNode != null)
             {
                 _genericDictionariesGenerated.Add(dictionaryNode);
+
+                if (dictionaryNode.OwningEntity is MethodDesc method && AllMethodsCanBeReflectable)
+                    _reflectableMethods.Add(method);
             }
 
             if (obj is StructMarshallingDataNode structMarshallingDataNode)
@@ -465,6 +468,12 @@ public void GetDependenciesDueToMethodCodePresence(ref DependencyList dependenci
             GetDependenciesDueToMethodCodePresenceInternal(ref dependencies, factory, method, methodIL);
         }
 
+        public virtual void GetConditionalDependenciesDueToMethodGenericDictionary(ref CombinedDependencyList dependencies, NodeFactory factory, MethodDesc method)
+        {
+            // MetadataManagers can override this to provide additional dependencies caused by the presence of
+            // method generic dictionary.
+        }
+
         public virtual void GetConditionalDependenciesDueToMethodCodePresence(ref CombinedDependencyList dependencies, NodeFactory factory, MethodDesc method)
         {
             // MetadataManagers can override this to provide additional dependencies caused by the presence of

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs

@@ -533,6 +533,26 @@ protected override void GetDependenciesDueToMethodCodePresenceInternal(ref Depen
             }
         }
 
+        public override void GetConditionalDependenciesDueToMethodGenericDictionary(ref CombinedDependencyList dependencies, NodeFactory factory, MethodDesc method)
+        {
+            Debug.Assert(!method.IsSharedByGenericInstantiations && method.HasInstantiation && method.GetCanonMethodTarget(CanonicalFormKind.Specific) != method);
+
+            if ((_generationOptions & UsageBasedMetadataGenerationOptions.CreateReflectableArtifacts) == 0
+                && !IsReflectionBlocked(method))
+            {
+                // Ensure that if SomeMethod<T> is considered reflectable, SomeMethod<ConcreteType> is also reflectable.
+                // We only need this because there's a file format limitation in the reflection mapping tables that
+                // requires generic methods to be concrete (i.e. SomeMethod<__Canon> can never be in the mapping table).
+                // If we ever lift this limitation, this code can be deleted: the reflectability is going to be covered
+                // by GetConditionalDependenciesDueToMethodCodePresence below (we get that callback for SomeMethod<__Canon>).
+                MethodDesc typicalMethod = method.GetTypicalMethodDefinition();
+
+                dependencies ??= new CombinedDependencyList();
+                dependencies.Add(new DependencyNodeCore<NodeFactory>.CombinedDependencyListEntry(
+                    factory.ReflectableMethod(method), factory.ReflectableMethod(typicalMethod), "Reflectability of methods is same across genericness"));
+            }
+        }
+
         public override void GetConditionalDependenciesDueToMethodCodePresence(ref CombinedDependencyList dependencies, NodeFactory factory, MethodDesc method)
         {
             MethodDesc typicalMethod = method.GetTypicalMethodDefinition();
@@ -721,6 +741,12 @@ public override void GetDependenciesForGenericDictionary(ref DependencyList depe
                     GetFlowDependenciesForInstantiation(ref dependencies, factory, owningType.Instantiation, owningType.GetTypeDefinition().Instantiation, method);
                 }
             }
+
+            // Presence of code might trigger the reflectability dependencies.
+            if ((_generationOptions & UsageBasedMetadataGenerationOptions.CreateReflectableArtifacts) != 0)
+            {
+                GetDependenciesDueToReflectability(ref dependencies, factory, method);
+            }
         }
 
         public override void GetDependenciesForGenericDictionary(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)

src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs

@@ -33,6 +33,7 @@ private static int Main()
         TestRunClassConstructor.Run();
         TestFieldMetadata.Run();
         TestLinqInvocation.Run();
+        TestGenericMethodsHaveSameReflectability.Run();
 #if !OPTIMIZED_MODE_WITHOUT_SCANNER
         TestContainment.Run();
         TestInterfaceMethod.Run();
@@ -1874,6 +1875,35 @@ public static void Run()
         }
     }
 
+    class TestGenericMethodsHaveSameReflectability
+    {
+        public interface IHardToGuess { }
+
+        struct SomeStruct<T> : IHardToGuess { }
+
+        public static void TakeAGuess<T>() where T : struct, IHardToGuess { }
+
+        class Atom1 { }
+        class Atom2 { }
+
+        static Type s_someStructOverAtom1 = typeof(SomeStruct<Atom1>);
+
+        public static void Run()
+        {
+            // Statically call with SomeStruct over Atom2
+            TakeAGuess<SomeStruct<Atom2>>();
+
+            // MakeGenericMethod the method with SomeStruct over Atom1
+            // Note the compiler cannot figure out a suitable instantiation here because
+            // of the "struct, interface" constraint on T. But the expected side effect is that the static
+            // call above now became reflection-visible and will let the type loader make this
+            // work at runtime. All generic instantiations share the same refection visibility.
+            var mi = typeof(TestGenericMethodsHaveSameReflectability).GetMethod(nameof(TakeAGuess)).MakeGenericMethod(s_someStructOverAtom1);
+
+            mi.Invoke(null, Array.Empty<object>());
+        }
+    }
+
     class TestRunClassConstructor
     {
         static class TypeWithNoStaticFieldsButACCtor