Linker is warning for Assembly.GetType for hard-coded type

[krwq]

In dotnet/runtime we have https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/DynamicDebuggerProxy.cs#L420

private static readonly Type ComObjectType = typeof(object).Assembly.GetType("System.__ComObject");

with a single usage of that field https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/DynamicDebuggerProxy.cs#L445 :

else if (obj != null && ComObjectType.IsAssignableFrom(obj.GetType()))

it seems that linker should be able to figure out what to do here but it warns instead:

D:\src\runtime\src\libraries\Microsoft.CSharp\src\Microsoft\CSharp\RuntimeBinder\DynamicDebuggerProxy.cs(420,9): Trim analysis warning IL2026: Microsoft.CSharp.RuntimeBinder.DynamicMetaObjectProviderDebugView..cctor(): Using method 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed. [d:\src\runtime\src\libraries\src.proj]

cc: @eerhardt

[MichalStrehovsky]

Can you change this to:

private static readonly Type ComObjectType = Type.GetType("System.__ComObject, System.Runtime");

It will be equivalent and illink models that API.

Modeling assembly was scoped out for now because this is the only situation where it would work (both the assembly and the type in question need to be visible).

Type.GetType can also work if the string comes from elsewhere so there's better payoff in implementing that one as intrinsic.

[marek-safar]

Should we tweak the error message or add a special version to include Type.GetType suggestion?

[eerhardt]

Roughly, the same pattern is found here:

https://github.com/dotnet/runtime/blob/e0bbfa0f555cf5b9d8b8749e1a91a78924c9d927/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs#L2924-L2926

public ComNativeDescriptorProxy()
{
    Assembly assembly = Assembly.Load("System.Windows.Forms");
    Type realComNativeDescriptor = assembly.GetType("System.Windows.Forms.ComponentModel.Com2Interop.ComNativeDescriptor", throwOnError: true);
    _comNativeDescriptor = (TypeDescriptionProvider)Activator.CreateInstance(realComNativeDescriptor);
}

Albeit, this is pointing to an Assembly and Type that is unknown when analyzing the runtimepack.

[eerhardt]

See https://github.com/dotnet/corefx/pull/36496#discussion_r270608490 for why it was written this way, and not

Type.GetType("System.Windows.Forms.ComponentModel.Com2Interop.ComNativeDescriptor, System.Windows.Forms");

This is purely because it is easier to determine what part failed - the assembly or type discovery. If and when we get better fusion logging, the one line makes sense for sure.

@AaronRobinsonMSFT - is it OK if I change ComNativeDescriptorProxy to use Type.GetType with an assembly qualified name to help the linker here? Or should I just suppress the linker warnings?

(If I change it, I'd like to test the change to make sure it works - can you tell me how to test it?)

[MichalStrehovsky]

This is purely because it is easier to determine what part failed - the assembly or type discovery. If and when we get better fusion logging, the one line makes sense for sure.

I'm not sure I understand the motivation:

Assembly.Load("Dont");
Type.GetType("Foo, Dont", throwOnError: true);

Either of these will end up with a FileNotFoundException saying Dont cannot be found. What does the two liner help with?

[vitek-karas]

We now have a very detailed tracing of assembly loading failures: https://docs.microsoft.com/en-us/dotnet/core/dependency-loading/collect-details

[eerhardt]

What does the two liner help with?

My understanding is the scenario where the assembly can be found, but the Type can't be found.

But even in the 1-liner case you get different exception types between the 2 error cases: FileNotFoundException vs. TypeLoadException

[AaronRobinsonMSFT]

@eerhardt I've tagged the appropriate owners for the scenario in the referenced PR. It looks fine to me and I think it should be okay. I've forwarded you an email about this area and how it impact WinForms.