codeconsole commented on PR #3286:
URL: https://github.com/apache/groovy/pull/3286#issuecomment-6812790172
@daniellansun @paulk-asert thanks, both. I compiled each program against
javac 25.0.4, against master and against this branch. New commits on top of
f3e8d5dc01, after merging #3285 again:
**Generic calls in an assignment or return context** (4ef7a2f21d). A call of
a generic method is now given its type arguments from the target type as well,
as in JLS 18.5.2. The call is accepted when the type arguments the target
determines keep the method's bounds and every argument fits its parameter with
them; closures and lambdas are checked by their return type. This is not
containment: `List<Class<? extends String>>` is still not a `List<Class<?>>`.
- These compile again: `Collections.singletonList(x.getClass())`,
`Arrays.asList(a.getClass(), b.getClass())`, `Optional.of(x.getClass())`,
`Collections.singletonMap` and `Collections.singleton` as `Map<String,
Class<?>>` and `Set<Class<?>>`, a field initializer, a return, and
`list.collect { it.getClass() }` for a `List<Class<?>>`.
- The same rule also accepts `List<Number> l =
Collections.singletonList(1)`, `Arrays.asList(1, 2)` for a `List<Number>`,
`List<Object> l = List.of("a")` and a `Supplier` lambda, which master rejected
and javac accepts.
- These are still rejected, as in javac: `Collections.nCopies(2, "x")` for a
`List<Number>`, `singletonList(x.getClass())` for a `List<Class<? extends
Number>>`, `<T extends Number> List<T> numbers(T)` for a `List<Object>`, and a
`collect` whose closure returns a `String` for a `List<Integer>`.
`list*.getClass()` and `list*.class` stay rejected for a `List<Class<?>>`,
as Daniel asked, while `collect` compiles. Paul, you asked for the spread forms
to compile too. That would mean treating a `List<Class<? extends String>>`
value as a `List<Class<?>>`, which Daniel asked me not to do. I can make it
work for the spread forms alone if you agree between you that you want that.
Two differences from javac are left, and both predate this PR: `List<Object>
l = s.stream().collect(Collectors.toList())` is rejected (inference through the
nested `toList()` call), and `Comparator<Object> c = Comparator.naturalOrder()`
is accepted.
**Overload selection** (4ac91c0b79).
`GenericsSTCTest.testClassOfReceiverSelectsOverloadForItsType` records
`f(StringBuilder)` for `f(x.getClass().cast(o))`,
`f(x.getClass().getDeclaredConstructor().newInstance())` and
`f(create(x.getClass()))`; under `GenericsStaticCompileTest` it fails on
master, which selects `f(Object)`. `GenericsJavaCompatibilityTest` compares the
first two with javac. The COMPATIBILITY entry now describes this, and the
target-type rule above.
**`instanceof`** (471cb3b9da). `o.getClass()` and `o.class` on a variable
narrowed by `instanceof` now take the variable's type without the test, so your
`f(Object o)` is an error again. A pattern variable, a cast and a new local are
`Class<? extends String>`. `def store = new Impl()` keeps `Impl`, because that
is not an `instanceof` narrowing. `o.substring(0)` and `String s = o` in that
`if` are unchanged, as you said.
**A member of a raw type** (487b2eca2f). Your `holder.type` compiles again,
and so does a method of that type. I made it narrower than all of JLS 4.8: a
member of a raw type is erased when its type uses a type parameter whose bound
is a raw type, as `T extends List` does, which is the case where the
substituted bound became `Class<? extends List>`. Erasing every member of a raw
type, as javac does, changes two existing tests:
- `GenericsSTCTest.testIncompatibleArgumentsForPlaceholders3` expects errors
for `stringProperty.eq(1234)` and `numberProperty.eq("xx")` through a raw
`Holder` delegate. With erasure those calls compile, as they do in javac with
an unchecked warning.
- `BugsStaticCompileTest.testShouldAllowSubscriptOperatorOnSet` then fails
in instruction selection on `set[0]` for `new
LinkedHashMap([a:1,b:2]).keySet()`.
I'm happy to do the full erasure, with those two tests changed and the
instruction-selection failure fixed, if you want it here.
**`List<Inner>` within `Outer<T>`** (190b50dceb). The enclosing type now
counts for a type argument, so `List<Inner> a = raw` and `List<? extends Inner>
b = raw` are rejected for a `List<Outer.Inner> raw`, and so are
`List<Outer.Inner> = List<Inner>` and `Map<String, Inner> = Map<String,
Outer.Inner>`. An enclosing type that still has type variables is not compared.
A member's type read through a parameterized receiver keeps `Outer<T>.Inner`,
because the enclosing type is not substituted from the receiver, and comparing
it rejected `List<Outer<String>.Inner> l = outer.list` for an `Outer<String>
outer`. A test covers that form too.
**`Class<T>` and a class type parameter** (a48ffb4db7). `<T> Class<T>
typeOf(T t) { t.getClass() }` and `Class<T> c = t.getClass()` are errors.
Within `Box<T>`, `accept(t.getClass())` for `accept(Class<? extends T>)` and
`byType.put(t.getClass(), t)` for a `Map<Class<? extends T>, T>` are errors, as
in javac. Before, the class's `T` was inferred from the argument; a parameter
typed by type variables in scope is now checked against the argument first. Two
older accepts remain: `<T> void put(Class<T>, T)` called as `put(t.getClass(),
t)` from a generic method, and `accept(String)` within `Box<T>`. Both are
rejected by javac and accepted on master. The `accept` error prints the
parameter as `Class<?>`, because the message substitutes the inferred argument;
that is how the message already read for `Class<?>`.
**Not changed**, as you suggested: `o instanceof Inner`, `new Inner[n]`, and
the simple name in a static method. Nor the narrower declared-type rule for
`Class<ArrayList>`; I can add that as its own commit if you want it.
**`GROOVY_6_0_X`.** The commits by part, for the split you are discussing:
- `getClass()` typing: 2ce6046aa3, f2d84108eb, after GROOVY-12754. The fixes
for Daniel's points that come with it: 4ef7a2f21d (target typing) and
471cb3b9da (`instanceof`).
- Raw bound: 7807d56395, 0750119074, and 487b2eca2f (raw member).
- Resolver: 053e7ead2d, and 190b50dceb (`List<Inner>`).
- Unbounded and lower-bounded wildcards: 2b99f21e83.
- `Class<T>`: a48ffb4db7.
I have not cherry-picked the new commits onto `GROOVY_6_0_X` yet. I'll do
that and run the tests there for whichever set you choose.
--
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]
