Skip to content

Commit

Permalink
Fix mapping of redeclared type parameters (#86353)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sbomer authored May 19, 2023
1 parent 3924920 commit ea94440
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -288,24 +288,11 @@ referencedMethod.OwningType is MetadataType generatedType &&
{
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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ referencedMethod.DeclaringType is var generatedType &&
// 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];
Expand All @@ -298,19 +298,6 @@ referencedMethod.DeclaringType is var generatedType &&
_cachedTypeToCompilerGeneratedMembers.Add (type, compilerGeneratedCallees);
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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public static void Main ()
AsyncCapture ();
AsyncTypeMismatch ();
AsyncInsideClosure ();
AsyncInsideClosureNonGeneric ();
AsyncInsideClosureMismatch ();

// Closures
Expand Down Expand Up @@ -220,6 +221,22 @@ private static void AsyncInsideClosure ()
}
}

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> ();
Expand Down

0 comments on commit ea94440

Please sign in to comment.