[
https://issues.apache.org/jira/browse/GROOVY-12754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18142714#comment-18142714
]
ASF GitHub Bot commented on GROOVY-12754:
-----------------------------------------
codeconsole commented on PR #3285:
URL: https://github.com/apache/groovy/pull/3285#issuecomment-6786530428
Thanks for the detailed review. I reproduced the compile results against
javac 25.0.4 and pushed a6322b5142.
**The bound check.** The `? super B` shortcut is gone. A wildcard that is a
type argument of the argument's type is now treated as its capture: a fresh
type variable bounded by the wildcard's bound and by the bound declared for
that type argument. It is checked against every bound of the type parameter,
with type arguments and with the capture in place of the type parameter, and
then the captured argument is checked against the parameter. With that, these
are rejected again, under `@TypeChecked` and `@CompileStatic`:
- `<T extends Comparable<T>>`, `<T extends Number & Comparable<T>>` and
`cmp` on `List<? extends Integer>`, `<T extends Enum<T>>` on `List<? extends
Thread.State>`, and the `Base`/`Sub` case
- `<T extends List<Integer>>` on `Class<? extends ArrayList<String>>`,
`Class<? extends List<String>>` and `List<? extends ArrayList<String>>`, `<T
extends List<? extends Integer>>`, and `ListBox<? extends ArrayList<String>>`
- `<T extends CharSequence & Appendable>` on `List<? extends String>`, so
the `append` cast failure and the `head` call that returned `120` no longer
compile
- `take(List<List<T>>)` on `List<List<? extends Integer>>` and `take(Map<T,
T>)` on `Map<? extends Integer, ? extends Integer>`
`Class<? extends List<String>>` for `<T extends List<Integer>>` compiled on
master as well; it is rejected now. So are a few calls that master accepted
because it compared only the erasure of the first bound, and that javac
rejects: `Enum.valueOf(type, name)` with a `Class<? extends Enum<?>>`,
`Collections.sort(list)` with a `List<? extends Comparable<?>>`, and `List<?
extends Serializable>` for `<T extends Number>`. `<T extends Comparable<? super
T>>`, `<T extends Number & Comparable<? super T>>` and `<T extends List<?
extends Number>>` are accepted for the matching wildcards, as javac accepts
them.
**`? extends E`.** A wildcard bounded by a type variable is checked through
the bound of that type variable, so the three you listed match javac: `<E
extends Integer>` is accepted for `<T extends Number>`, and `<E extends
Number>` for `narrow` and `<E>` for `first` are rejected.
**Tests.** Each of those programs is in `GenericsSTCTest` (so
`GenericsStaticCompileTest` runs it too), and the Java forms are in
`GenericsJavaCompatibilityTest`. The new negative tests fail on the previous
revision, and the new positive ones fail on master.
**`?`, `? extends Object` and `? super`.** These are done as well, in a
separate commit on #3286 (2b99f21e83) rather than here. With it `List<?>`,
`List<? extends Object>`, `List<? super Integer>` and `List<? super Number>`
are rejected for `<T extends Number>`, and `Class<?>` and `Class<? extends
Object>` for `<T extends List<Integer>>`, while `NumBox<?>` and `NumBox<? super
Integer>` for a `NumBox<N extends Number>` and `EnumSet<?>` for `<T extends
Enum<T>>` stay accepted, as in javac. The tests are in the same three classes.
It is on #3286 because of what it does before GROOVY-12755:
- `x.getClass()` and `x.class` are typed `Class<?>` until GROOVY-12755, so
`foo(l.class, l)` in `testMethodLevelGenericsForMethodCall` (GROOVY-5891)
fails, and `<T extends CharSequence> len(Class<T>)` rejects
`len(s.getClass())`, which javac accepts.
It also makes code that compiles today an error, as it is in javac:
`Enum.valueOf(type, name)` and `method.getAnnotation(type)` with a `Class<?>`,
`Collections.sort(list)` with a `List<?>`. If you prefer it in this PR, I can
move it here and change the GROOVY-5891 test.
Two parts of the review are not changed. For each I've described what the
change involves; I'm happy to make either, here or as a follow-up, whichever
you prefer.
**1. `First.<Integer>first(ints)`.** Explicit type arguments are not
compared with the arguments on master: `C.<String>id(listOfInteger)` and
`C.<Integer>first(listOfNumber)` compile there. Master rejected the listed call
only because it rejected every `? extends` argument for a bounded type
parameter. Rejecting it needs a check of the arguments against the explicit
type arguments, which would also reject those two calls.
**2. `Double d = first(arg)`.** The call is typed `Integer`, and the
assignment then follows Groovy's assignment rules, which let an `Integer` be
assigned to a `Double`. `Double d = first(listOfInteger)` compiles on master in
the same way and also gives `4.0`, while `Date d = first(arg)` is rejected.
Rejecting it would mean changing that rule, or using the target type in
inference for this case.
One more thing I noticed while testing: with an unbounded type parameter,
`<T> void take(Map<T, T>)` on `Map<? extends Integer, ? extends Integer>` and
`<T> void take(List<List<T>>)` on `List<List<? extends Integer>>` compile on
master, and javac rejects both. This PR does not change those.
Understood on GROOVY_6_0_X. Both commits of this PR cherry-pick onto it
without conflicts, for when the open points above are settled.
> STC: an upper-bounded wildcard argument is checked against a type parameter's
> bound the wrong way round
> -------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12754
> URL: https://issues.apache.org/jira/browse/GROOVY-12754
> Project: Groovy
> Issue Type: Bug
> Components: Static Type Checker
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> Static type checking rejects a call that passes an upper-bounded wildcard,
> such as {{List<? extends Integer>}}, to a generic method whose type parameter
> is bounded by a supertype of the wildcard's bound. It accepts the call when
> the wildcard's bound is wider than the type parameter's. javac does the
> opposite in both cases.
> {code:groovy}
> @groovy.transform.CompileStatic
> class C {
> static <T extends Number> T first(List<T> list) { list.get(0) }
> static <T extends CharSequence> int len(Class<T> type) { 0 }
> static <T extends Integer> T narrow(List<T> list) { list.get(0) }
> static Object a(List<? extends Integer> ints) { first(ints) } //
> rejected, javac accepts
> static int b(Class<? extends String> type) { len(type) } //
> rejected, javac accepts
> static Object c(List<? extends Number> nums) { narrow(nums) } //
> accepted, javac rejects
> }
> {code}
> {noformat}
> [Static type checking] - Cannot call <T extends java.lang.Number>
> C#first(java.util.List<T>) with arguments [java.util.List<? extends
> java.lang.Integer>]
> [Static type checking] - Cannot call <T extends java.lang.CharSequence>
> C#len(java.lang.Class<T>) with arguments [java.lang.Class<? extends
> java.lang.String>]
> {noformat}
> The capture of {{? extends B}} is a subtype of {{B}} (JLS 5.1.10), so it is
> within the bound of {{T}} exactly when {{B}} is. javac reports "inference
> variable T has incompatible bounds" for {{c}}. Groovy compiles {{c}}, and
> {{c([1.5d])}} returns {{1}}.
> {{StaticTypeCheckingSupport#compatibleConnection}} tests a wildcard witness
> with {{connection.isCompatibleWith(bound)}}, which asks whether the type
> parameter's bound fits within {{? extends B}}. For a type argument {{B}}, it
> tests {{? super B}} against the bound. An upper-bounded wildcard should take
> the same test.
> PR: https://github.com/apache/groovy/pull/3285
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
