Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 of 4 tasks
KathleenDollard opened this issue May 8, 2024 · 7 comments
Closed
1 of 4 tasks
Milestone

Comments

@KathleenDollard
Copy link
Collaborator

KathleenDollard commented May 8, 2024

readonly parameters in primary constructors for non-record types

  • Proposed
  • Prototype: Not Started
  • Implementation: Not Started
  • Specification: Not Started

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

HaloFour commented May 8, 2024

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
Copy link
Contributor

HaloFour commented May 8, 2024

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 333fred added this to the Likely Never milestone May 8, 2024
@333fred
Copy link
Member

333fred commented May 8, 2024

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.

@333fred 333fred closed this as not planned Won't fix, can't repro, duplicate, stale May 8, 2024
@mungojam
Copy link

mungojam commented May 9, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants