[lworld] RFR: Make "PrimitiveParameterizedClass.default" a poly expression.

[Maurizio Cimadamore]

On Fri, 19 Mar 2021 10:57:22 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Make .default a separate node type in the parser.
>> 
>> ## Issue
>> [JDK-8211914](https://bugs.openjdk.java.net/browse/JDK-8211914): [lworld] Javac should support type inference for default value creation
>> 
>> Note: The Linux x86 builds in GitHub actions seem to fail with something completely unrelated to these changes.
>
> Nice work - it's a very nice cleanup of the code base! I like how the code that was previously scattered in several places now can just move where it belongs.
> 
> Functionality-wise, I'm not sure this supports poly expressions yet - e.g. I see that you set the polyKind tag correctly, but I'm afraid that this along is not sufficient to support stuff  like:
> 
> List<String> ls = List.default;
> 
> or:
> 
> void m(List<String> ls) { ... }
> 
> m(List.default)
> 
> I think you might be led to believe that the current impl is doing the right thing - but my feeling is that javac might be tricking you: if the expected type is e.g. `List<String>` but the qualifier of the default expression is `List`, the compiler will check (as per checkId) that List <: List<String> - which is true by subtyping conversion. I don't think there is any inference going on, in other words. You might try debugging and looking at the type of the expression that comes out of checkId.
> 
> Anyway, even if this works and I did miss something, I don't see how this approach can scale to default expression passed as method arguments - given that in that case there is no `pt()`. To add support for that you need to add another case/node in `ArgumentAttr` (see what's been done for anonymous inner classes - this should be simpler as there's no constructor call).
> 
> In other words, you want Attr.visitApply to treat your default expression method argument in a special way - by creating what we call a "deferred type" - that is a type that is not fully inferred until _after_ overload resolution (when you DO know the pt()).
> 
> One possibility would be to push this change as is - as the refactoring part is really good - and maybe issue raw type warning when generic types are involved, or when default expressions are used in method context. And then in another, separate PR we can refine what happens in truly poly cases.

So, I debugged what happens in case where we have

`List<String> ls = List.default`

What I missed is that we create a variable symbol for `default` which has the right type (from the expected type) - so the expression is correctly typed as `List<String>`. But this seems to happen "just because" the compiler detects that the expected type has same base class as the default expression type. Then, the check performed by `checkId` is always vacuously satisfied, given that we're passing in a synthetic variable whose type is the same as the expected one.

Counter example:

interface Foo<X extends Number> extends List<X> { }
...
List<String> ls = Foo.default; // still ok, type is `Foo`

This example is more complex, for two reasons:

* in this case, Foo is a sub-interface, so the trick of saying "if base class is the same just use the expected type" doesn't work. In fact the compiler types this with `Foo` - which is a raw type
* in this particular instance, the code should not even be allowed - because the constraint on the `Foo` type parameter are incompatible with those of the expected type List<String> (X must extend Number).

All this stuff needs to work correctly if we want to claim that default expressions are true poly expressions.

-------------

PR: https://git.openjdk.java.net/valhalla/pull/369