[
https://issues.apache.org/jira/browse/GROOVY-12254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12254:
-------------------------------
Description:
h3. Problem
Groovy 6.0.0-beta-1 introduced {{defer}} as a contextual keyword as part of
native async/await support (GROOVY-9381). While {{defer}} remains usable as an
identifier in most positions (variable names, assignments, map keys, property
access, explicit-receiver calls, expression-position calls), the
{{deferStmtAlt}} grammar alternative is unguarded, so any statement matching
{{defer <expression>}} is claimed by the keyword in _all_ contexts, not just
inside {{async}} closures.
This silently breaks pre-Groovy-6 code that calls a user-defined {{defer}}
method in statement position:
{code:groovy}
def defer(Closure c) { c() }
defer { println 'hi' } // Groovy 5: method call; 6.0.0-beta-1: parsed as a
defer statement
defer 'command style' // likewise
defer('parens') // likewise — ('parens') is itself a valid expression
{code}
In beta-1 such code compiles but fails at runtime with an obscure error that
leaks an implementation detail:
{code}
groovy.lang.MissingPropertyException: No such property: $__deferScope__ for
class: Foo
{code}
A genuinely misplaced {{defer}} (i.e. outside any {{async}} closure, with no
user method intended) produces the same unfriendly error, including under
{{@CompileStatic}} — there is currently no compile-time diagnostic on any path.
h3. Proposed fix
Gate the statement alternative with a semantic predicate, following the
existing {{yieldStatement}} / {{inSwitchExpressionLevel}} pattern in the
grammar:
* a new parser field {{inAsyncClosureLevel}}, incremented/decremented around
the closure/lambda of {{asyncClosureExprAlt}}
* {{deferStmtAlt}} guarded so it is only viable when {{inAsyncClosureLevel > 0}}
Outside an {{async}} closure the {{defer}} token then falls through to the
identifier path, so statement-position calls parse as ordinary method calls
again, restoring Groovy 5 source compatibility. A misplaced {{defer}} with no
such method in scope fails with Groovy's normal {{MissingMethodException}}
rather than the internal-variable error.
In addition, an AstBuilder check reports a clear compile-time error for
{{defer}} used inside a _nested_ (non-async) closure within an {{async}}
closure. The runtime semantics there were never defined — the scope wrapping
deliberately does not descend into nested closures
({{AsyncTransformHelper.containsDefer}}) — so this now fails fast:
{code}
defer must be used directly in the body of an async closure, not in a nested
closure
{code}
h3. Behaviour comparison
|| code || Groovy 5 || 6.0.0-beta-1 || with this fix ||
| {{defer { ... }}} / {{defer x}} / {{defer(x)}} as a statement, user {{defer}}
method in scope | method call | runtime {{MissingPropertyException}} (internal
variable) | method call (compatible with Groovy 5) |
| same, no {{defer}} method in scope | {{MissingMethodException}} | runtime
{{MissingPropertyException}} (internal variable) | {{MissingMethodException}} |
| {{defer}} directly inside {{async { }}} (closure or lambda) | n/a | works |
works (unchanged) |
| {{defer}} in a nested non-async closure inside {{async { }}} | n/a |
undefined behaviour / obscure runtime error | clear compile-time error |
h3. Known limitation
A statement-position call to a user-defined {{defer}} method _inside_ an
{{async}} closure (at any nesting depth) is still claimed by the keyword. This
is the irreducible cost of the contextual-keyword design; the fix confines it
to async closure bodies instead of applying everywhere.
h3. Alternative considered
An AstBuilder-only check (no grammar change) turning every out-of-async
{{defer}} statement into a compile-time error with a dedicated message. This
gives the best possible diagnostics but keeps beta-1's source incompatibility
for DSLs with a {{defer}} method, so the predicate approach is preferred.
(Proposed pending team review.)
> Make defer fully contextual: parse as a keyword only inside async closures
> --------------------------------------------------------------------------
>
> Key: GROOVY-12254
> URL: https://issues.apache.org/jira/browse/GROOVY-12254
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> h3. Problem
> Groovy 6.0.0-beta-1 introduced {{defer}} as a contextual keyword as part of
> native async/await support (GROOVY-9381). While {{defer}} remains usable as
> an identifier in most positions (variable names, assignments, map keys,
> property access, explicit-receiver calls, expression-position calls), the
> {{deferStmtAlt}} grammar alternative is unguarded, so any statement matching
> {{defer <expression>}} is claimed by the keyword in _all_ contexts, not just
> inside {{async}} closures.
> This silently breaks pre-Groovy-6 code that calls a user-defined {{defer}}
> method in statement position:
> {code:groovy}
> def defer(Closure c) { c() }
> defer { println 'hi' } // Groovy 5: method call; 6.0.0-beta-1: parsed as a
> defer statement
> defer 'command style' // likewise
> defer('parens') // likewise — ('parens') is itself a valid expression
> {code}
> In beta-1 such code compiles but fails at runtime with an obscure error that
> leaks an implementation detail:
> {code}
> groovy.lang.MissingPropertyException: No such property: $__deferScope__ for
> class: Foo
> {code}
> A genuinely misplaced {{defer}} (i.e. outside any {{async}} closure, with no
> user method intended) produces the same unfriendly error, including under
> {{@CompileStatic}} — there is currently no compile-time diagnostic on any
> path.
> h3. Proposed fix
> Gate the statement alternative with a semantic predicate, following the
> existing {{yieldStatement}} / {{inSwitchExpressionLevel}} pattern in the
> grammar:
> * a new parser field {{inAsyncClosureLevel}}, incremented/decremented around
> the closure/lambda of {{asyncClosureExprAlt}}
> * {{deferStmtAlt}} guarded so it is only viable when {{inAsyncClosureLevel >
> 0}}
> Outside an {{async}} closure the {{defer}} token then falls through to the
> identifier path, so statement-position calls parse as ordinary method calls
> again, restoring Groovy 5 source compatibility. A misplaced {{defer}} with no
> such method in scope fails with Groovy's normal {{MissingMethodException}}
> rather than the internal-variable error.
> In addition, an AstBuilder check reports a clear compile-time error for
> {{defer}} used inside a _nested_ (non-async) closure within an {{async}}
> closure. The runtime semantics there were never defined — the scope wrapping
> deliberately does not descend into nested closures
> ({{AsyncTransformHelper.containsDefer}}) — so this now fails fast:
> {code}
> defer must be used directly in the body of an async closure, not in a nested
> closure
> {code}
> h3. Behaviour comparison
> || code || Groovy 5 || 6.0.0-beta-1 || with this fix ||
> | {{defer { ... }}} / {{defer x}} / {{defer(x)}} as a statement, user
> {{defer}} method in scope | method call | runtime
> {{MissingPropertyException}} (internal variable) | method call (compatible
> with Groovy 5) |
> | same, no {{defer}} method in scope | {{MissingMethodException}} | runtime
> {{MissingPropertyException}} (internal variable) | {{MissingMethodException}}
> |
> | {{defer}} directly inside {{async { }}} (closure or lambda) | n/a | works |
> works (unchanged) |
> | {{defer}} in a nested non-async closure inside {{async { }}} | n/a |
> undefined behaviour / obscure runtime error | clear compile-time error |
> h3. Known limitation
> A statement-position call to a user-defined {{defer}} method _inside_ an
> {{async}} closure (at any nesting depth) is still claimed by the keyword.
> This is the irreducible cost of the contextual-keyword design; the fix
> confines it to async closure bodies instead of applying everywhere.
> h3. Alternative considered
> An AstBuilder-only check (no grammar change) turning every out-of-async
> {{defer}} statement into a compile-time error with a dedicated message. This
> gives the best possible diagnostics but keeps beta-1's source incompatibility
> for DSLs with a {{defer}} method, so the predicate approach is preferred.
> (Proposed pending team review.)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
