# C# Language Design Meeting for January 5th, 2022

## Agenda

1. [Required Members](#discussion)

## Quote of the Day

- ![OHI Mark!](LDM-2022-01-05-OHIMark.jpg "Image of Johnny from The Room in the roof scene, saying 'OHI Mark'")
- "`System.Runtime.CompilerServices.HemlockAttribute`"

## Discussion

https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md

Today was entirely devoted to going over open questions in required members, validating the recent specification
updates and debating the restrictions introduced.

### Required properties accessibilities

In our initial version of the proposal, we had a complex meta-language around contracts, allowing individual members
to be added or removed on a per-constructor basis. However, our recent design work has sought to pare back the complexity
of the feature, which has lead to needing to revisit the topic of members that cannot be set by consumers of a type.
Since constructors can only opt out everything or nothing, they don't have the tools to effectively deal with less visible
members. Ultimately, we see 4 options for dealing with this:

1. Disallow the scenario. This is the most conservative approach, and the rules in the OHI are currently written with
this assumption in mind. The rule is that any member that is required must be at least as visible as its containing type.
2. Require that all constructors are either:
    1. No more visible than the least-visible required member.
    2. Have the NoRequiredMembersAttribute applied to the constructor. These would ensure that anyone who can see a
    constructor can either set all the things it exports, or there is nothing to set. This could be useful for types that
    are only ever created via static Create methods or similar builders, but the utility seems overall limited.
3. Readd a way to remove specific parts of the contract to the proposal, as discussed in LDM previously.
4. Require that the derived constructor must set any properties that are not at least as visible as itself.

Of these, we are most interested in 1 or 2. 3 is readding complexity we specifically wanted to remove from this proposal,
and while we can add it back at a later date, we're concerned about feature creep for the initial version. 4 has some 
concerning impacts on implicit changes to the public API of a constructor: by making a member more visible than it is
currently, a user could unintentionally expose requirements to their consumers they didn't mean to.

For 1 and 2, we think either would be acceptable. 2 is more flexible, but we also don't think the scenario it would cover
is important to the feature. We think starting with the most conservative set of rules is acceptable, and keeping 2 or a
more complex meta-langauge in our back pocket for future requests.

#### Conclusion

We will go with option 1: all required members must be at least as visible as their containing type.

### Hiding

We verified the section of the specification on hiding, now that we have a decision on the principle for accessibility.
Users cannot access hidden members in an object initializer, so we agree that required members cannot be hidden. In the
future, if we come up with some syntax for accessing a specific (potentially hidden) member, we should keep this scenario
in mind.

#### Conclusion

Specification upheld. We will disallow hiding required members.

### Overriding

#### Adding/removing `required` on override

In order to preserve our future design space on contract modifications, we want to disallow removing `required`ness on
override. We don't think adding `required` is a problem, as contracts are already additive today, but removing on a
per-member basis is not supported in the other aspects of the proposal, so we think we should disallow here too.

##### Conclusion

Adding `required` on override is allowed. If the overridden member is `required`, the overridding member must also be
`required`.

#### Overriding `required` virtuals

We have another interesting question on overriding:

```cs
abstract class Base
{
    public required abstract int Prop1 { get; set; }
    public required virtual int Prop2 { get; set; }
}

class Derived : Base
{
    public required override int Prop1 { get; set; } // This is probably fine?
    public required override int Prop2 { get; set; } // Is this ok?
    
    public void ToString()
    {
        _ = base.Prop1; // Already illegal
        _ = base.Prop2; // What happens, was base.Prop2 initialized?
    }
}
```

There is a general anti-pattern in C# around overriding a virtual property that has storage, and not delegating to that
original storage. In particular, the above code overrides Prop2, and then explicitly goes to the base storage. While this
is certainly an anti-pattern, we don't think `required` is the place to solve it. There are other logic errors that can result
from this case, all of which relate to stale or incorrect data from accessing the wrong storage location. A warning or
analyzer would be better suited to addressing that problem in general, and `required` should not be opinionated beyond that.

##### Conclusion

No specific restrictions. An analyzer can generally warn about this type of anti-pattern.

### Metadata Representation

#### `RequiredMembersAttribute`

Finally today, we looked through the proposed metadata representation, verifying the general design. We have two proposals
for indicating the required members in a type:

1. Put a single `RequiredMembersAttribute` on the type, listing every member of that type that is required.
2. Put a `RequiredMemberAttribute` on the type, and then put a `RequiredMemberAttribute` on every member in that type that
is required.

We're happy that both of these preserve our desire for _additive_ contracts: they both require walking up a type chain to
get the full list of required members for a type, and additions to a base type do not require downstream changes. We think
the version with a single attribute is slightly more attractive, so we'll go with that one, but with a small modification
to the lookup rules. The current proposal says that the lookup rules should be standard member lookup: we think this is
needlessly complex, and we can simply look at members defined in the current type. Future contract modification attributes
might need more complicated lookup rules, but we can address that when we get there.

##### Conclusion

Design 1 is upheld, and the member lookup rules are simplified to just looking at members of the current type.

#### Constructor protection

One of our desires for this feature is that removing required members is _not_ a binary breaking change. This means that our
usual protection trick, putting a `modreq` on any constructor that must set required members, isn't going to work for this
scenario; if a user removed the last `required` member from a type, that constructor would no longer have a `modreq` on it,
breaking binary compat. We therefore need to look at a different method of protecting required members. Our last prior art
here is in ref structs, which put an `ObsoleteAttribute` on the type with a specific message, that newer compilers could
recognize and specifically ignore. This solution is imperfect, however: `ObsoleteAttribute` can only be applied once, and if
the user is in an obsolete context (such as in an obsolete type/member) that obsolete marker is ignored. While some users do
this intentionally to work around restrictions, it's also very easy to accidentally hit just by virtue of working in a
legitimately actually obsolete member. We think this is unfortunate, and that we'd like to add a new trick to our toolbox for
these scenarios. We will look at adding a new attribute to the runtime for the express use of the compiler to "poison"
something without affecting binary compat, and have the compiler start recognizing this attribute and specifically disallowing
it. This won't help us for required members immediately: we'll need to apply both this new poison attribute and
`ObsoleteAttribute`, and may need to continue doing so for some time. But we regret not adding such an attribute for ref
structs then, so we should go ahead and do this now.

##### Conclusion

We will apply `System.ObsoleteAttribute` to constructors of types with required members, and work with the runtime team and
other .NET language teams to add a new attribute specifically for poisoning types/members without affecting binary compat.
