AOT/analyzer should not warn for MakeGenericType of reference type

[sbomer]

• MakeGenericType over reference types is supported in native AOT, so the following should not warn:

void Foo(Type t) {
    if (!t.IsValueType)
        typeof(G<>).MakeGenericType(t); // currently warns with IL3050
}

• Probably the class constraint should be supported as well:

void Foo<T>() where T : class {
    typeof(G<>).MakeGenericType(typeof(T));
}

• We may also want to introduce an annotation to propagate the fact that a type is a reference type across methods, for example:

void Foo(Type t) {
    if (!t.IsValueType)
        Bar(t);
}

void Bar([ReferenceType] Type t) {
    typeof(G<>).MakeGenericType(t);
}

(The latter idea is something that has come up in discussion with @vitek-karas and @MichalStrehovsky, not sure if there's another tracking issue for it already.)

Context: https://github.com/dotnet/maui/pull/20058/files/7f4d26ca0aee7dc9d995e9776573125b35ad44b6#r1463169163

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

MakeGenericType over reference types is supported in native AOT, so the following should not warn:

void Foo(Type t) {
    if (!t.IsValueType)
        typeof(G<>).MakeGenericType(t); // currently warns with IL3050
}

Probably the class constraint should be supported as well:

void Foo<T>() where T : class {
    typeof(G<>).MakeGenericType(typeof(T));
}

We may also want to introduce an annotation to propagate the fact that a type is a reference type across methods, for example:

void Foo(Type t) {
    if (!t.IsValueType)
        Bar(t);
}

void Bar([ReferenceType] Type t) {
    typeof(G<>).MakeGenericType(t);
}

(The latter idea is something that has come up in discussion with @vitek-karas and @MichalStrehovsky, not sure if there's another tracking issue for it already.)

Context: https://github.com/dotnet/maui/pull/20058/files/7f4d26ca0aee7dc9d995e9776573125b35ad44b6#r1463169163

Author:	sbomer
Assignees:	-
Labels:	untriaged, area-NativeAOT-coreclr, needs-area-label
Milestone:	-

[MichalPetryka]

Does NativeAOT always generate a shared generic impl for this or would it need to scan for MakeGenericType and generate it then?

[sbomer]

Based on some experimentation, it looks like it doesn't always generate the shared impl. I'm not sure exactly what is enough to generate the shared impl. Just calling G<SomeOtherReferenceType>.Method() won't do it - but accessing some of the generic type's methods via reflection will. For example MakeGenericType fails here:

typeof(G<>).MakeGenericType(typeof(C));
// typeof(G<>).GetMethod("Foo");

class G<T> {
    public static void Foo() {
        Console.WriteLine(typeof(T));
    }
}

class C {}

but succeeds if you uncomment the GetMethod("Foo").

@MichalStrehovsky could probably give a better answer.

[MichalStrehovsky]

Based on some experimentation, it looks like it doesn't always generate the shared impl. I'm not sure exactly what is enough to generate the shared impl. Just calling G<SomeOtherReferenceType>.Method() won't do it - but accessing some of the generic type's methods via reflection will. For example MakeGenericType fails here:

typeof(G<>).MakeGenericType(typeof(C));
// typeof(G<>).GetMethod("Foo");

class G<T> {
    public static void Foo() {
        Console.WriteLine(typeof(T));
    }
}

class C {}

but succeeds if you uncomment the GetMethod("Foo").

@MichalStrehovsky could probably give a better answer.

Looks like we don't treat MakeGenericType specially in dataflow analysis - the only special handling is to trigger the AOT warning here:

runtime/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs

Lines 376 to 382 in 135fec0

	// Special case some intrinsics for AOT handling (on top of the trimming handling done in the HandleCallAction)
	switch (intrinsicId)
	{
	case IntrinsicId.Type_MakeGenericType:
	case IntrinsicId.MethodInfo_MakeGenericMethod:
	CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresDynamicCodeAttribute);
	break;

It works if a method/constructor/field is subsequently accessed because dataflow will report "G<>.Foo is reflection accessed" and then the compiler will try to make G<>.Foo work on at least something.

It shouldn't be hard to make this work for an unused type.