[
https://issues.apache.org/jira/browse/GROOVY-12754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18142908#comment-18142908
]
ASF GitHub Bot commented on GROOVY-12754:
-----------------------------------------
paulk-asert commented on PR #3285:
URL: https://github.com/apache/groovy/pull/3285#issuecomment-6793082468
Thanks Scott, the capture-based revision is a big improvement. I ran a set
of `@CompileStatic` probes against master and against this branch (and the same
patch applied to `GROOVY_6_0_X`; results were identical on both). The headline
cases behave as described, and subtype arguments, multi-parameter methods,
constructors, class-level bounds and DGM calls on wildcard receivers all came
out fine. Two things before this goes further.
**1. A bound that names another type parameter is skipped and never
checked.**
In `compatibleCapture`:
```java
if (bound.isGenericsPlaceHolder()) continue; // another type parameter
```
Both of these are rejected on master and compile on this branch:
```groovy
static <T extends Number, U extends T> U v(List<U> b) { b.get(0) }
static g(List<? extends String> l) { Number n = v(l); n }
// g(['x']) -> GroovyCastException: Cannot cast object 'x' with class
'java.lang.String' to class 'java.lang.Number'
static <T extends Number, U extends List<T>> int v(Class<U> c) { 1 }
static g(Class<? extends ArrayList<String>> c) { v(c) }
```
javac 25.0.5 rejects both ("inference variable T has incompatible bounds"),
and accepts the same calls with `List<? extends Integer>` and `Class<? extends
ArrayList<Integer>>`. Could you add them to `GenericsSTCTest` and
`GenericsJavaCompatibilityTest` and resolve the bound through the other type
parameter's own bound rather than skipping it? This is the same kind of hole as
`narrow`, on calls the PR newly accepts, so I'd like it closed before merging
to master.
**2. Backport to `GROOVY_6_0_X`.**
I'd prefer not to cherry-pick this as it stands. Matching javac here means
that code which compiles and runs correctly on 6.0.0 becomes a compile error in
a patch release:
- `Enum.valueOf(type, name)` and `EnumSet.allOf(type)` with a `Class<?
extends Enum<?>>`, which is a natural way to type an "any enum class" parameter
or field
- `Collections.sort(list)` with a `List<? extends Comparable<?>>`
- any wildcard declared looser than the method needs, e.g. a `Class<?
extends Runnable>` holding `Thread` passed to `<T extends Thread> m(Class<T>)`
The workarounds are easy and the COMPATIBILITY entry describes them well,
but that entry is rightly headed "Groovy 7", and #3286 extends the same
tightening to `Class<?>` and `List<?>`.
If GROOVY-12755 needs the accepting half on 6.0.x, one option is an
accept-only variant for that branch: when the capture check fails, fall through
to the existing `compatibleConnection` path instead of returning incompatible.
That admits the calls that were wrongly rejected without rejecting anything
6.0.0 accepts. It leaves the `narrow` miscompile in place on 6, which is
existing behaviour. I haven't built that variant, so treat it as a suggestion;
happy to hear other views, @daniellansun included, since this differs from the
earlier "cherry-pick once it matches javac".
One observation, not a request: with `m(List<T extends Integer>)` and
`m(Collection<?>)`, passing a `List<? extends Number>` is now an error rather
than selecting the second overload as javac would. That's how the checker
already treats a failed generics check, and it does mean there is no silent
re-dispatch, so I'm fine leaving it.
> 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)
