paulk-asert commented on PR #3073:
URL: https://github.com/apache/groovy/pull/3073#issuecomment-6488309928
Thanks Daniel. While reviewing this I found that the 5.x behaviour I
described in the JIRA (`def (_, y, _) = [3, 4, 5]; println _` printing 5 in
dynamic code) already changed in **6.0.0**, before this PR. I'm not sure that
was intended: it came from a4d584762f (GROOVY-12242, pattern-variable scoping).
A variable that is not declared in scope now always resolves as a property,
even when a local of that name exists. Repeated `_` has never been declared for
scoping, so every read of it became a property lookup. That change wasn't
announced in COMPATIBILITY.md either. This PR keeps that behaviour and adds a
test that asserts it.
Since we're keeping single `_` as a valid identifier, I'd rather not make
repeated `_` a different kind of thing. Developers shouldn't have to think
about single-assignment and multi-assignment differently, and Java doesn't have
multi-assignment, so it doesn't settle this for us. I've put up #3075 as an
alternative: `_` is always an ordinary variable that may be declared again,
each declaration replacing the earlier `_`. It's a ~10-line change in
`VariableScopeVisitor` and needs no classgen changes. It also flips the three
GROOVY-10943 STC "undeclared" tests and adds a 6.0.1 COMPATIBILITY entry. Could
you take a look and see which you prefer?
### The rule developers need to know
| Version | Rule |
|---|---|
| 5.1.3 | A single `_` is an ordinary variable. A `_` repeated in one
declaration or parameter list is not declared, but its values are still stored.
Dynamic code reads the last value, while STC/SC reports it as undeclared and
closures can't see it. Redeclaring `_` is the usual duplicate error, except
that a repeated-`_` declaration silently overwrites an existing `_`. |
| 6.0.0 | As 5.1.3, except that reading `_` after a repeated `_` is a
property lookup (binding, field or `MissingPropertyException`). The values are
still stored, so an existing local `_` is still overwritten. |
| #3073 | A repeated `_` is unnamed: it declares and stores nothing. Reading
`_` afterwards finds the `_` that existed before (local, field or binding) or
fails. A single `_` in a tuple is an ordinary variable and is checked for
duplicates. |
| #3075 | `_` is an ordinary variable that may be declared again. Each
declaration (single, repeated, tuple, parameter or `catch`) replaces the
earlier `_`, so you see the last value. Other names, and method/constructor
parameters, are still checked for duplicates. |
### Behaviour
| # | Code | 5.1.3 | 6.0.0 | #3073 | #3075 |
|---|---|---|---|---|---|
| 1 | `def (_, y, _) = [3, 4, 5]; _` | 5 | MPE | MPE | 5 |
| 2 | `def c = { _, _ -> _ }; c(2, 3)` | 3 | MPE | MPE | 3 |
| 3 | binding `_ = 'b'`; `def (_, y, _) = [3, 4, 5]; _` | 5 | `'b'` | `'b'`
| 5 |
| 4 | `def _ = 1; def (_, _) = [2, 3]; _` (the ticket) | 3 | 3 | 1 | 3 |
| 5 | `def _ = 1; def x = (_, _) -> _; x(2, 3)` (the ticket) | 3 | 3 | 1 | 3
|
| 6 | `def (_, y, _) = [3, 4, 5]; def (_, z, _) = [6, 7, 8]; _` | 8 | MPE |
MPE | 8 |
| 7 | `def (_, y, _) = …; def (_, z) = [6, 7]; def (_, w, _) = [8, 9, 10];
_` | 10 | 10 | 6 | 10 |
| 8 | `def (_, y) = [1, 2]; def (_, z, _) = [3, 4, 5]; _` | 5 | 5 | 1 | 5 |
| 9 | `def (_, y) = [1, 2]; def (_, z) = [3, 4]; _` | error | error | error
| 3 |
| 10 | `def _ = 1; def _ = 2; _` | error | error | error | 2 |
| 11 | `def _ = 1; [10].each { _ -> … }; _` | error | error | error | 1
(closure parameter shadows `_`) |
| 12 | `def (_, y, _) = [3, 4, 5]; def c = { _ }; c()` | MPE | MPE | MPE | 5
|
| 13 | `@CompileStatic`: `def (_, y, _) = [3, 4, 5]; _` | STC error | STC
error | STC error | 5 |
| 14 | `@CompileStatic`: `def (int _, String s, String _) = [1, 'a', 'bc'];
_.length()` | STC error | STC error | STC error | 2 |
| 15 | `@CompileStatic`: `def _ = 1; def (_, y, _) = [3, 4, 5]; _` | 5 | 5 |
1 | 5 |
| 16 | `def (x, _, y, _, z) = [1, 2, 3, 4, 5]` (named slots) | 1 3 5 | 1 3 5
| 1 3 5 | 1 3 5 |
| 17 | `(_, y, _) = [3, 4, 5]` (assignment, no `def`) | 5 | 5 | 5 | 5 |
| 18 | `def x = 1; def x = 2` / `def m(_, _) {}` | error | error | error |
error |
MPE = `MissingPropertyException`; "error" = "The current scope already
contains a variable of the name …".
A few observations:
- Rows 1–3, 6 and 12 are the 6.0.0 regression. #3075 restores 5.x for them,
and also makes row 12 work, which no version did before.
- #3073 makes rows 7 and 8 depend on whether an earlier tuple had one `_` or
two. With #3075, every row follows from the single rule above.
- #3075 does keep 5.x's answer for the ticket's own case (rows 4/5: `_` is
the last value). That's consistent with "declaring `_` again replaces it" and
is documented as such. Java also allows `var _ = a(); var _ = b();`, so
declaring `_` again is closer to Java than today's error.
- Bytecode: in every version, each `_` declaration already gets its own
local slot, and the ranges in the local variable table don't overlap. A
debugger therefore shows one `_`, the latest one. #3075 doesn't change classgen
at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
