[
https://issues.apache.org/jira/browse/GROOVY-12550?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136934#comment-18136934
]
ASF GitHub Bot commented on GROOVY-12550:
-----------------------------------------
jdaugherty commented on code in PR #3079:
URL: https://github.com/apache/groovy/pull/3079#discussion_r4449834961
##########
src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java:
##########
@@ -1475,7 +1475,9 @@ protected ClassNode createClosureClass(final
ClosureExpression expression, final
// GROOVY-9971: closure return type is mapped to Groovy cast by
classgen
ClassNode returnType =
expression.getNodeMetaData(INFERRED_RETURN_TYPE);
if (returnType == null) returnType = ClassHelper.OBJECT_TYPE; // not
STC or unknown path
- else if (returnType.isPrimaryClassNode()) returnType =
returnType.getPlainNodeReference();
+ // the class node itself carries the declared type parameters; a
parameterized reference to a class of the
+ // compilation unit keeps its type arguments, as a reference to a
class read from bytecode does
+ else if (returnType.isPrimaryClassNode() &&
!returnType.isRedirectNode()) returnType = returnType.getPlainNodeReference();
Review Comment:
You are seeing right, the two blocks were identical. The copy came in with
the packed closure path, and this PR touched both. `createClosureClass` now
calls `inferredClosureReturnType`: 75775b7553.
> STC: a closure's doCall drops the type arguments of a return type whose class
> is in the same compilation unit
> -------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12550
> URL: https://issues.apache.org/jira/browse/GROOVY-12550
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation
> Affects Versions: 4.0.33, 5.1.3
> Reporter: James Daugherty
> Priority: Major
>
> With static type checking, the {{doCall}} method of a closure is declared
> with the closure's inferred return type. When that type is a parameterized
> type whose class is in the same compilation unit, its type arguments are
> dropped. When the class is read from the class path, they are kept. The same
> source therefore compiles to a closure class with a different signature
> depending on what else is compiled, which incremental builds and build caches
> see as a change.
> {code:groovy}
> interface Up {}
> class Res<T> { List<T> items = [] }
> @groovy.transform.CompileStatic
> class Svc {
> static <V> V timeIt(Closure<V> body) { body.call() }
> def <U extends Up> Res<U> read(Class<U> type) { new Res<U>() }
> Res<Up> load(Class<Up> type) { timeIt { -> read(type) } }
> }
> {code}
> {noformat}
> Svc compiled together with Res: public Res doCall()
> Svc compiled against a compiled Res: public Res<Up> doCall()
> {noformat}
> Reproduced with 4.0.33, 5.1.3 and master. A reproducer with a groovyc script
> and a Gradle build:
> https://github.com/jdaugherty/groovy-closure-return-type-signature
> Grails applications hit this in incremental builds: {{@Transactional}} and
> similar transformations move a method body into a new method without the
> method's type parameters, so a closure there returns a concrete parameterized
> type.
> *Cause*
> {{ClosureWriter.createClosureClass}} normalises the inferred return type with:
> {code:java}
> else if (returnType.isPrimaryClassNode()) returnType =
> returnType.getPlainNodeReference();
> {code}
> {{isPrimaryClassNode()}} is true for the class node of a class in the
> compilation unit and for every reference to it, including a parameterized one
> such as {{Res<Up>}}, and {{getPlainNodeReference()}} drops the type
> arguments. A reference to a class read from bytecode is not primary, so it
> keeps them unless they are unresolved. On master,
> {{ClosureWriter.inferredClosureReturnType}} does the same for packed closures.
> *Proposed fix*
> Only replace the class node itself, which carries the declared type
> parameters, by its plain reference: {{returnType.isPrimaryClassNode() &&
> !returnType.isRedirectNode()}}. A parameterized reference then goes through
> the same check for unresolved generics as any other type, and both builds
> produce {{Res<Up> doCall()}}, the signature a closure already gets for a
> parameterized library type such as {{List<String>}}.
> Branches with the fix and a test, {{ClosureReturnTypeSignatureTest}}, that
> compiles the example both ways and compares the {{doCall}} signatures. The
> full core test suite passes on both:
> * master:
> https://github.com/jdaugherty/groovy/tree/closure-return-type-signature
> * GROOVY_5_1_X:
> https://github.com/jdaugherty/groovy/tree/closure-return-type-signature-5_1_X
> Related: GROOVY-10932 and GROOVY-12544, other differences between classes of
> the compilation unit and pre-compiled classes that affect incremental builds.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
