[
https://issues.apache.org/jira/browse/GROOVY-12126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18094510#comment-18094510
]
ASF GitHub Bot commented on GROOVY-12126:
-----------------------------------------
testlens-app[bot] commented on PR #2677:
URL: https://github.com/apache/groovy/pull/2677#issuecomment-4911850894
## ✅ All tests passed ✅
🏷️ Commit: da4e8fbd9f19a048285e3a6d9bff4b84e8eb31ab
▶️ Tests: 103476 executed
⚪️ Checks: 31/31 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
> Non-local control flow from closures (return / break / continue)
> ----------------------------------------------------------------
>
> Key: GROOVY-12126
> URL: https://issues.apache.org/jira/browse/GROOVY-12126
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
> Labels: GEP
> Fix For: 7.x
>
>
> h1. Non-local control flow from closures (return / break / continue)
> h2. Status
> Proposed — pending team review. Core feasibility de-risked by spikes (see
> Evidence).
> h2. Summary
> Give closures explicit, opt-in non-local control flow:
> * {{return@methodName expr}} — return a value from the lexically enclosing
> method.
> * {{break}} / {{continue}} inside cooperating iteration methods, via a
> repaired
> loop-control protocol.
> Bare {{return}} inside a closure keeps its current meaning (return from the
> closure) —
> unchanged. This enables Smalltalk/Kotlin-style control-flow DSLs (custom
> iteration with
> early exit, {{ifFalse}}/{{unless}}, builder short-circuits) that today force
> users into
> {{find}}/{{any}}/{{findResult}} or hand-rolled sentinel exceptions.
> h2. Motivation
> Today {{return}} in a closure returns only from the closure; there is no
> language form to
> abort the enclosing method, nor to {{break}}/{{continue}} an iteration, from
> within a passed
> block. The existing {{Closure.DONE}}/{{SKIP}} "directive" is a half-built
> attempt: DONE is
> honored by only 3 DGM methods, SKIP is honored nowhere, and it is mutable
> state on the shared
> Closure instance. This GEP provides a coherent, type-safe,
> backward-compatible replacement.
> h2. Prior art & relationship
> * GROOVY-8301 ("break/return/continue in appended block closures") proposes
> an {{@Inline}}
> annotation (Kotlin-style) that inlines the closure so the keywords compile
> to real jumps.
> * GROOVY-6880 is the underlying inline-AST-transform infrastructure.
> This GEP is complementary, not competing. It provides mechanisms that work in
> DYNAMIC Groovy
> with no inlining machinery; {{@Inline}} remains an optional zero-cost
> optimization (Part 3).
> ============================================================
> h2. Part 1 — Non-local return: return@methodName
> ============================================================
> h3. Syntax (proposed, pending team review)
> {code:groovy}
> def firstMatch(rows, pred) {
> rows.each { row ->
> row.each { cell ->
> if (pred(cell)) return@firstMatch cell // unwinds both
> closures, returns from firstMatch
> }
> }
> null
> }
> {code}
> * Bare {{return}} — unchanged (returns from the closure). 100% backward
> compatible.
> * {{return@name expr}} — new; {{@name}} must be an enclosing method's name
> (compiler-verified).
> Script bodies use the reserved label {{return@script}}.
> Rationale: after {{return}}, {{@}} cannot begin an expression today, so
> {{return@x}} is
> unparseable now and free to claim; Kotlin-familiar, self-documenting, visible
> at the call site.
> Alternatives considered: {{^expr}} (Smalltalk caret; cryptic, names no
> target); {{returnFrom
> name expr}} (Common Lisp; wordier). Both rejected as primary.
> h3. Two Groovy-specific facts
> # A closure's non-local target is the NEAREST enclosing method. Method scopes
> CAN nest — via
> anonymous inner classes (today) or hypothetical method-local classes — so
> {{@name}} is
> REQUIRED to disambiguate; it is load-bearing, not mere documentation.
> Resolution stays fully
> static (closure and target are in the same source).
> # Closures have indefinite extent (may be stored/returned/run on another
> thread), so a
> non-local return CANNOT be a guaranteed jump; it must fail loudly when the
> target activation
> is gone.
> h3. Boundary rule
> Non-local control flow does NOT cross a class boundary. From a closure inside
> an anonymous or
> method-local inner class, the legal {{@name}} targets are the methods of the
> NEAREST enclosing
> class only; targeting an outer class's method (e.g. {{return@outer}} from
> inside a local
> class's {{inner()}}) is a COMPILE ERROR. Precedent: Java forbids non-local
> return from local/
> anonymous class methods; Kotlin permits it only from inline lambdas, never
> from object/local-
> class method bodies. This keeps @CompileStatic resolution and @Inline
> lowering clean and
> confines the "target frame gone" failure surface. The token mechanism COULD
> cross the
> boundary (tokens are captured like any enclosing value), so this is a
> deliberate policy choice,
> not a limitation.
> h3. Implementation (compiler-generated, no runtime magic)
> # At entry of a targeted method, synthesize a unique per-activation token
> (recursion-safe).
> # Wrap the body: {{try { ... } catch (NonLocalReturn e) { if (e.token == tok)
> return (T) e.value; else throw e; }}}.
> # At the {{return@m}} site: {{throw new NonLocalReturn(capturedToken,
> value)}}.
> # {{NonLocalReturn}} is a lightweight throwable with a SUPPRESSED stack trace
> (mandatory — see perf).
> Because the closure and its target method are always in the same source,
> resolution and
> type-checking are fully static; {{@CompileStatic}} checks {{expr}} against
> {{m}}'s return type.
> h3. Semantics
> || Situation || Behavior ||
> | Closure invoked in-place within the method | Non-local return (intended
> path) |
> | Code after {{return@m}} in same block | Unreachable (compile error) |
> | {{@name}} not an enclosing method | Compile error |
> | Returned type vs method return type | Static under @CompileStatic; runtime
> otherwise |
> | Closure invoked after {{m}} returned, or on another thread | Loud runtime
> failure (frame gone) |
> | Nested closures | Unwinds through all intermediate closure frames to {{m}} |
> | {{return@outer}} across an inner-class boundary (anon/local class) |
> Compile error (see Boundary rule) |
> | {{try/finally}} between site and {{m}} | {{finally}} runs during unwind
> (exception semantics) |
> h3. Performance
> ~15 ns overhead per non-local return (JIT-warm), ~5x a plain return. A NAIVE
> traced exception
> is ~562 ns (31x worse) — hence stack-trace suppression is mandatory. Fine for
> DSL/early-exit;
> document "not for hottest inner loops."
> ============================================================
> h2. Part 2 — break / continue via a loop-control protocol
> ============================================================
> h3. Current state (the half-feature being repaired)
> {{Closure.DONE}} honored by 3 DGM methods
> (collect(Iterable,Collection,Closure), collectNested,
> times) via a POST-call check; {{Closure.SKIP}} honored nowhere; directive is
> mutable state on
> the shared Closure instance. Consequences: fragile under
> re-entrancy/concurrency, and a
> semantic wart — {{break}} in {{collect}} still collects the current element
> (value added before
> the DONE check).
> h3. Two facts
> # {{continue}} ~= "return from the closure body". For value-IGNORING
> iterators (each,
> eachWithIndex, times, upto, step) it needs NO protocol — it lowers to a
> plain closure-local
> return. Universal, zero cooperation.
> # {{break}} inherently requires the loop to cooperate — no closure can stop
> an arbitrary
> caller's iteration without the caller checking something (also true of
> @Inline).
> h3. Design — keyword sugar over a compiler-internal loop signal (NOT a
> return-value convention)
> CRITICAL: {{break}}/{{continue}} are CONTROL-FLOW STATEMENTS, not
> expressions, and are lowered
> by the COMPILER to an internal, NON-USER-CONSTRUCTIBLE loop signal. This is
> what preserves type
> safety and prevents data collision:
> * Type safety: the static checker type-checks only the value-returning paths
> of the body
> against {{T}} and treats {{break}}/{{continue}} like loop
> {{break}}/{{return}} — they
> contribute nothing to the inferred type. The closure's source-level return
> type stays {{T}},
> so e.g. {{collect}} stays {{List<T>}}. The signal is injected during
> CODEGEN, after type
> checking, riding the Object-typed {{call()}} that all closures already have
> — invisible to
> inference. (A naive "body returns a public {{LoopControl}} enum" convention
> does NOT work:
> under @CompileStatic the body's type widens to LUB(T, signal) and
> {{collect}} degrades away
> from {{List<T>}}. Verified in spike.)
> * No data collision: because the signal is non-user-constructible (only the
> compiler emits it
> via the keywords), no genuine data value can equal it, so the identity
> check never misfires.
> h3. Keyword desugaring
> * {{continue}} -> internal CONTINUE signal (or plain {{return}} for
> value-ignoring iterators)
> * {{break}} -> internal BREAK signal
> SAFETY GATE: the keyword sugar is enabled ONLY when the target method is
> statically resolved
> and annotated {{@SupportsLoopControl}}; otherwise it is a COMPILE ERROR.
> (Without the gate,
> {{break}} on a non-cooperating method would silently degrade to "return from
> closure" — a
> silent correctness bug.) This trades @Inline's inlinability requirement for a
> protocol-conformance requirement, while remaining dynamic-dispatch capable.
> h3. Per-iterator semantics
> || Iterator kind || break || continue ||
> | value-ignoring (each, times, upto, step) | stop | proceed (== plain return)
> |
> | value-consuming (collect, findAll, inject) | stop, do NOT contribute
> current element | skip contributing current element |
> The value-consuming BREAK correctly excludes the breaking element (unlike the
> legacy post-hoc
> directive, which includes it). Verified in spike: legacy {{collect}} ->
> [10,20,30,999];
> protocol -> [10,20,30].
> h3. SPI / rollout
> * Add {{@SupportsLoopControl}} and roll the signal checks across DGM
> iterators.
> * Expose the annotation + a detection helper (e.g. {{LoopSignal.of(r)}} ->
> BREAK/CONTINUE/VALUE)
> so third-party iterators (the issue's {{sql.forEachRow}}, GPars, custom
> builders) opt in.
> SPI authors detect via the helper; they never compare against a
> user-returnable value.
> * Give the previously-dead SKIP a real meaning ("skip contributing"), or
> explicitly scope
> break/continue to value-ignoring iterators in v1.
> h3. Performance
> Below measurement noise (~0 ns/element; identity checks), vs ~15 ns/break for
> the Part-1
> exception path. The protocol is the cheap path for break/continue.
> ============================================================
> h2. Part 3 — @Inline as an optional zero-cost lowering
> ============================================================
> Where a callee is {{@Inline}} (GROOVY-8301 / GROOVY-6880), statically
> resolved, and the closure
> is non-escaping, the compiler may LOWER Part-1 return@m and Part-2
> break/continue to real jumps
> (0 ns). It is the fast path, not the only path, and is out of scope for THIS
> GEP's first
> increment.
> h2. Compatibility
> * Bare {{return}} semantics unchanged; {{return@ident}} is new,
> previously-unparseable syntax.
> * Legacy {{Closure.DONE}}/{{SKIP}}/{{directive}}: keep honoring on the
> current 3 methods;
> deprecate in favor of the loop-control protocol.
> * No existing code is affected: new signal returns are compiler-internal and
> additive.
> h2. Scope / Non-goals
> * In scope: value-returning non-local return ({{return@m}}); break/continue
> via the
> loop-control protocol.
> * Out of scope: changing bare {{return}} semantics; making {{break}} work
> through an arbitrary
> NON-cooperating method (impossible); non-local control flow across a class
> boundary (Boundary
> rule); full closure inlining (the @Inline / GROOVY-6880 track, referenced
> as the zero-cost
> optimization).
> h2. Evidence (spikes)
> # @CompileStatic desugaring of return@m (token + method try/catch + throw
> from captured nested
> closure) compiles statically and is correct.
> # Escaped/cross-thread closure surfaces {{NonLocalReturn}} loudly — no silent
> corruption.
> # Perf: baseline plain return ~3.4 ns/op; return@m no-trace exception ~18
> ns/op (~5x);
> naive traced exception ~562 ns/op (31x) — suppression mandatory.
> # Loop-control protocol: break/each stops early; break/collect excludes the
> breaking element;
> continue/collect skips it; legacy directive collect includes it (wart).
> Overhead below noise.
> # Type safety: a public-enum return convention widens the body type under
> @CompileStatic
> (rejected assignment to Integer) — confirming the feature must be keyword
> sugar with a
> compiler-internal signal, not a return-value convention.
> # Nesting: anonymous inner classes already nest method scopes (outer -> inner
> -> closure),
> confirming {{@name}} must be required and motivating the Boundary rule.
> h2. Open questions (pending team review)
> * RESOLVED: whether {{@name}} is required — yes. Because method scopes nest
> (anonymous/local
> inner classes), the label is required for disambiguation, not optional
> sugar.
> * return@m surface syntax: {{return@m}} vs {{^}} vs {{returnFrom}}.
> * Whether value-ignoring {{continue}} lowers to bare {{return}} or always to
> the signal.
> * Labeled {{break@method}}/{{continue@method}} for nested loops, mirroring
> {{return@method}}
> and adopting the same Boundary rule (no crossing a class boundary).
> * Best-effort lint when a closure containing non-local control flow is
> stored/returned rather
> than invoked in place.
> * Cautionary precedent: Scala DEPRECATED non-local {{return}} — but theirs
> was IMPLICIT; this
> proposal is EXPLICIT and opt-in, which is the mitigation.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
