[
https://issues.apache.org/jira/browse/GROOVY-12559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18134510#comment-18134510
]
James Daugherty commented on GROOVY-12559:
------------------------------------------
While reviewing the pull request
([apache/groovy#3087|https://github.com/apache/groovy/pull/3087]) Paul King
probed more shapes, and two of them also fail with classic call sites, so the
problem is not limited to invokedynamic. Checked against the released jars on
JDK 21.0.12 ({{java -Dgroovy.target.indy=true|false -cp
groovy-<version>.jar[:groovy-callsite-6.0.0.jar] groovy.ui.GroovyMain ...}}):
||Shape||4.0.33||5.1.3 / 6.0.0, indy||5.1.3 / 6.0.0, classic||
|{{a ? (b ? First : Second) : (c ? Third : Fourth)}}
(reported)|ok|ClassCastException|ok|
|{{(a ? First : Second) ?: (c ? Third : Fourth)}}|ok|ClassCastException|ok|
|{{a ? (First ?: Second) : (Third ?: Fourth)}}|ok|ClassCastException|ok|
|{{b ? (c ? First : Second) : marker}} with {{Marker
marker}}|ok|GroovyCastException|GroovyCastException|
|{{a ? (b ? String[] : Integer[]) : (b ? Long[] :
Short[])}}|ok|GroovyCastException|GroovyCastException|
In the first three shapes both branches of the outer expression already have
the common supertype, so the only conversion written is at the use site, from
{{Marker}} to {{Class}}: with classic call sites that is
{{ShortTypeHandling.castToClass(Object)}}, which accepts the {{Class}} object,
while the indy call site {{cast:(LMarker;)Ljava/lang/Class;}} casts its
argument to {{Marker}} first. In the last two the conversion _to_ the common
supertype ({{Marker}}, or {{Serializable[]}} for the array literals) is written
inside the ternary, on the branch that is the inner ternary, as
{{ScriptBytecodeAdapter.castToType(Object, Class)}} with classic call sites and
{{cast:(Ljava/lang/Class;)LMarker;}} with indy, and fails either way:
{noformat}
GroovyCastException: Cannot cast object 'class First' with class
'java.lang.Class' to class 'Marker'
{noformat}
The change in the pull request types the class literal operand as {{Class<T>}}
before the common supertype is computed, and a master build with
{{groovy-callsite}} passes all five shapes, and the null-coalescing form {{(a ?
First : Second) ?? (c ? Third : Fourth)}}, with {{-Dgroovy.target.indy=true}}
and {{false}}. The test in the pull request and on the 6.0.x and 5.1.x backport
branches now covers these shapes. The classic call-site failures affect the
same versions, 5.1.3 and 6.0.0; 4.0.33 is not affected either way.
> Ternary of two ternaries of class literals is cast to the classes' common
> supertype under invokedynamic
> -------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12559
> URL: https://issues.apache.org/jira/browse/GROOVY-12559
> Project: Groovy
> Issue Type: Bug
> Components: Compiler
> Affects Versions: 5.1.3, 6.0.0
> Reporter: James Daugherty
> Priority: Major
>
> Since 5.0.0-alpha-4 (GROOVY-10936) the type of a ternary expression is the
> common supertype of its branches' types. A class literal's type is the class
> it names, not {{java.lang.Class}}, so a ternary that chooses between two
> ternaries of class literals
> {code:groovy}
> interface Marker {}
> class First implements Marker {}
> class Second implements Marker {}
> class Third implements Marker {}
> class Fourth implements Marker {}
> class Chooser {
> Class<?> choose(boolean a, boolean b, boolean c) {
> a ? (b ? First : Second) : (c ? Third : Fourth)
> }
> }
> assert new Chooser().choose(false, true, true) == Third
> {code}
> is typed as the classes' common supertype, {{Marker}}. The chosen {{Class}}
> object is then converted _from_ {{Marker}} wherever it is used: returned,
> passed as an argument, or assigned to a typed variable. With invokedynamic
> that conversion is a {{cast}} call site declared as {{(Marker)Class}}, and
> linking it casts the {{Class}} object to {{Marker}}:
> {noformat}
> java.lang.ClassCastException: Cannot cast java.lang.Class to Marker
> {noformat}
> Groovy 4, and Groovy 5 and 6 with classic call sites, return the chosen
> class. A single-level ternary of class literals, {{a ? First : Second}}, is
> unaffected, because the bytecode writer resolves a class literal operand to
> {{Class<First>}} itself; a ternary's own type is consulted only when it is an
> operand of another ternary. A Grails application moving from classic call
> sites to invokedynamic (the Grails 9 default) meets this in code that picks a
> class by conditions.
> *Reproducer:*
> https://github.com/jdaugherty/groovy-ternary-class-literal-reproducer
> ({{./run.sh 4.0.33 5.1.3 6.0.0}}): returning the chosen class and passing it
> as an argument both fail on 5.1.3 and 6.0.0 with indy, and pass on 4.0.33 and
> with classic call sites.
> *Cause:* {{TernaryExpression.getType()}} returns
> {{WideningCategories.lowestUpperBound()}} of its branches' {{getType()}}, and
> {{ClassExpression.getType()}} is the class the literal names.
> {{BinaryExpressionHelper.evaluateTernaryExpression()}} types each branch with
> {{StatementMetaTypeChooser.resolveType()}}, which answers {{Class<T>}} for a
> class expression but {{getType()}} for a nested ternary. So the inner
> ternaries are typed {{Marker}}, the outer ternary's common type is
> {{Marker}}, and the operand stack carries {{Marker}} for a value that is a
> {{Class}}. The conversion to {{Class}} is then written as {{invokedynamic
> cast:(LMarker;)Ljava/lang/Class;}}, whose method type makes the JVM cast the
> argument to {{Marker}} before the call site sees it. Classic call sites
> convert the value to {{Class}} directly and accept it.
> *Proposed fix:* in {{TernaryExpression.getType()}}, give a class literal
> operand the type {{Class<T>}}, as {{StatementMetaTypeChooser.resolveType()}}
> does, so a ternary of class literals is typed {{Class}} wherever it appears.
> A patch with a test follows as a pull request against master, with backports
> for the 6.0.x and 5.1.x lines.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
