Revision of §13.6.2 *Local variable declarations* to fix errors and tidy up #940
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…idy up. This clause has evolved over time as first implicitly typed, and now in v7 ref locals, have been introduced; the result ending up as something worthy of Heath Robinson or Professor Branestawm ;-) The result alas is also not entirely correct, the initial version was (almost), but subsequent kinds of local variables don't follow exactly the same rules... As a simple example of what ended up going wrong is the assertion: > A local variable declaration that declares multiple variables is equivalent to multiple declarations of single variables with the same type and *ref_kind*. and its associated example. This was (almost) correct originally, but is not correct for implicitly typed or ref local declarations where the initialization cannot be split from the declaration. (It was “almost” is it didn't allow for *for_initializer* or *resource_acqusition* cases where such a split is not possible either.) The rework divides local variable declarations into three subgroups, there really are four but we can get away with three (whether we should can be debated). These groups are directly represented in the reworked grammar, which also replaces a lot of conditional English prose. The clause starts with the general rules and then has a section for each subgroup detailing just the rules that apply in that case. The changes have been kept to the §13.6.2 clause; there are related issues in at least §7.7.1 Scopes:General and §13.14 The using statement – neither of these for example define the scope of a variable introduced in a *resource_acqusition*. Such issues are left for future (pre-v8) work!
Nigel-Ecma
added
the
meeting: discuss
Nigel-Ecma
requested review from
jskeet,
BillWagner,
gafter,
MadsTorgersen,
RexJaeschke and
ericlippert
… by *foreach_statement*. This may point to a revision being required there but it has just been added to §13.9.5 where it is used. So this PR now impacts 2 clauses…
|
This inspired me to play with |
BillWagner
reviewed
Sep 19, 2023
There was a problem hiding this comment.
I also haven't reviewed which current issues are fixed by this PR.
That said, I like the updated description of these features.
The grammar check passed, so the updated grammar didn't break anything in the grammar elsewhere.
This PR should be updated to replace any of the removed grammar productions in text in the standard (for instance, local_variable_declarator occurs in a number of places in basic-concepts, other locations in statements, and one in variables.
I didn't check other removed or updated grammar elements.
…dealing with thge linkages between them. In particular: - The term *declarator* has been turned into the defined term ***declarator*** thus allowing other clauses which used to refer to *local_variable_declarator* to use it instead and simplify language at the same time. - The wording of the scope of a `for` variable has been made consisent between Statements & Basic Concepts – just word smithing using a blending of the two, no change in semantics. - The production *local_variable_type* has been moved from `foreach` to the textually earlier declaration expression, just seems more logical. I'll now remove the "draft" status of the PR, go at it folks!
jskeet
approved these changes
Sep 20, 2023
jskeet
reviewed
Sep 20, 2023
Co-authored-by: Jon Skeet <[email protected]>
MadsTorgersen
approved these changes
Sep 20, 2023
BillWagner
approved these changes
Sep 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This clause has evolved over time as first implicitly typed, and now in v7 ref locals, have been introduced; the result ending up as something worthy of Heath Robinson or Professor Branestawm ;-) The result alas is also not entirely correct, the initial version was (almost), but subsequent kinds of local variables don't follow exactly the same rules...
As a simple example of what ended up going wrong is the assertion:
and its associated example. This was (almost) correct originally, but is not correct for implicitly typed or ref local declarations where the initialization cannot be split from the declaration. (It was “almost” as it doesn't allow for for_initializer or resource_acqusition cases where such a split is not possible either.)
The rework divides local variable declarations into three subgroups, there really are four but we can get away with three (whether we should can be debated). These groups are directly represented in the reworked grammar, which also replaces a lot of conditional English prose. The clause starts with the general rules and then has a section for each subgroup detailing just the rules that apply in that case.
All examples have been kept, though I'm not entirely persuaded by the value for the (now) first one.
The changes have been kept to the §13.6.2 clause; there are related issues in at least §7.7.1 Scopes:General and §13.14 The using statement – neither of these for example define the scope of a variable introduced in a resource_acqusition. Such issues are left for future (pre-v8) work!
I think some of the introduced errors this fixes are important enough to make v7, I expect we will debate that! :-), so I'll tag it that way for now.
This should address some of the more recent issues raised – the revision came from reading some of those, heading to this clause, and finding things needed fixing apart from those issues – but I haven't gone through to see which ones it addresses.