[
https://issues.apache.org/jira/browse/GROOVY-12397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12397:
-------------------------------
Description:
Two related problems with JEP 394 {{instanceof}} pattern variables
(GROOVY-12242), present in 6.0.0-RC-1.
h3. 1. VerifyError for a pattern variable bound in a later {{&&}} / {{||}}
operand
A pattern variable declared in the right operand of {{&&}} or {{||}} has its
slot allocated and null-initialised only at its own {{instanceof}} site. That
operand is skipped when the left operand decides the result, so at the join the
slot is undefined on one path and any later read of it (a following conjunct,
the then branch) fails bytecode verification:
{code:groovy}
def trimmed(Object p) {
if (p instanceof String s && s.length() > 0 && s.trim() instanceof String
t) {
return t
}
return null
}
trimmed(' ab ')
{code}
{noformat}
java.lang.VerifyError: Bad local variable type
{noformat}
The same happens with {{||}}:
{code:groovy}
if (!(p instanceof String s) || s.isEmpty() || !(s.trim() instanceof String t))
return null
return t
{code}
h3. 2. A pattern binding does not survive an if arm ending in {{break}} or
{{continue}}
JLS §14.22 says {{break}} and {{continue}} cannot complete normally, so per
§6.3.2.2-200-C the other path's bindings are in scope after the {{if}}.
{{VariableScopeVisitor}} and the if-statement writer only recognised
{{return}}, {{throw}} and {{yield}}:
{code:groovy}
def upper(List items) {
def out = []
for (o in items) {
if (!(o instanceof String s)) continue
out << s.toUpperCase()
}
out
}
upper(['a', 1, 'b'])
{code}
{noformat}
groovy.lang.MissingPropertyException: No such property: s for class: Script1
{noformat}
h3. Fix
* {{BinaryExpressionHelper}} pre-declares the pattern variables of the right
operand of {{&&}} / {{||}} before the left operand runs; the {{instanceof}}
site reuses the pre-declared slot ({{CompileStack}} keeps the set).
* {{VariableScopeVisitor}} and {{StatementWriter.writeIfElse}} use
{{mayCompleteNormally}} for the "arm is abrupt" test, so {{break}} and
{{continue}} count.
Reproducer script covering all four cases (on 6.0.0-RC-1 it reports two
VerifyErrors and two MissingPropertyExceptions):
{code:groovy}
// Save as patternSlots.groovy and run: groovy patternSlots.groovy
def cases = [
'pattern variable bound in a later && operand': '''
def trimmed(Object p) {
if (p instanceof String s && s.length() > 0 && s.trim() instanceof
String t) {
return t
}
return null
}
assert trimmed(' ab ') == 'ab'
assert trimmed('') == null
assert trimmed(42) == null
''',
'pattern variable bound in a later || operand': '''
def viaOr(Object p) {
if (!(p instanceof String s) || s.isEmpty() || !(s.trim()
instanceof String t)) {
return null
}
return t
}
assert viaOr(' cd ') == 'cd'
assert viaOr(42) == null
''',
'binding survives an if arm ending in continue': '''
def upper(List items) {
def out = []
for (o in items) {
if (!(o instanceof String s)) continue
out << s.toUpperCase()
}
out
}
assert upper(['a', 1, 'b']) == ['A', 'B']
''',
'binding survives an if arm ending in break': '''
def upper(List items) {
def out = []
for (o in items) {
if (o !instanceof String s) break
out << s.toUpperCase()
}
out
}
assert upper(['a', 'b', 1, 'c']) == ['A', 'B']
''',
]
println "Groovy ${GroovySystem.version}"
cases.each { name, source ->
try {
new GroovyShell().evaluate(source)
println "PASS $name"
} catch (Throwable t) {
println "FAIL $name: ${t.class.simpleName}:
${t.message?.readLines()?.head()}"
}
}
{code}
> VerifyError for a pattern variable bound in a later &&/|| operand (JEP 394
> flow scoping)
> ----------------------------------------------------------------------------------------
>
> Key: GROOVY-12397
> URL: https://issues.apache.org/jira/browse/GROOVY-12397
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 6.0.0-RC-1
> Reporter: Paul King
> Priority: Major
>
> Two related problems with JEP 394 {{instanceof}} pattern variables
> (GROOVY-12242), present in 6.0.0-RC-1.
> h3. 1. VerifyError for a pattern variable bound in a later {{&&}} / {{||}}
> operand
> A pattern variable declared in the right operand of {{&&}} or {{||}} has its
> slot allocated and null-initialised only at its own {{instanceof}} site. That
> operand is skipped when the left operand decides the result, so at the join
> the slot is undefined on one path and any later read of it (a following
> conjunct, the then branch) fails bytecode verification:
> {code:groovy}
> def trimmed(Object p) {
> if (p instanceof String s && s.length() > 0 && s.trim() instanceof String
> t) {
> return t
> }
> return null
> }
> trimmed(' ab ')
> {code}
> {noformat}
> java.lang.VerifyError: Bad local variable type
> {noformat}
> The same happens with {{||}}:
> {code:groovy}
> if (!(p instanceof String s) || s.isEmpty() || !(s.trim() instanceof String
> t)) return null
> return t
> {code}
> h3. 2. A pattern binding does not survive an if arm ending in {{break}} or
> {{continue}}
> JLS §14.22 says {{break}} and {{continue}} cannot complete normally, so per
> §6.3.2.2-200-C the other path's bindings are in scope after the {{if}}.
> {{VariableScopeVisitor}} and the if-statement writer only recognised
> {{return}}, {{throw}} and {{yield}}:
> {code:groovy}
> def upper(List items) {
> def out = []
> for (o in items) {
> if (!(o instanceof String s)) continue
> out << s.toUpperCase()
> }
> out
> }
> upper(['a', 1, 'b'])
> {code}
> {noformat}
> groovy.lang.MissingPropertyException: No such property: s for class: Script1
> {noformat}
> h3. Fix
> * {{BinaryExpressionHelper}} pre-declares the pattern variables of the right
> operand of {{&&}} / {{||}} before the left operand runs; the {{instanceof}}
> site reuses the pre-declared slot ({{CompileStack}} keeps the set).
> * {{VariableScopeVisitor}} and {{StatementWriter.writeIfElse}} use
> {{mayCompleteNormally}} for the "arm is abrupt" test, so {{break}} and
> {{continue}} count.
> Reproducer script covering all four cases (on 6.0.0-RC-1 it reports two
> VerifyErrors and two MissingPropertyExceptions):
> {code:groovy}
> // Save as patternSlots.groovy and run: groovy patternSlots.groovy
> def cases = [
> 'pattern variable bound in a later && operand': '''
> def trimmed(Object p) {
> if (p instanceof String s && s.length() > 0 && s.trim()
> instanceof String t) {
> return t
> }
> return null
> }
> assert trimmed(' ab ') == 'ab'
> assert trimmed('') == null
> assert trimmed(42) == null
> ''',
> 'pattern variable bound in a later || operand': '''
> def viaOr(Object p) {
> if (!(p instanceof String s) || s.isEmpty() || !(s.trim()
> instanceof String t)) {
> return null
> }
> return t
> }
> assert viaOr(' cd ') == 'cd'
> assert viaOr(42) == null
> ''',
> 'binding survives an if arm ending in continue': '''
> def upper(List items) {
> def out = []
> for (o in items) {
> if (!(o instanceof String s)) continue
> out << s.toUpperCase()
> }
> out
> }
> assert upper(['a', 1, 'b']) == ['A', 'B']
> ''',
> 'binding survives an if arm ending in break': '''
> def upper(List items) {
> def out = []
> for (o in items) {
> if (o !instanceof String s) break
> out << s.toUpperCase()
> }
> out
> }
> assert upper(['a', 'b', 1, 'c']) == ['A', 'B']
> ''',
> ]
> println "Groovy ${GroovySystem.version}"
> cases.each { name, source ->
> try {
> new GroovyShell().evaluate(source)
> println "PASS $name"
> } catch (Throwable t) {
> println "FAIL $name: ${t.class.simpleName}:
> ${t.message?.readLines()?.head()}"
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
