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. -- 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]
