paulk-asert commented on PR #3087:
URL: https://github.com/apache/groovy/pull/3087#issuecomment-6510120674
Thanks @jdaugherty, the diagnosis is right and the fix is at the right
level. `TernaryExpression.getType()` now agrees with
`StatementMetaTypeChooser.resolveType()` about class literals, so a nested
ternary is typed the way the bytecode writer types its branches. Static
compilation is unaffected because it uses its own inferred type.
I probed a few more shapes against 4.0.33, 5.1.3, 6.0.0 and master, with and
without this PR:
| Shape | 4.0.33 | 5.1.3 / 6.0.0 / master, indy | 5.1.3 / 6.0.0, classic |
this PR |
|---|---|---|---|---|
| reported case (return, typed local, argument, `First.class` form,
`@TypeChecked`) | ok | ClassCastException | ok | ok |
| `(a ? First : Second) ?: (b ? Third : Fourth)` | ok | ClassCastException |
ok | ok |
| `b ? (First ?: Second) : (Third ?: Fourth)` | ok | ClassCastException | ok
| ok |
| `(a ? First : Second) ?? (b ? Third : Fourth)` (master only) | – |
ClassCastException | – | ok |
| `a ? (b ? First : Second) : marker` (`Marker marker`) | ok |
GroovyCastException | **GroovyCastException** | ok |
| `a ? (b ? String[] : Integer[]) : (b ? Long[] : Short[])` | ok |
GroovyCastException | **GroovyCastException** | ok |
So `?:` and `??` (both `ElvisOperatorExpression`, a `TernaryExpression`) are
fixed with no further change. The last two rows also fail with classic call
sites, so the problem isn't limited to indy. That's worth a note on the JIRA,
and it strengthens the case for the backports.
Could you add those shapes to the test? Something like:
@Test
void testNestedTernaryOfClassLiteralsIsJavaLangClass() {
assertScript '''
interface Marker {}
class First implements Marker {}
class Second implements Marker {}
class Third implements Marker {}
class Fourth implements Marker {}
String name(Class<?> type) { type.simpleName }
boolean a = false, b = true, c = true
assert name(a ? b ? First : Second : c ? Third : Fourth) ==
'Third'
assert name(b ? (a ?: c ? First : Second) : Third) == 'First'
Class<?> chosen = a ? b ? First : Second : c ? Third : Fourth
assert chosen == Third
// Elvis operands
chosen = (a ? First : Second) ?: (c ? Third : Fourth)
assert chosen == Second
chosen = a ? (First ?: Second) : (Third ?: Fourth)
assert chosen == Third
// a class literal versus an instance of the class it names
Marker marker = new Fourth()
Object either = b ? (c ? First : Second) : marker
assert either == First
// array class literals
chosen = a ? (b ? String[] : Integer[]) : (b ? Long[] : Short[])
assert chosen == Long[]
'''
}
On master only, add the null-coalescing form as well:
chosen = (a ? First : Second) ?? (c ? Third : Fourth)
assert chosen == Second
Nit: in `TernaryOperatorsTest` the static import sits above `import
java.util.regex.Pattern`. Please move it below the regular imports.
--
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]
