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`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
