daniellansun commented on PR #2773:
URL: https://github.com/apache/groovy/pull/2773#issuecomment-5242278626
@paulk-asert Thank you for the review. Responses point-by-point.
---
## 1. Breaking-change documentation
> *…silent behavior change… no release-notes/docs change. It should be in the
> beta-2 notes and the Groovy 6 breaking-changes list.*
> *(You noted: JIRA `breaking` label added; release notes on your next
edit.)*
**In-repo documentation added (this PR):**
| Location | Content |
|----------|---------|
| [`COMPATIBILITY.md`](COMPATIBILITY.md) | New **Groovy 6 — `instanceof`
pattern variable flow scoping (GROOVY-12242)** section: who is affected, silent
dynamic-mode failure mode (`MissingPropertyException`), partial `while`
contract |
| [`src/spec/doc/core-semantics.adoc`](src/spec/doc/core-semantics.adoc) |
New subsection **`instanceof` pattern variables (JEP 394)** under Statements:
user-facing rules, dynamic vs `@TypeChecked`, while note, link to JIRA /
COMPATIBILITY |
Beta-2 release-note prose remains with you via the JIRA `breaking`
cross-check — happy to paste a one-liner into the notes draft if useful:
> GROOVY-12242: `instanceof` / `!instanceof` pattern variables follow Java
flow
> scoping; out-of-scope uses are dynamic property lookups in dynamic Groovy
> (typically `MissingPropertyException`) and compile errors under type
checking.
---
## 2. `while` — partial flow scoping (documented decision)
> *while loops deliberately get only partial flow scoping (no after-loop
> introduction as Java has — fine, but should be an explicit documented
decision)*
**Agreed and made explicit.**
**Behaviour (6.0 contract):**
| Path | Support |
|------|---------|
| Short-circuit inside the condition (`&&` / `\|\|`) | Yes |
| `e.whenTrue()` in the **while body** | Yes (aligned with if-then) |
| `e.whenFalse()` **after** the loop when the body cannot complete normally
(JLS §6.3.2.3) | **No** — intentional |
| Leak past the loop | Never |
**Code:** `VariableScopeVisitor.visitWhileLoop` now isolates the condition
visit,
declares only `whenTrue` for the body, and does not declare after the loop.
Javadoc on that method (and on `visitDoWhileLoop`) states the JLS divergence.
User docs: `core-semantics.adoc` NOTE + `COMPATIBILITY.md`.
**Tests:** AST matrix (`InstanceofScopeTest`) and runtime
(`InstanceofTest.testWhileNegated_bodyAndAfterNotVisible`) cover body / after
and abrupt-body “still no after-loop introduction”.
Full after-loop introduction can be a later enhancement if needed; it is not
a
silent 6.0 follow-up.
---
## 3. `writeIfElse` breakable-frame region (GROOVY-7463)
> *writeIfElse changed the breakable-frame region (it now wraps both arms
where
> it previously ended before the else block) — GROOVY-7463 tests pass, but
the
> labeled-break-from-else behavior change should be confirmed as
intentional.*
**Restored historical region; intentional.**
| Region | Pre-12242 | Mid-redesign | **Now** |
|--------|-----------|--------------|---------|
| Condition | inside breakable | outer (for survivors) | outer |
| Then arm | inside | inside | inside |
| Else arm | **outside** | inside | **outside** |
Pattern slots stay on the **outer** frame (so survivors need no put-back
API);
the breakable still ends **before else**, matching GROOVY-7463. Javadoc on
`StatementWriter.writeIfElse` records that choice.
**Tests added:**
- `BreakContinueLabelTest.testBreakLabelFromElseOfLabeledIf` — break from
else
to the if’s label still works (named-break map is method-level; pop does
not
un-register names).
- `BreakContinueLabelTest.testBreakLabelInIfWithInstanceofPattern` — then-arm
break with a pattern variable.
Existing GROOVY-7463 then-arm tests remain green.
---
## 4. Bottom line / Jochen sign-off
> *technically strong and converging fast, but let Jochen sign off on the
redesign*
Acknowledged. The CompileStack push/hide/pop redesign and
`InstanceofFlowBindings` as single analysis+metadata type are the answer to
blackdrag’s layering review; we will wait for Jochen’s explicit OK before
merge.
--
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]
