[
https://issues.apache.org/jira/browse/GROOVY-12551?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18134517#comment-18134517
]
ASF GitHub Bot commented on GROOVY-12551:
-----------------------------------------
jdaugherty commented on PR #3080:
URL: https://github.com/apache/groovy/pull/3080#issuecomment-6511415502
Thanks for the review. I have tightened the test: it now asserts that the
line number tables of the two compilations are equal, and that the `setName`
invoke of the named argument is attributed to line 8 in both. Without the fix,
the compilation against the compiled `Ctx` attributes it to line 6 and its
table has no entry for line 8.
`together == separate` cannot pass here even with the fix, because the two
compilations also differ in how the setter call itself is written:
```
Ctx in the compilation unit Ctx already compiled
LINENUMBER 8 LINENUMBER 8
ALOAD 2 ALOAD 1
ALOAD 1 DUP
INVOKEVIRTUAL p/Ctx.setName ASTORE 3
ALOAD 2
ALOAD 3
INVOKEVIRTUAL p/Ctx.setName
POP
```
When `Ctx` is already compiled, the type checker finds the real `setName`
method and records it as the direct call target of the key, which
`ConstructorCallTransformer.setProperties` copies onto the property expression.
`BinaryExpressionTransformer.transformAssignmentToSetterCall` then rewrites the
assignment through `StaticPropertyAccessHelper.transformToSetterCall` with
`returnValue` hard-coded to `true` (there is a TODO on that line), so the value
is kept in a temporary variable and popped afterwards; the
`ELIDE_EXPRESSION_VALUE` flag that `setProperties` puts on the result is not
consulted on that path. When `Ctx` is in the compilation unit, the property has
no setter method node yet, so the assignment stays a `BinaryExpression` and
`StaticTypesBinaryExpressionMultiTypeDispatcher.evaluateEqual` honours the
flag. The missing line number had the same root: only the `BinaryExpression`
path emitted one.
The remaining difference is harmless at run time (an extra local and a dead
`DUP`/`POP`), but it is another case of the class file depending on whether the
other class was in the compilation unit. Would you prefer that I fix it now, in
this PR or in a separate one, or that I file a JIRA issue for it as a
follow-up? Once it is fixed, the test can be changed to `together == separate`.
> SC: a named argument of a constructor call gets no line number when the
> constructed class is already compiled
> -------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12551
> URL: https://issues.apache.org/jira/browse/GROOVY-12551
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation
> Affects Versions: 5.1.3
> Reporter: James Daugherty
> Priority: Major
>
> With static compilation, a constructor call with named arguments, {{new
> Ctx(name: name)}}, is expanded into a constructor call and one assignment per
> argument. The assignment gets a line number entry only when {{Ctx}} is
> compiled together with the caller. When {{Ctx}} is already compiled, the
> entry is missing, so the same source compiles to a different class file
> depending on what else is compiled, and a debugger cannot stop on that line.
> Groovy 4.0.33 writes the entry in both cases.
> {code:groovy}
> class Ctx {
> String name
> }
> @groovy.transform.CompileStatic
> class Svc {
> static Object use(int i, Ctx ctx) { ctx }
> Object make(String name) {
> use(
> 1,
> new Ctx(name: name) // no line number entry when Ctx is
> already compiled
> )
> }
> }
> {code}
> Reproduced with 5.1.3 and master. A reproducer with a groovyc script and an
> incremental Gradle build:
> https://github.com/jdaugherty/groovy-named-argument-line-numbers
> *Cause*
> {{ConstructorCallTransformer}} builds {{tmp.name = name}} for each named
> argument, transforms it, and only then sets the position of the map entry on
> the result. When {{Ctx}} is compiled, static type checking knows its setter,
> so {{BinaryExpressionTransformer}} rewrites the assignment into a setter call
> built from the untransformed expression, which has no position yet, and no
> line number is written. When {{Ctx}} is in the compilation unit, the
> assignment stays a binary expression, which writes the line of the position
> set afterwards.
> *Proposed fix*
> Write the line of the map entry before generating the code for its
> assignment, whatever expression the transformer returns.
> Found while making incremental builds of a Grails application produce the
> same classes as a full build; see also GROOVY-12550 and GROOVY-12544.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
