Readonly Type fields should be treated like Type variables

[eerhardt]

In trying to make the following code trim compatible:

https://github.com/dotnet/runtime/blob/1a1ad943288b512a877222e2fdede0bc198df333/src/libraries/System.Private.Xml/src/System/Xml/Xsl/IlGen/GenerateHelper.cs#L81-L103

I wanted to write the following code:

internal class XmlILStorageMethods
{
    public readonly Type SeqType;
    public readonly MethodInfo SeqAdd;

    public XmlILStorageMethods(Type storageType)
    {
        if (storageType == typeof(XPathNavigator))
        {
            SeqType = typeof(XmlQueryNodeSequence);
            SeqAdd = SeqType.GetMethod("AddClone")!;
        }
        else if (storageType == typeof(XPathItem))
        {
            SeqType= typeof(XmlQueryItemSequence);
            SeqAdd = SeqType.GetMethod("AddClone")!;
        }
        else
        {
            SeqType = typeof(XmlQuerySequence<>).MakeGenericType(storageType);
            SeqAdd = SeqType.GetMethod("Add")!;
        }

        FieldInfo? seqEmpty = SeqType.GetField("Empty");
    }
}

However, the linker warns here that I haven't annotated the Type SeqType field appropriately on the calls to .GetMethod and that final line SeqType.GetField("Empty");. But in this case, the field is readonly, and it is unconditionally set above the usage.

F:\git\runtime\src\libraries\System.Private.Xml\src\System\Xml\Xsl\IlGen\GenerateHelper.cs(99,17): Trim analysis warning IL2080: System.Xml.Xsl.IlGen.XmlILStorageMethods.XmlILStorageMethods(Type): The requirements declared via the 'DynamicallyAccessedMembersAttribute' on the field 'System.Type System.Xml.Xsl.IlGen.XmlILStorageMethods::SeqType' don't match those on the implicit 'this' parameter of method 'System.Type.GetMethod(String)'. The source value must declare at least the same requirements as those declared on the target location it's assigned to [F:\git\runtime\src\libraries\src.proj]
F:\git\runtime\src\libraries\System.Private.Xml\src\System\Xml\Xsl\IlGen\GenerateHelper.cs(116,13): Trim analysis warning IL2080: System.Xml.Xsl.IlGen.XmlILStorageMethods.XmlILStorageMethods(Type): The requirements declared via the 'DynamicallyAccessedMembersAttribute' on the field 'System.Type System.Xml.Xsl.IlGen.XmlILStorageMethods::SeqType' don't match those on the implicit 'this' parameter of method 'System.Type.GetMethod(String,Type[])'. The source value must declare at least the same requirements as those declared on the target location it's assigned to [F:\git\runtime\src\libraries\src.proj]
F:\git\runtime\src\libraries\System.Private.Xml\src\System\Xml\Xsl\IlGen\GenerateHelper.cs(115,13): Trim analysis warning IL2080: System.Xml.Xsl.IlGen.XmlILStorageMethods.XmlILStorageMethods(Type): The requirements declared via the 'DynamicallyAccessedMembersAttribute' on the field 'System.Type System.Xml.Xsl.IlGen.XmlILStorageMethods::SeqType' don't match those on the implicit 'this' parameter of method 'System.Type.GetMethod(String,Type[])'. The source value must declare at least the same requirements as those declared on the target location it's assigned to [F:\git\runtime\src\libraries\src.proj]
F:\git\runtime\src\libraries\System.Private.Xml\src\System\Xml\Xsl\IlGen\GenerateHelper.cs(104,17): Trim analysis warning IL2080: System.Xml.Xsl.IlGen.XmlILStorageMethods.XmlILStorageMethods(Type): The requirements declared via the 'DynamicallyAccessedMembersAttribute' on the field 'System.Type System.Xml.Xsl.IlGen.XmlILStorageMethods::SeqType' don't match those on the implicit 'this' parameter of method 'System.Type.GetMethod(String)'. The source value must declare at least the same requirements as those declared on the target location it's assigned to [F:\git\runtime\src\libraries\src.proj]
F:\git\runtime\src\libraries\System.Private.Xml\src\System\Xml\Xsl\IlGen\GenerateHelper.cs(109,17): Trim analysis warning IL2080: System.Xml.Xsl.IlGen.XmlILStorageMethods.XmlILStorageMethods(Type): The requirements declared via the 'DynamicallyAccessedMembersAttribute' on the field 'System.Type System.Xml.Xsl.IlGen.XmlILStorageMethods::SeqType' don't match those on the implicit 'this' parameter of method 'System.Type.GetMethod(String)'. The source value must declare at least the same requirements as those declared on the target location it's assigned to [F:\git\runtime\src\libraries\src.proj]
F:\git\runtime\src\libraries\System.Private.Xml\src\System\Xml\Xsl\IlGen\GenerateHelper.cs(112,13): Trim analysis warning IL2080: System.Xml.Xsl.IlGen.XmlILStorageMethods.XmlILStorageMethods(Type): The requirements declared via the 'DynamicallyAccessedMembersAttribute' on the field 'System.Type System.Xml.Xsl.IlGen.XmlILStorageMethods::SeqType' don't match those on the implicit 'this' parameter of method 'System.Type.GetField(String)'. The source value must declare at least the same requirements as those declared on the target location it's assigned to [F:\git\runtime\src\libraries\src.proj]
F:\git\runtime\src\libraries\System.Private.Xml\src\System\Xml\Xsl\IlGen\GenerateHelper.cs(117,13): Trim analysis warning IL2080: System.Xml.Xsl.IlGen.XmlILStorageMethods.XmlILStorageMethods(Type): The requirements declared via the 'DynamicallyAccessedMembersAttribute' on the field 'System.Type System.Xml.Xsl.IlGen.XmlILStorageMethods::SeqType' don't match those on the implicit 'this' parameter of method 'System.Type.GetMethod(String)'. The source value must declare at least the same requirements as those declared on the target location it's assigned to [F:\git\runtime\src\libraries\src.proj]

To work around the linker, I am forced to introduce an intermediate variable for Type sequenceType, and use that to call .GetMethod and .GetField("Empty"). Then set the field to the variable at the end.

In this case, since the field is readonly and it is unconditionally set to a concrete Type above, I shouldn't need to be forced to inject a local variable just to make the linker happy.

[marek-safar]

@vitek-karas @MichalStrehovsky for thoughts. I suspect the flow analysis with fields is not something that we can easily support and although this special case can be supported I'm not sure it's worth it.

[MichalStrehovsky]

Yeah, we always model instance fields as having some unknown value initially and the analysis is strictly additive (there can only be more potential values assigned to a variable - we never remove from the set; the analysis is flow-insensitive).

For a moment I thought we could assume the initial value is null in a constructor for readonly field, but even there things get complicated because constructors can be chained.

class Foo
{
    readonly Type _type;

    public Foo()
    {
        _type = typeof(X);
    }

    public Foo(int x)
        // Chain the other constructor so that _type is not actually null.
        : this()
    {
        if (x > 0)
        {
            _type = typeof(Y);
         }

        // The analysis is flow-insensitive, so it models things the same no matter
        // if this GetMethod call is in the if branch above or not. The result of analysis will be the same.
        _type.GetMethod("Gotcha");
    }
}

We would need to make the analysis flow sensitive first and that's a bigger workitem.

[vitek-karas]

Note that part of the analysis is already flow sensitive. Constant propagation works across multiple method calls and thus is flow sensitive (the methods have to be processed in the right order). I haven't thought about this case too much, but maybe a similar approach could be done for this problem.