-
Notifications
You must be signed in to change notification settings - Fork 1k
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]: Field and value as contextual keywords #7964
Comments
Does 'simple name' just mean 'pure identifier'? Or does it include type arguments, a-la |
fwiw, i don't love this. We don't allow |
We might want to do this for event accessors with |
We should be clear and explicit as to what extent those bodies extend to. For example, do they apply once you're inside a lambda/local funciton within an accessor? Since the spec allows for Note: the hybrid approach again slightly unnerves me. Primarily because i think getting it right will be hard. Here are examples of places we'd need to still allow
My compiler hat makes me just want this to always be a keyword (like |
True. But as the fix there is trivial as well, i would ask if that's important enough. Def something to get a read of people on. Personally, i think taht code would now look weird. If |
I'm 100% in favor of taking this approach. My preference is to tweak slightly. But that tweak is def optional, and this proposal is great even without that :) |
Implementation note: We were able to squeeze out a few more bits to store on nodes. This means we can store the contextual information about where Specifically, we need to make sure that there is no issue transforming:
into
That node (and its parents) for the |
Good point of clarification, thanks. I don't think it has real-world impact, but we do need to decide one way or the other. |
I understand, and the "Broaden the context" alternative was intended to acknowledge that issue. With That said, perhaps the parameter and local declarations wouldn't be so bad to break. Presumably, if you have such a declaration in existing code, you'd also 9 times out of 10 have at least one usage of the declared variable that would also be broken. So it'd be rare for an otherwise "clean" property to break only because of declarations. Once you're in there fixing, you would just fix the declaration too (or the automated fix would). |
Yes they do. If that is not clear enough, it should be! ;-) I don't think the
According to this proposal,
I get the compiler-hat thinking. I just worry it's too breaking. |
Since |
If public int X
{
set
{
var n1 = nameof(value); // no error
var n2 = nameof(this); // CS8081
var n3 = nameof(int); // CS1525
}
} |
That's a really good point, since |
That feels reasonable to me. But, what should |
Would that problem be solved if it is recommended to use |
@HaloFour I agree that @ufcpp There would have to be throw helpers for every overload of every argument exception type, like ArgumentOutOfRange, and potentially user-defined argument exceptions. |
I suppose this change would not affect the processing of |
I added a few things to the proposal text ahead of Monday's LDM, primarily based on feedback here on this issue - thanks! ❤️ I added:
|
@MadsTorgersen Could we consider treating |
I do not like this. In my opinion, the team's obsession with using contextual keywords instead of breaking syntax is wrong. It causes sharp edges ( Rust shows that breaking changes can be done safely through editions, and we already have such a thing: the In addition, there's talk here already about how this "contextual keyword" isn't actually contextual, but would break things. If I have |
That wouldn't work no matter what approach we took. In other words,
I don't get how the two clauses relate. You say you do not like what is being proposed here, and that we should break syntax instead. But that's very much what this proposal does. It breaks syntax. Existing code changes meaning (there's a breaking change), and you would have to change your code then to get the old meaning. |
You ignored my point about As for how they relate: I also understand that this proposal includes breaking changes. My gripe is that syntax evolution tends to be halted in the name of backwards compatibility. How long has this issue been requested? Eight years (3.5 if you count the age of |
I don't know how the point remains, when you're both stating but then discarding the most important part.
The point of this proposal is that that's not the case. We're literally considering syntax evolution, achieved through breaking back-compat.
I don't know what that means. This proposal literally breaks the syntax here to make this possible. I don't know what you mean by "going all the way" here. |
If |
@HaloFour Right. To us, |
I still want my |
I have high confidence that given enough time, we will have |
In Foof we Trust. |
If public string P { get => nameof(field); set => /*something that doesn't use 'field'*/ } |
It would make sense to me to disallow |
Constraints are easier to lift than to add also, but I wouldn't think there's too much use for it. I seem to remember that |
How about making properties into a scope under the class? Maybe even allow methods in them.
Pros:
|
Closing out. #8635 covers |
Field and value as contextual keywords
Summary
This proposal explores introducing
field
as a contextual keyword instead of an implicit parameter, and changingvalue
to similarly be a contextual keyword.Motivation
As we're adding "Field access in auto-properties" to C# we've been aiming at having
field
be an implicitly declared parameter in property accessors, just asvalue
already is in property set and init accessors.Issue #7918 and subsequent discussion explores the breaking change aspects of this design, along with proposed default fixes.
Here we propose instead adding
field
as a contextual keyword. It would only be a keyword when used as a simple name inside a property accessor. This would result in fewer breaking changes and simpler "default fixes" for those breaks that do occur.Declarations of
field
as a local variable would no longer be a breaking change:This would eliminate the arguably most gnarly automated default fix, in that such locals would have had to be renamed, leading to a non-obvious choice of which name to change them to.
For the remaining breaks, the implicit parameter approach involves changing existing occurrences of
field
as a simple name to a member access, possibly rewriting primary constructor parameters to introduce an explicit field declaration. Instead, withfield
as a contextual keyword all those occurrences can simply be replaced with@field
.The proposal also suggests changing
value
to a contextual keyword for consistency. This is a breaking change, but likely a very small one occurring only in niche situations. The default fixes for those are simple, changingvalue
to@value
or vice versa.The idea for this proposal came from this comment on issue #7918.
Detailed design
Contextual keywords
Whenever
field
orvalue
occurs as a simple_name within a set, init or (forfield
) get accessor of a property, it is considered a contextual keyword. This allows code to refer both to the special parameters (usingfield
andvalue
) and to the nearest explicit declaration of that name (using@field
and@value
).From a specification point of view, we would make the following changes:
field
andvalue
to the list of contextual keywords in the Keywords section.value
and add a description offield
.Note that this narrowly treats
field
andvalue
as keywords only when used as simple names. It does not prohibit declarations of local variables and parameters with those names within applicable accessors, but access to those variables and parameters would have to use@
.Some existing contextual keywords are reserved throughout a given region of code, e.g. await expressions, which reserve the
await
identifier throughout the body of an async function. Others are only keywords in a very specific syntactic position, e.g.set
andget
. This proposal is a combination of both those two flavors:field
andvalue
are only keywords when used as simple_names and even then only within applicable property accessor bodies.Breaks and fixes
field
as a simple name within the bodies of existing property accessors: These occurrences would now be contextual keywords and denote a field_access. This break occurs no more or less frequently than the equivalent break with the implicit-parameter design. The default fix is to change this occurrences to@field
.The fix is significantly simpler than the default fix in the implicit-parameter design, which is to replace with a member access. The receiver of that member access would vary with the specifics -
this
or a type - and also may lead to generation of a new field, whenfield
is a captured primary constructor parameter.The proposal avoids breaking changes to declarations of locals or parameters named
field
within a property body, since those would continue to be allowed with the same meaning. However, see "Alternatives" below.value
as a simple name within the bodies of existing property set or init accessors: As long asvalue
already refers to the implicit parameter of the accessor this is not a break. However, if it refers to a different variable it would now be a contextual keyword denoting a value_access. Current C# rules prevent shadowing of parameters and local values except within nested - local or anonymous - functions, so this break can only occur within a nested function of the accessor. Furthermore, the programmer will have had to deliberately declare a parameter or local variable with the namevalue
, despite the fact that it shadows the implicit parameter to the property and prevents access to it. All in all, while this is certainly possible, it is likely very rare. The fix is to replace with@value
.nameof(value)
within the bodies of existing property set or init accessors: This is an exception to the previous category.nameof
accepts a simple_name, sovalue
would now be a keyword, but a keyword is not allowed here. Adding an@
wouldn't help, sincevalue
may no longer be declared as an identifier. The fix is to replace with the "value" string itself. However, see "Alternatives" below.@value
as a simple name within the bodies of existing property set or init accessors: If@value
currently refers to the implicit parameter of the accessor, it will no longer do so. Instead it will denote the nearest explicit declaration ofvalue
, yielding an error if not found. Either is a break. It seems unlikely that existing code would prefixvalue
with@
, since it does not currently change the meaning in any way, so this is probably quite rare. The fix is to replace@value
withvalue
.Here are representative examples of the breaks and fixes:
Drawbacks
Different breaks: Compared to the field-as-implicit-parameter approach, the proposal avoids a class of breaks around
field
(local variable declarations within the accessor). On the other hand it introduces new breaks aroundvalue
, though they are arguably much more rare.More churn: By subtly changing the semantics of
value
the proposal arguably introduces more conceptual churn, even if the actual breaks might be fewer. On the other hand, many people probably already thinkvalue
is a contextual keyword, since it is "just there" without declaration and actually colors as a keyword in popular editors.Alternatives
Stick with implicit-parameter design: We could choose to keep the current design. We'll need to decide which names to generate for the default fix for local variable declarations of
field
.Don't change
value
: We could choose to adopt only thefield
part of the proposal. This is a bit less breaking, while introducing a subtle difference between the workings offield
andvalue
.Broaden the context: The fact that
field
andvalue
are still allowed as identifiers within applicable property bodies except as simple names might allow for confusing scenarios. On the one hand it is probably nice thatthis.field
doesn't break. However, is it weird that you can declarevar field = 4;
in your accessor body, yet you need to say@field
to refer to that variable?Any broadening of the context would lead to further breaks, so there's a very significant trade-off there. However, a sensible landing point might be for local and parameter declarations of
field
andvalue
to "refer to the keyword" and thus be an error, similar to howvar this = 5;
is an error today. This feels like it targets all uses (declare and consume) of local variables/parameters equally, for a more balanced/less confusing experience. It would reintroduce breaks around variable declarations similar to the "implicit-parameter" design, but the fixes remain simple: add@
in front.Handle
nameof
gracefully: We could specially allownameof(value)
andnameof(field)
on the keywords, producing the obvious strings as a result.Unresolved questions
No known open questions at this point.
Design meetings
https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-03-04.md#breaking-changes-making-field-and-value-contextual-keywords
Issue #7918 was discussed here:
The text was updated successfully, but these errors were encountered: