[
https://issues.apache.org/jira/browse/GROOVY-12441?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137109#comment-18137109
]
ASF GitHub Bot commented on GROOVY-12441:
-----------------------------------------
paulk-asert commented on PR #3068:
URL: https://github.com/apache/groovy/pull/3068#issuecomment-6600664880
I agree something is needed here beyond documentation:
`MethodPointerExpression.getExpression()` hands out `THIS_EXPRESSION` for a
null receiver and `transformExpression` bakes it in, so after any
`ClassCodeExpressionTransformer` pass the compiler's own AST carries the shared
node, and groovy-contracts uses it in generated code too. On master 7 of the 16
cases in `Groovy12441` fail, plus one the test doesn't cover yet: `this::tag`
inside a closure fails with "Failed to find method 'tag()' for the type:
C$_run_closure1". This PR fixes that one as well, so it's worth adding.
I also tried the other obvious fix, making SC resolve an untyped `this` in a
closure to the enclosing class (`StaticTypesTypeChooser` +
`StaticInvocationWriter`, 14 lines). It passes all 16 cases but breaks 8
existing tests (Groovy7996, 9063, 9288, 9293, `testMapPropertyAccess10`,
Memoized record serialization), because classgen itself creates untyped `this`
nodes inside closure classes that really do mean the closure. So typing each
use before classgen, as done here, is the right layer.
On Jochen's point about the amount of visitor code: the same four hook
points work with a `ClassCodeExpressionTransformer` subclass whose only
override is `transform()` (fresh node for either sentinel; descend into
closures/lambdas as its Javadoc shows), plus a small `CodeVisitorSupport`
presence check so members without a shared node are never rebuilt. That's about
100 lines including the header, and gives the same result:
STC/SC/bugs/gls/transform/typing suites green (13,323 tests), all 18 cases
passing. It also removes the `getText()` special case:
`MethodPointerExpression.transformExpression` never routes a truly-null
receiver through `transform()`, so implicit pointers stay implicit on their
own. (As written, the `startsWith("::")` branch is dead code:
`MethodReferenceExpression.getText()` goes through `getExpression()`, which
already substitutes the shared node, so an implicit `::m` would currently be
rewritten to an explicit `this::m`.) The trade-off is that
`transformExpression` rebuilds the composite nodes of the affected member
rather than mutating in place; metadata and positions are copied, as
`StaticCompilationTransformer` already relies on. With that, the rewriter's own
unit test could shrink to the `bugs` test.
Happy to share the patch if useful. Separately, a Javadoc on
`THIS_EXPRESSION`/`SUPER_EXPRESSION` pointing transform authors at
`varX("this")` would address Björn's request, either here or as a follow-up.
> STC gets confused when THIS_EXPRESSION is used
> ----------------------------------------------
>
> Key: GROOVY-12441
> URL: https://issues.apache.org/jira/browse/GROOVY-12441
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 2.5.23, 3.0.25, 4.0.33, 5.1.3, 6.0.0
> Reporter: Björn Kautler
> Assignee: Daniel Sun
> Priority: Major
>
> If you use {{VariableExpression.THIS_EXPRESSION}} the STC and thus
> static-compile gets confused.
> I assume this is a bug, if it is expected, sorry for the noise.
> We use this at some places in the Spock transform.
> While trying to make Spock specs compatible with STC and SC, it was observed
> that STC stores type information at the current node and using
> {{VariableExpression.THIS_EXPRESSION}} then causes conflicting casts when
> used inside and outside a closure.
> As a work-around we can instead use {{{}new VariableExpression("this"){}}},
> but I guess this actually is a bug, thus this report.
> As an example.
> {code:java}
> class ASpec extends Specification {
> def "mock init closure"() {
> given:
> List<String> list = Mock(List) {
> size() >> 3
> }
> when:
> int size = list.size()
> then:
> size == 3
> }
> }
> {code}
> compiles in semantic analysis to
> {code:java}
> import spock.lang.*
> @org.spockframework.runtime.model.SpecMetadata(filename = 'script.groovy',
> line = 1)
> public class ASpec extends spock.lang.Specification {
> @org.spockframework.runtime.model.FeatureMetadata(name = 'mock init
> closure', ordinal = 0, line = 2, blocks =
> [@org.spockframework.runtime.model.BlockMetadata(kind =
> org.spockframework.runtime.model.BlockKind.SETUP, texts = []),
> @org.spockframework.runtime.model.BlockMetadata(kind =
> org.spockframework.runtime.model.BlockKind.WHEN, texts = []),
> @org.spockframework.runtime.model.BlockMetadata(kind =
> org.spockframework.runtime.model.BlockKind.THEN, texts = [])], parameterNames
> = [])
> public void $spock_feature_0_0() {
> org.spockframework.runtime.ValueRecorder $spock_valueRecorder = new
> org.spockframework.runtime.ValueRecorder()
> org.spockframework.runtime.ErrorCollector $spock_errorCollector =
> org.spockframework.runtime.ErrorRethrower.INSTANCE
> org.spockframework.runtime.SpockRuntime.callBlockEntered(this, 0)
> java.util.List<String> list =
> org.spockframework.runtime.SpecInternals.MockImpl(this, 'list',
> java.util.List, java.util.List, { ->
>
> this.getSpecificationContext().getMockController().addInteraction(new
> org.spockframework.mock.runtime.InteractionBuilder(5, 7, 'size() >>
> 3').addEqualTarget(it).addEqualMethodName('size').setArgListKind(true,
> false).addConstantResponse(3).build())
> })
> org.spockframework.runtime.SpockRuntime.callBlockExited(this, 0)
> org.spockframework.runtime.SpockRuntime.callBlockEntered(this, 1)
> java.lang.Integer size = list.size()
> org.spockframework.runtime.SpockRuntime.callBlockExited(this, 1)
> org.spockframework.runtime.SpockRuntime.callBlockEntered(this, 2)
> try {
>
> org.spockframework.runtime.SpockRuntime.verifyCondition($spock_errorCollector,
> $spock_valueRecorder.reset(), 'size == 3', 12, 5, null,
> $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(2),
> $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(0),
> size) ==
> $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(1), 3)))
> }
> catch (java.lang.Throwable $spock_condition_throwable) {
>
> org.spockframework.runtime.SpockRuntime.conditionFailedWithException($spock_errorCollector,
> $spock_valueRecorder, 'size == 3', 12, 5, null, $spock_condition_throwable)}
> finally {
> }
> org.spockframework.runtime.SpockRuntime.callBlockExited(this, 2)
> this.getSpecificationContext().getMockController().leaveScope()
> }
> }
> {code}
> The important part here is the two {{getSpecificationContext()}} calls, one
> inside the mock init closure, one as last line in the feature method, both
> added by the Spock transform.
> If those use {{VariableExpression.THIS_EXPRESSION}} then at runtime you get
> {{{}java.lang.ClassCastException: apackage.ASpec cannot be cast to
> apackage.ASpec$__spock_feature_0_0_closure1{}}}.
> If you use {{new VariableExpression("this")}} instead, it works as intended.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
