jdaugherty commented on PR #3087: URL: https://github.com/apache/groovy/pull/3087#issuecomment-6511117232
Thanks both. Your review and my push crossed, @daniellansun: everything you and @paulk-asert asked for is in 3cf5e647f5, and the JIRA note is up. **Test.** `testNestedTernaryOfClassLiteralsIsJavaLangClass` now covers the Elvis operands, a class literal against a `Marker` instance, the array class literals and, on master, the `??` form. The static import sits below the regular imports. The same additions without `??` are on the backport branches: [d87b7f831f](https://github.com/jdaugherty/groovy/commit/d87b7f831fec93e44cbd8e5a99c85e8a103d33c0) on `GROOVY-12559-6_0_X` and [af5d739a4c](https://github.com/jdaugherty/groovy/commit/af5d739a4c32bc73f765e726f05686ad7c95220f) on `GROOVY-12559-5_1_X`; the description links them. **Verification.** With the pre-fix `TernaryExpression.java` swapped in, the method fails on the first nested shape with `ClassCastException: Cannot cast java.lang.Class to Marker`; with the fix, `TernaryOperatorsTest` passes on all three branches. Paul's table reproduces exactly against the released 4.0.33, 5.1.3 and 6.0.0 jars with indy on and off, and a master build with `groovy-callsite` passes all seven shapes under both `-Dgroovy.target.indy=true` and `false`. **Why the last two rows fail with classic call sites too.** In the first three shapes both branches of the outer ternary already have the common supertype, so the only conversion written is at the use site, from `Marker` to `Class`: classic `ShortTypeHandling.castToClass(Object)` accepts the `Class` object, while indy's `cast:(LMarker;)Ljava/lang/Class;` casts the 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, through `castToType`, which fails either way. That is now on GROOVY-12559 with the table: https://issues.apache.org/jira/browse/GROOVY-12559?focusedCommentId=18134510 I left the test additions as a second commit so the delta is easy to review; happy to squash if you'd rather have one. -- 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]
