dotnet / csharplang

The official repo for the design of the C# programming language
11.02k stars 1.01k forks source link

[Proposal]: `readonly` parameters in primary constructors for non-record types #8105

Closed KathleenDollard closed 3 weeks ago

KathleenDollard commented 3 weeks ago

readonly parameters in primary constructors for non-record types

Summary

This proposal isolates one part of the Readonly parameters proposal for separate consideration.

Motivation

There is currently no way to fully mimic the scenario of a constructor that sets readonly fields, when using primary constructors for non-record types.

A significant piece of feedback on primary constructors in non-record types is that users would like to be able to ensure that primary constructor parameters are not modified. The scope of such parameters is much larger, so the danger of accidental modification is much higher. Some users have determined that the need for readonly is sufficient that they cannot use primary constructors for non-record types.

We therefore propose allowing readonly as a parameter modifier for primary constructors in non-record types.

Record types have their own behavior for primary constructors because records intentionally have their own approach to common operations and are excluded from this proposal.

Non-constructor methods and explicit (non-primary constructors) have been successfully in use for a long time without readonly and we have concerns about adding this behavior. These scenarios are also excluded from this proposal.

Detailed design

readonly would be a legal modifier on primary constructors on non-record types.

readonly parameters in non-record classes would not be changeable. The value passed as the primary constructor argument would be immutable.

The approach of disallowing change during construction is consistent with the primary constructor being an invisible constructor without a body. It leaves open the door for a readonly modifier in other scenarios - such as the parameters to methods or local readonly values. It also may be the most logical approach in a future world with primary constructor bodies.

This approach leans into primary constructor parameters being first and foremost parameters, over an approach that treats them more like fields.

Drawbacks

Modifying values

Primary constructor arguments could not be modified, even simple things like trimming strings.

If normal constructors are combined with primary constructors, and the other constructors modified a value, the primary constructor parameter could not be treated the same way.

Both of these scenarios could be handled by non-readonly parameters, or by not using primary constructors.

There might be the possibility of an init scope in the future.

Interaction with a property request

There has also been interest in a way to indicate that a property should be created for primary constructors in non-record types. readonly does not appear to block that future, but we should ensure that.

Alternatives

Do nothing.

Allow primary constructor parameters for non-record types to be modified during construction. While this appears inconsistent with a possible future with readonly parameters on methods and non-primary constructors, that possible future is quite controversial.

Unresolved questions

Design meetings

https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-05-08.md#readonly-for-primary-constructor-parameters

CyrusNajmabadi commented 3 weeks ago

would be immutable.

I would not use this word. It def implies too much. We should use existing readonly terminology from teh spec around things like fields and read-only locals (like you get in a from/using/foreach/etc.).

CyrusNajmabadi commented 3 weeks ago

Primary constructor arguments could not be modified, even simple things like trimming strings.

I don't understand this. That's like saying that readonly fields have a drawback that they are readonly.

CyrusNajmabadi commented 3 weeks ago

Honestly, since shipping, it feels like the feedback on this died way down. I'm def curious if this was more an initial reaction to this feature, vs a a sustained and significant problem warranting change.

HaloFour commented 3 weeks ago

Primary constructor arguments could not be modified, even simple things like trimming strings.

I don't understand this. That's like saying that readonly fields have a drawback that they are readonly.

I can kind of understand this. It depends on whether this feature is specific to primary constructors, or it's a stepping stone to readonly parameters in general. If it is the former, then I agree, it makes sense that the parameter remains reassignable during initialization. However, if it is the latter, the expectation would be that a readonly parameter to a constructor is itself readonly as it is disconnected from any fields that may or may not exist.

public class C {
    public readonly int y;

    public C(readonly int x) {
        x++; // I would expect this to be an error
        y = x;
    }
}
HaloFour commented 3 weeks ago

Honestly, since shipping, it feels like the feedback on this died way down. I'm def curious if this was more an initial reaction to this feature, vs a a sustained and significant problem warranting change.

Or you just kicked the hornets' nest suggesting that nothing needs to happen here. 😅

333fred commented 3 weeks ago

LDM discussed this today. We have rejected the narrow version of this proposal. We have not rejected readonly parameters in general, we have only rejected doing just primary constructors parameters and no other parameters.

mungojam commented 3 weeks ago

Honestly, since shipping, it feels like the feedback on this died way down. I'm def curious if this was more an initial reaction to this feature, vs a a sustained and significant problem warranting change.

For me, that's because I understood it would be strongly considered for .net 9 given how much of a fuss we made on it for .net 8.

It's definitely causing a hassle/annoyance not having it regularly. We're stuck between getting the full code reduction, but accepting less immutability guarantees than we had before, or only getting a partial code reduction by keeping the original read-only fields.