Fix mapping of redeclared type parameters (#86353)
authorSven Boemer <sbomer@gmail.com>
Fri, 19 May 2023 17:25:45 +0000 (10:25 -0700)
committerGitHub <noreply@github.com>
Fri, 19 May 2023 17:25:45 +0000 (10:25 -0700)
We were skipping the type parameter mapping of compiler-generated
types whose only generic parameters were those implicitly created
for the declaring type's type parameters.

For the testcase in question, the nested state machine inherited
a generic parameter from the display class. This was causing
unnecessary warnings in a field assignment that assigned
`this` (an instance of the display class) to a field on the state
machine. In IL, that assignment references a field type like
`DisplayClass<T>` where `T` is the generic parameter on the state
machine. Here we were properly mapping type parameters of the
display class back to the annotated enclosing method's type
parameters, so we could tell that the "target" required
`PublicMethods`. But the substituted `T` from the state machine
was not mapped, causing the mismatch.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/CompilerGeneratedState.cs
src/tools/illink/src/linker/Linker.Dataflow/CompilerGeneratedState.cs
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs

index 359ccf7..cbfb822 100644 (file)
@@ -288,24 +288,11 @@ namespace ILCompiler.Dataflow
                     {
                         Debug.Assert(generatedType == generatedType.GetTypeDefinition());
 
-                        if (HasGenericParameters(generatedType))
+                        if (generatedType.HasInstantiation)
                             MapGeneratedTypeTypeParameters(generatedType);
                     }
                 }
 
-                /// <summary>
-                /// Check if the type itself is generic. The only difference is that
-                /// if the type is a nested type, the generic parameters from its
-                /// parent type don't count.
-                /// </summary>
-                static bool HasGenericParameters(MetadataType typeDef)
-                {
-                    if (typeDef.ContainingType == null)
-                        return typeDef.HasInstantiation;
-
-                    return typeDef.Instantiation.Length > typeDef.ContainingType.Instantiation.Length;
-                }
-
                 void MapGeneratedTypeTypeParameters(MetadataType generatedType)
                 {
                     Debug.Assert(CompilerGeneratedNames.IsGeneratedType(generatedType.Name));
index cc7d205..0461779 100644 (file)
@@ -283,7 +283,7 @@ namespace Mono.Linker.Dataflow
                        // Now that we have instantiating methods fully filled out, walk the generated types and fill in the attribute
                        // providers
                        foreach (var generatedType in generatedTypeToTypeArgs.Keys) {
-                               if (HasGenericParameters (generatedType)) {
+                               if (generatedType.HasGenericParameters) {
                                        MapGeneratedTypeTypeParameters (generatedType, generatedTypeToTypeArgs, _context);
                                        // Finally, add resolved type arguments to the cache
                                        var info = generatedTypeToTypeArgs[generatedType];
@@ -299,19 +299,6 @@ namespace Mono.Linker.Dataflow
                        return type;
 
                        /// <summary>
-                       /// Check if the type itself is generic. The only difference is that
-                       /// if the type is a nested type, the generic parameters from its
-                       /// parent type don't count.
-                       /// </summary>
-                       static bool HasGenericParameters (TypeDefinition typeDef)
-                       {
-                               if (!typeDef.IsNested)
-                                       return typeDef.HasGenericParameters;
-
-                               return typeDef.GenericParameters.Count > typeDef.DeclaringType.GenericParameters.Count;
-                       }
-
-                       /// <summary>
                        /// Attempts to reverse the process of the compiler's alpha renaming. So if the original code was
                        /// something like this:
                        /// <code>
index f99a80b..c414bc2 100644 (file)
@@ -32,6 +32,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
                        AsyncCapture ();
                        AsyncTypeMismatch ();
                        AsyncInsideClosure ();
+                       AsyncInsideClosureNonGeneric ();
                        AsyncInsideClosureMismatch ();
 
                        // Closures
@@ -220,6 +221,22 @@ namespace Mono.Linker.Tests.Cases.DataFlow
                        }
                }
 
+               private static void AsyncInsideClosureNonGeneric ()
+               {
+                       Outer<string> ();
+                       void Outer<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T1> ()
+                       {
+                               int x = 0;
+                               Inner ().Wait ();
+                               async Task Inner ()
+                               {
+                                       await Task.Delay (0);
+                                       x++;
+                                       _ = typeof (T1).GetMethods ();
+                               }
+                       }
+               }
+
                private static void AsyncInsideClosureMismatch ()
                {
                        Outer<string> ();