Getting to the Subclass of a MemberExpression using Type is not trim compatible

[mikes-gh]

Consider the following code with an inherited model that I want to retrieve an attribute from. The result should be "Bar". Note the attribute we want to get is on derived class.

Just accessing memberExpression.Member won't work as that returns the base class (as per the IL) I have to use Type property to get to the derived class.

However

memberExpression.Expression?.Type.GetProperty(memberExpression.Member.Name);

gives me a trim warning I cant seem to resolve.

(local variable) PropertyInfo property
'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String)'. The return value of method 'System.Linq.Expressions.Expression.Type.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. \[MudBlazor\]csharp(IL2075)

Is there anyway I can make this trim compatible 🙏 . Our library uses the Type property in a few places and we want to support trimming and reading attributes from subclassed models.

Sample code (not tested)

class TestFailingModel
{
    [Label("Foo")]
    public virtual string Foo { get; set; }
}

class TestFailingModel2 : TestFailingModel
{
    [Label("Bar")]
    public override string Foo { get; set; }
}

var model = new TestFailingModel2();
Expression<Func<T> expression = () => model.Foo;
Console.WriteLine(expression.GetLabelString());

Output should be "Bar"

public static string GetLabelString<T>(this Expression<Func<T>> expression)
{
    var memberExpression = (MemberExpression)expression.Body;
    var propertyInfo = memberExpression.Expression?.Type.GetProperty(memberExpression.Member.Name);
    return propertyInfo?.GetCustomAttributes(typeof(LabelAttribute), true).Cast<LabelAttribute>().FirstOrDefault()?.Name ?? string.Empty;
}

Reference https://github.com/MudBlazor/MudBlazor/issues/5586 https://stackoverflow.com/questions/74315845/get-propertyinfo-for-a-subclass-from-expression-in-a-trim-friendly-way?noredirect=1&lq=1

[marek-safar]

/cc @vitek-karas

[vitek-karas]

I can't really think of a way to make this trim compatible as designed above.

I find this code weird to be honest. Is it intentional that if I only change it slightly it will print out Foo:

TestFailingModel model = new TestFailingModel2();  // I just declared the type of the variable to be the base class
Expression<Func<T> expression = () => model.Foo;
Console.WriteLine(expression.GetLabelString());

The only feature I can remotely think of as being useful here is to annotate the base type of the model with DynamicallyAccessedMemberTypes(PublicProperties) and then look at the actual instance the MemberExpression operates on, cast it to the base type and call GetType on it. And then on that type query for the property. But this requires you to have the instance of the model the expression operates on, so the above would work, but if the model comes in as a parameter it won't work.

@sbomer any ideas?

High level this is a problem of abstraction over reflection. The LINQ expression trees hide the underlying reflection, but the current trimming attributes don't have a way to "annotate the abstraction" (basically this would require something like DAM attribute on the MemberExpression or something similar). Similar problem to component model - which is another abstraction on top of reflection.

I guess this could be done with source generators somehow (since it's all static type information), but that's not simple.

[mikes-gh]

Thank you for looking at this.

As an aside I don't get any trim warnings if I just use memberExpression.Member. I presume that is a scenario the trimmer can reason against?

I find this code weird to be honest. Is it intentional that if I only change it slightly it will print out Foo:

In that case I think the Expression.Type property and memberExpression.Member will both return the base class attribute so that would be correct. The main crux of this issue is that memberExpression.Member returns the base class properties which is unexpected to me. Therefore forcing us to go through hoops to get the real type of the member.

This code is about Form or Validation models. Currently the way memberExpression.Member works means you cant use any inheritance in your models which is very restrictive.

As an aside its also weird that VB compiler actually emits different IL for the same source and actually gives the child class in response to memberExpression.Member (I haven't tested this) which would be what most people expect.

Heres an example of how we use it in our For Blazor parameter. We use a lambada to bind the fields and get the Label,Validation atributes. Similar to DisplayFor in aspnetcore.

https://github.com/MudBlazor/MudBlazor/blob/f7116cba0a83b9f93d020fa8fe8393963d5d50c6/src/MudBlazor.UnitTests.Viewer/TestComponents/Form/FormValidationOverrideFieldValidationTest.razor

[mikes-gh]

cc @henon @ScarletKuro

[vitek-karas]

I don't know VB, but I know the C# compiler always emits calls to the base method, not the override (unless there are covariant returns, but that's a different story). And since the LINQ expression trees should model the IL, it probably does the same there as well.

I thought about it some more, but this is pretty hard. The problem is that if I make a small change in the above example - to avoid the call to new up the model class, it's actually perfectly valid for the trimmer to remove the property from the derived class (Since there's no visible instantiation in the program, so there's no point to keep instance methods/properties as they will never be called) - but the expression tree stuff should still work and find the attribute.

So without some way to fully annotate the system such that the annotation passes through multiple layers this is very hard to do.

Honestly our current suggestion would probably be source generators. This is a fully static problem, there's no dynamism in it at all. So have a source generator which reacts to all Label attributes and emits code to describe the owning member somewhere (after all the validation process will have some kind of internal model typically, so it can probably generate that directly).

[ScarletKuro]

Isn't this warring mainly because Expression.Type(https://github.com/dotnet/runtime/blob/215b39abf947da7a40b0cb137eab4bceb24ad3e3/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Expression.cs#L97) missing annotation?

var model = new SomeModel();
Expression<Func<T>> expression = () => model.Foo;
MemberExpression memberExpression = (MemberExpression)expression.Body;
var type = memberExpression.Expression.Type; //SomeModel type
type.GetProperty(...) //Trim warrning

But the code should be safe and not be trimmed because the SomeModel and Foo property is used.

[vitek-karas]

Isn't this warring mainly because Expression.Type missing annotation?

Technically yes, but what should the annotation be. For each instance it would need to be different based on what the needs are and what the type used can actually provide. Which is exactly what trimming does for System.Type instances (it tracks and flows annotations). To solve this problem, the trimming would have to start treating Expression values as interesting and flow annotations on them just like it does for Type. Because it is effectively a higher-level representation of Type.

The problem is that it's not 100% the same (each of these higher-level abstractions have its own quirks) and that it's not scalable. Let's say we teach the trimming tools about Expression. Great, it solves the LINQ problems, but what about component model (same problem) do we hardcode that as well? And then what about 3rd party libraries...

So far it's not clear if there's a need for a more general mechanism to handle these cases...

About the code being trim compatible: A simple example as above is compatible, but it's likely that other examples may not be. As mentioned before if the SomeModel class is actually never instantiated, then trimming can remove the Foo without annotations, because it would only see the model.Foo access, which would never happen if there are no instances of SomeModel. So Foo can be removed. But the above code breaks, because it uses reflection which the trimmer is not aware of.

Similarly the above works right now, but if it actually didn't look for a simple attribute on the property, but also wanted to do something with the setter of the property, trimmer would have actually removed the setter again because there's no use case for it.

[ScarletKuro]

A simple example as above is compatible, but it's likely that other examples may not be.

True, if you use more reflection it will not work. But was a reply to @mikes-gh and that in case of MudBlazor there is nothing to worry about because the model is always instantiated(code example) and properties are explicitly used.

[mikes-gh]

OK I have one more question because my understanding is instantiation is not enough to guarantee reflection on a type?

A clarification would really help me 🙏

My basis for understanding is taken from here. https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming?source=recommendations#unconditionalsuppressmessage

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2063",
    // Invalid justification and suppression: property being non-reflectively
    // used by the app doesn't guarantee that the property will be available
    // for reflection. Properties that are not visible targets of reflection
    // are already optimized away with Native AOT trimming and may be
    // optimized away for non-native deployment in the future as well.
    Justification = "*INVALID* Only need to serialize properties that are used by the app. *INVALID*")]
public string Serialize(object o)
{
    StringBuilder sb = new StringBuilder();
    foreach (var property in o.GetType().GetProperties())
    {
        AppendProperty(sb, property, o);
    }
    return sb.ToString();
}

[vitek-karas]

@mikes-gh Yes that's correct. For example in NativeAOT the compiler will only store metadata for things it recognized as accessed through reflection as the runtime itself doesn't need the metadata for anything. So if the property is used by the code but it's not seen as used by the reflection its metadata will be removed. This actually saves quite a bit of size.

IL trimming currently doesn't do this yet, but there are plans to implement something along those lines - for example properties are not used by runtime at all (there's nothing in IL instructions which would access a property, only its getter/setter but those are basically just methods). So if the property is not accessed through reflection it should be OK to remove it completely and only leave the getter/setter. This might seem insignificant but it's actually quite interesting size saving because there are a lot of properties in .NET and especially their names tend to be long in some cases.

Similarly in the future we might be able to save more size by removing names of things which are not used by reflection - NativeAOT already does this, IL trimming could do that as well. For example it's valid to have a field without a name in IL.

So in short, there's an important difference between thing existing because it's used at runtime and that thing being accessed through reflection. The former doesn't imply the latter. Above I was talking mostly about IL trimming current behavior where this implication still mostly works, but it's not a good idea to rely on it.

[mikes-gh]

Thanks @vitek-karas that really helped me. I keep hearing justification for ignoring trim warnings is that the type is used in the app so it wont be trimmed for relflection. I now know that this is not a good stratergy for the future (or present for aot). So trying to annotate correctly with DynamicallyAccessedMembers now is important. In terms of IL trimming of metadata is that planned for net8?

[vitek-karas]

The "App won't be trimmed" is actually not true in .NET 7 for console apps anymore. We changed the default to TrimMode=full which will trim everything - this applies to both IL trimming and NativeAOT. It is still true for other scenarios like Blazor, MAUI, ...

Re trimming metadata in IL trimming - currently there are no concrete plans, but it's basically the next big thing we can do in the trimmer to reduce size of apps across the board (and since NativeAOT already does it, it would make sense and probably won't cause too much trouble).

[MichalStrehovsky]

IL trimming currently doesn't do this yet, but there are plans to implement something along those lines

It already does with IL trimming in some cases:

Reporter.Report(typeof(ClassWithUnusedProperties));

ClassWithPropertiesUsedFromCode.SomeProperty.ToString();
Reporter.Report(typeof(ClassWithPropertiesUsedFromCode));

typeof(ClassVisiblyReflectedOn).GetProperties();
Reporter.Report(typeof(ClassVisiblyReflectedOn));

static class Reporter
{
    // Suppress trimming warning. The reflection cannot be reasoned about; but that's the point.
    [UnconditionalSuppressMessage("", "IL2070", Justification = "That's the point")]
    public static void Report(Type t)
    {
        Console.WriteLine($"{t}:\t{(t.GetProperties().Length == 0 ? "Trimmed" : "Not trimmed")}");
    }
}

class ClassWithUnusedProperties { public static int SomeProperty => default; }
class ClassWithPropertiesUsedFromCode { public static int SomeProperty => default; }
class ClassVisiblyReflectedOn { public static int SomeProperty => default; }

This will print 0 1 1 in .NET 6, but 0 0 1 in .NET 7.