Support SetsRequiredMembersAttribute

[333fred]

The SetsRequiredMembersAttribute prevents the compiler from checking the required member list of a type when calling that constructor, and suppresses any errors from a base type's list being invalid.

Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
Test plan: #57046

[333fred]

@RikkiGibson @jcouv please take a look.

[333fred]

@jcouv @RikkiGibson please take a look

[RikkiGibson]

Just had a few comments so far. Haven't looked at the tests yet--possible those will hold answers to some of the questions anyway. Will review the tests a little later this afternoon.

[333fred]

@RikkiGibson @jcouv please take a look.

[RikkiGibson]

LGTM aside from some minor comments.

[333fred]

@jcouv for the second review please.

[jcouv]

Done with review pass (iteration 7)

[jcouv]

Just curious, what was the problem you ran into that the used assemblies leg detected?

---

In reply to: 1083559918

[333fred]

Just curious, what was the problem you ran into that the used assemblies leg detected?

d9da29b. It wasn't happy about mismatched collection of dependencies.

[333fred]

@RikkiGibson @jcouv please take another look.

[RikkiGibson]

I was contemplating whether it's reasonable for constructors to delegate to 'SetRequiredMembers' constructors, without being 'SetsRequiredMembers' themselves. For example, test RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_08.

class Base
{
    public required string Prop1 { get; set; }

    [SetsRequiredMembers]
    public Base() { Prop1 = "a"; }
    
    public Base(int unused) { }
}

class Derived : Base
{
    public required string Prop2 { get; set; }

    // maybe compiler needs to emit [SetsRequiredMembersOf(typeof(Base))] here?
    public Derived() : base() { }

    // maybe compiler needs to emit [SetsRequiredMembersOf(typeof(Base))] here?
    public Derived(int unused) : this() { }
}

class Program
{
    static void Main()
    {
        // how do we realize the user doesn't need to set Prop1?
        var derived = new Derived() { Prop2 = "a" };
        
        // we seem even more at sea here :)
        var derived2 = new Derived(1) { Prop2 = "a" };
    }
}

The metadata encoding spec doesn't appear to be able to represent this as written. Did I miss something?

I could imagine complex type hierarchies peppering in 'SetsRequiredMembers' at arbitrary levels. It feels like representing that accurately would require tracking the most-derived type in the hierarchy which has the attribute. When the compiler binds a constructor it can determine which base constructor it ultimately delegates to, perhaps using a similar mechanism as is currently used to prevent cycles in constructor initializers.

However, it feels to me like it would be a very reasonable starting place to require any constructors which delegate to a 'SetsRequiredMembers' constructor to also have 'SetsRequiredMembers'. This would make it an all-or-nothing thing down the hierarchy to start with, and I think it leaves the space open for us to add something like what I described above later on.

[333fred]

@RikkiGibson we could do that. My initial thought was, however, not to do any form of restrictions. In short, in your example, there is no way to know that Prop2 should not be set from the derived constructors. I'm ok with considering a simplification of this, however, as that would significantly reduce the complexity of the changes in NullableWalker here.

[333fred]

@RikkiGibson @jcouv please take another look.

[jcouv]

LGTM Thanks (iteration 12)

[RikkiGibson]

It seems like the code paths which handle SetsRequiredMembers on base but not this could be eliminated.

[333fred]

I'm not sure what you mean by this? Can you elaborate?

[RikkiGibson]

I was wrong about this. I recall now that you're still expected to get a different initial state when the 'this()' constructor you chain to has SetsRequiredMembers, versus the 'base()' constructor you chain to having it.

For some reason I expected a further simplification in NullableWalker to fall out of the decision to require SetsRequiredMembers to be used on constructors which chaining to a SetsRequiredMembers constructor.

[RikkiGibson]

LGTM with one comment which can be addressed in a subsequent PR if you prefer.