Select better auto-prop backing field type to help with devirtualization

[stephentoub]

Given code like:

public class C
{
    public static A Shared { get; } = new B();
}
public abstract class A {  public virtual void Foo(); }
public sealed class B : A { public override void Foo(); }

the compiler currently produces an equivalent for the Shared property like:

    private static readonly A <Shared>k__BackingField = new B();
    public static A Shared => <Shared>k__BackingField;

If the backing field were instead created as type B instead of as A, e.g.

    private static readonly B <Shared>k__BackingField = new B();
    public static A Shared => <Shared>k__BackingField;

it would be easier for the JIT to devirtualize calls to C.Shared.Foo(); It seems like the C# compiler could do this fairly easily, using the type of the RHS expression rather than the type of the property, as long as there isn't a conversion/boxing/etc. involved.

[benaadams]

As seen in dotnet/coreclr#20637 (comment)

[jaredpar]

Spurred by @AustinWise proposing #30992 as a fix for this I followed up internally with our language design team to see if they were okay with the change. It's a border line spec'd behavior as the spec does discuss the type of emitted fields for auto-implemented properties. The feedback was that LDM does not oppose this change. It's an optimization and unobservable.

However in the discussion we found that implementing this is a lot more complex than we originally realized. Particularly because the property can be initialized in two different places: inline and in the constructor body.

public class E { 
  public object Value { get;  } = "example";
  public E() { 
    Value = new object();
  }
}

This is problematic because it creates a fundamental ordering problem in the compiler. In order to bind the constructor properly you need to have the property / field types set. In order to set the property / field types you need to bind the constructor.

We discussed this a bit and thought about all of the following options as ways to still make this doable:

Delay setting the field type

Essentially try and avoid setting the type of the backing field until after we've bound all of the constructors. That would let us make the decision with all of the right information in hand.

This isn't really feasible. From the API perspective it is possible for a consumer to inspect the properties, and their backing field, before we bind the constructor. This means we'd actually have to change the logic which calculates the field type to eagerly bind the constructors in the background. That is feasible ... but it's also a lot of work. Probably significantly more than justifies the wins that we would get.

Third type

Give auto-implemented properties three types: property, field which always matches property and the emit field which has this optimization. The emit field type would be calculated during binding and possibly not even exposed publicly.

This was rejected because the feeling was that it meant the symbol API was effectively lying to the customer at this point. That goes against our design goals.

Casting

Rather than emitting the field as the derived type, emit it as the declared type and insert a cast into the property getter for the derived type.

    private static readonly A <Shared>k__BackingField = new B();
    public static A Shared => (B)<Shared>k__BackingField;

This would be pretty doable as there is no need to publicly expose this cast type, it's an implementation detail. At the same time though it would add type checks to every single get and hence was rejected.

Overall we didn't really see a clean and simple way to do this. If there are other ideas I'd love to hear and consider them. The ones we found just aren't palatable.

Really do appreciate the effort at trying to get this done. But after digging into the design it seems like we should probably close this out.

Couple other items we came across when thinking through this:

• Generic type parameters are tricky. Imagine a T which is constrained to class, IEnumerable and is always initialized with List<int>. Emitting a backing field as List<int> means that in order to implement the property getter we essentially have to do return (T)(object)_backingField. Messy. Probably wouldn't want to optimize this scenario.
• Expression type of assignment. When initializing an auto-implemented property in the constructor the assignment goes directly to the backing field. The type of this assignment though needs to remain the declared type of the property, not the optimized type of the backing field.
• The discussion also did come up as to whether this could be done for VB or not. Given that VB directly exposes the backing field it definitely cannot be done: clear back compat change.

[stephentoub]

Thanks for investigating.

[AustinWise]

Oh, that's a bummer. I had no idea you could assign a readonly auto property in the constructor.

I already had to add delaying binding the field type, as binding the initializer sometimes requires binding the property, causing infinite recursion. This delayed binding part of my PR was the most questionable part. Trying to also poke around constructors to find assignments seems overly complex compared to the benefit of this optimization.