[
https://issues.apache.org/jira/browse/GROOVY-12063?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135376#comment-18135376
]
ASF GitHub Bot commented on GROOVY-12063:
-----------------------------------------
eric-milles commented on PR #3067:
URL: https://github.com/apache/groovy/pull/3067#issuecomment-6531791728
Can you please hold on this one? I added a comment to the JIRA ticket
several days ago. I'm not sure about adding so much code for this odd edge
case. And having AIC and Closure cases work differently may be a concern as
well.
> Anonymous inner class extending Map: enclosing-class field references in the
> instance initializer compile to dynamic getProperty on 'this' and silently
> resolve to null - regression in 5.0.x
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12063
> URL: https://issues.apache.org/jira/browse/GROOVY-12063
> Project: Groovy
> Issue Type: Bug
> Components: class generator, Compiler
> Affects Versions: 6.0.0-alpha-1, 5.0.5, 5.0.6, 5.0.7
> Environment: JDK 21; reproduces with and without @CompileStatic, and
> in both indy and classic codegen
> Reporter: James Fredley
> Assignee: Daniel Sun
> Priority: Major
>
> h2. Summary
> In an anonymous inner class that extends {{java.util.Map}} (e.g.
> {{HashMap}}), bareword references to the *enclosing class's fields* inside
> the AIC instance-initializer block are no longer resolved to those fields. As
> of Groovy 5.0.x they compile to dynamic property reads on {{this}}
> ({{this.getProperty("FIELD")}}); because {{this}} is a {{Map}}, the MOP turns
> that into a key lookup {{this.get("FIELD")}} on the still-empty map, so the
> reference silently evaluates to {{null}}. This is a regression: 3.0.x and
> 4.0.x resolve it correctly. The {{Map}} base class makes it fail *silently* -
> a non-Map AIC would instead throw {{MissingPropertyException}}.
> h2. Steps to reproduce
> Single self-contained script:
> {code:groovy}
> class Holder {
> static final String VALUE = 'expected'
> static final Map<String, String> MAP = new HashMap<String, String>() {
> {
> put('literalKey', VALUE) // 'VALUE' is the enclosing class's
> static field
> }
> }
> }
> println "Groovy ${GroovySystem.version} -> ${Holder.MAP}"
> assert Holder.MAP.get('literalKey') == 'expected'
> {code}
> h2. Actual result (Groovy 5.0.5 / 5.0.6 / 5.0.7-SNAPSHOT / 6.0.0-alpha-1)
> {noformat}
> Groovy 5.0.7-SNAPSHOT -> [literalKey:null]
> Assertion failed:
> assert Holder.MAP.get('literalKey') == 'expected'
> | | | |
> | | null false
> {noformat}
> h2. Expected result (as on Groovy 3.0.24 / 4.0.32)
> {noformat}
> Groovy 4.0.32 -> [literalKey:expected]
> (assertion passes)
> {noformat}
> h2. Affected versions
> || Groovy || Holder.MAP || Result ||
> | 3.0.24 | {{[literalKey:expected]}} | correct |
> | 4.0.32 | {{[literalKey:expected]}} | correct |
> | 5.0.5 | {{[literalKey:null]}} | broken |
> | 5.0.6 | {{[literalKey:null]}} | broken |
> | 5.0.7-SNAPSHOT | {{[literalKey:null]}} | broken |
> | 6.0.0-alpha-1 | {{[literalKey:null]}} | broken |
> Notes: reproduces for a plain class, a {{@CompileStatic}} class, and a
> {{@CompileStatic}} interface - so it is not specific to {{@CompileStatic}}.
> The literal key survives (an {{LDC}} constant); only the bareword field
> reference degrades to {{null}}. If both key and value are field references,
> the map collapses to {{[null:null]}}.
> h2. Bytecode evidence
> {{Holder$1.<init>}} (the anonymous HashMap initializer), {{javap -c}}, Groovy
> 5.0.7-SNAPSHOT:
> {noformat}
> 13: aload_0 // receiver = this (the
> HashMap)
> 14: ldc "literalKey" // key: literal ->
> correct
> 16: aload_0
> 17: invokedynamic #0:getProperty:(LHolder$1;) // value:
> this.getProperty("VALUE") -> BUG
> 22: invokedynamic #1:invoke:(...) // this.put(key, value),
> dynamic
> 28: return
> {noformat}
> The BootstrapMethods table confirms the call-site name is the source
> identifier ({{IndyInterface.bootstrap(..., "VALUE", 12)}}). The enclosing
> {{<clinit>}} is correct - {{VALUE}} is assigned before {{new Holder$1()}}
> runs - so the defect is purely the AIC initializer compiling the bareword as
> a dynamic read on {{this}} instead of {{GETSTATIC}}. At runtime
> {{MetaClassImpl.getProperty}} special-cases {{Map}} receivers as {{((Map)
> this).get(name)}}; the map is empty during its own initializer, so the read
> returns {{null}}. The JDK is not involved - the emitted bytecode is wrong.
> h2. Root cause
> * The bareword {{VALUE}} inside the AIC initializer is resolved as a dynamic
> property read on {{this}} ({{invokedynamic getProperty}}) rather than a
> {{GETSTATIC}} of the enclosing class's field.
> * Because the AIC extends {{Map}}, Groovy's MOP turns that property read into
> a map-key lookup {{this.get("VALUE")}}; the map is empty while its own
> initializer runs, so it returns {{null}} instead of failing fast.
> * Groovy 3.0.x / 4.0.x resolve the bareword to the enclosing-class field
> correctly.
> h2. Real-world impact
> Apache Grails {{grails.gorm.validation.ConstrainedProperty}} builds its
> default-messages map with exactly this idiom ({{new HashMap<String,String>()
> { { put(CODE, MESSAGE) } }}}); under Groovy 5 the map ends up effectively
> empty ({{[null:null]}}), so default validation messages fail to resolve
> (apache/grails-core PR #15557).
> h2. Workarounds
> * Avoid the anonymous-Map-with-instance-initializer idiom; use
> {{Map.of(...)}}, a literal map, or a statically compiled static block.
> * Or fully qualify the references: {{put('literalKey', Holder.VALUE)}}.
> h2. Suggested fix
> Resolve bareword identifiers inside an AIC instance-initializer against the
> lexical scope (enclosing-class members) before falling back to a dynamic
> property read on {{this}}, regardless of whether {{this}} is a {{Map}} - i.e.
> restore 4.0.x name-resolution for AIC initializer blocks.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
