codeconsole commented on PR #3286:
URL: https://github.com/apache/groovy/pull/3286#issuecomment-6784788096
@daniellansun thanks. I ran each program against javac and against the
branch. Three new commits on top of f2d84108eb:
1. **Simple name of an inner class.** `ResolveVisitor` now records the
enclosing type of such a name. `Inner` is `Outer<T>.Inner` within `Outer<T>`,
and `Outer<String>.Inner` within a subclass of `Outer<String>`. Both of your
programs are now errors, and the two tests are in `GenericsSTCTest` and,
against javac, in `GenericsJavaCompatibilityTest`:
```java
Class<? extends Inner> f() { return getClass(); } // inside
Outer<T>.Inner
Class<? extends Inner> x = c; // c is a Class<?
extends Outer.Inner>
```
The inherited name is covered too: in `Sub extends Outer<String>`,
`Class<? extends Inner> x = inner.getClass()` is an error. In the other
direction, two forms that the previous tip rejected and javac accepts now
compile, because the two names are the same type: `Class<? extends
Outer<T>.Inner> y = x` for a `Class<? extends Inner> x`, and `Class<? extends
Outer<String>.Inner> y = x` in `Sub`.
Left as they were, on purpose:
- Where the type variables are out of scope (a static method or field, or
under a method type parameter of the same name) the name stays raw. javac
rejects the name there; Groovy accepts it.
- A class literal, `instanceof Inner` and `new Inner[n]` still name the
class. javac rejects the last two for `Outer<T>.Inner`. Groovy accepts them
today, and I did not want this PR to break `equals` methods and array creation.
- A nested record, enum or interface is static, so its name has no
enclosing type. A generic inner class used without type arguments stays raw.
This is a resolver change, so it reaches further than the rest of the PR:
it applies to every such name, with or without static compilation. Outside the
type checker the visible effect is the `Signature` attribute. `Inner make()` in
`Outer<T>` now gets `()LOuter<TT;>.Inner;`, which is what javac writes; the
positive test compares the generic signatures with javac's.
2. **Diagnostic.** A use of an inner class with no enclosing type prints as
the raw type, so the message is `Cannot assign java.lang.Class<? extends
Outer.Inner> to: java.lang.Class<? extends Outer<T>.Inner>`. The declaration
itself still prints as `Outer<T>.Inner`.
3. **The three assignments.** The second and third are now errors. Within
the bounds of two wildcards, a type variable is taken as its bound, and a type
argument that is a raw type is not accepted for a parameterized one:
```java
Class<? extends List<List<String>>> g(Class<? extends List<List>> c) {
return c; }
<T extends List> Class<? extends List<String>> h(Class<? extends T> c) {
return c; }
```
The first one, `Class<ArrayList>` for `Class<? extends List<String>>`, is
unchanged. I tried it: treating a raw type argument like a raw bound rejects
your program and `Class<? extends List<String>> c = ArrayList.class`, but it
also rejects these, which javac accepts because it infers the element type from
the target:
```java
List<? extends List<String>> a = List.of(new ArrayList());
List<? extends List<String>> b = Arrays.asList(new ArrayList());
Map<String, ? extends List<String>> m = Map.of("k", new ArrayList());
```
and with them the Groovy forms `[new ArrayList()]`, `[k: new
ArrayList()]` and `Optional.of(new ArrayList())`. STC types all of those
bottom-up as `List<ArrayList>`, and the check cannot tell that from a declared
`List<ArrayList>`. So it needs target typing for those expressions first, or it
turns programs that javac accepts into errors. It is also not on the
`getClass()` path, which always gives a wildcard. I think it should be its own
issue.
4. **`instanceof`.** I left this one, and added a test that states it. In
the branch of `if (o instanceof String)`, STC gives `o` the type `String` for
every member: `o.substring(0)` and `String s = o` compile there, and javac
rejects both, just as it rejects `o.getClass()` for a `Class<? extends
String>`. `getClass()` takes the receiver type that any other call on `o` takes
at that point, which is the role the pattern variable has in Java, where the
two compilers agree. Using the declared type for `getClass()` only would make
it the one member that sees a different receiver. After the branch
`o.getClass()` is `Class<?>` again, and the test covers that too. If you still
want the declared type here, say so and I'll change it.
5. **Other differences** I found while checking these. None of them is on
the `getClass()` path:
- `Class<? extends List<String>> x = rawHolder.type`, for `class Holder<T
extends List> { Class<? extends T> type }`, is rejected since 7807d56395. javac
gives an unchecked warning. STC reads a member of a raw receiver with `T`
replaced by its bound, `Class<? extends List>`, where javac erases the member
type to `Class`.
- `List<Outer.Inner>` is accepted for `List<? extends Inner>` and for
`List<Inner>` (javac rejects both). That is a raw type argument again, as in 3.
- An inherited inner class named from a class nested in the subclass does
not resolve at all (`unable to resolve class Inner`). Master does the same.
The new negative tests fail on the previous tip, and so does the positive
simple-name test. The `instanceof` test passes there, since it records
behaviour that is not changed. The whole build's tests pass locally.
For GROOVY_6_0_X: the seven commits cherry-pick cleanly onto the current
branch, and the only failure in the core tests there is still the `inClosure()`
line of `testGetClassIsClassOfReceiverErasure2` (GROOVY-12576). Given the reach
of the resolver change, you may want to decide on that commit separately for
the port.
--
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]
