[enhanced-enums]: Enhanced enums failure with raw types?

[B. Blaser]

On 18 September 2017 at 10:50, Maurizio Cimadamore
<maurizio.cimadamore at oracle.com> wrote:
>
>
> On 16/09/17 12:27, B. Blaser wrote:
>>
>> On 15 September 2017 at 19:10, Maurizio Cimadamore
>> <maurizio.cimadamore at oracle.com> wrote:
>>>
>>> On 12/09/17 11:34, B. Blaser wrote:
>>>>
>>>> I agree with you, but this works with a warning (with the latest
>>>> "enhanced enums" version - I've not tried with the JDK 9).
>>>>
>>>> It seems the inference occurs only on the actual argument (E = G) and
>>>> then an unchecked cast is made from "EnumSet<G>" to
>>>> "EnumSet<G<Object>>" when returning a value, producing a warning.
>>>>
>>>> Is this a bug?
>>>
>>> This is a known non-confromance issue - see:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8026527
>>>
>>> In other words, the code passes because javac does an unchecked subtyping
>>> between G and G<Object> (this check would fail if standard subtyping were
>>> used, as mandated by the spec).
>>
>> I'm not sure we are really in the same situation.
>>
>> Here, the return type constraint is simply not incorporated because G
>> is raw, see [1] - Infer.instantiateMethod().
>
> I think it is the same, but perhaps the timing with which things occur is
> slightly different; from EnumSet.allOf, we have the infreence variable E,
> with the following constraint:
>
> E <: Enum<E>
>
> Then, from checking actual against formal, we get this additional
> constraint:
>
> Class<G> <: Class<E>
>
> Now, from (18.2.3), we derive that:
>
> G = E
>
> And then, from (18.3.1)
>
> G <: Enum<E>

All is right until there.

> So, to prove the above subtyping, we project the LHS to the supertype (which
> is erased, as G is raw):
>
> Enum <: Enum<E>

Intuitively, I think I agree, but this step isn't obvious.
Could you eventually point me to the JLS (I didn't find where it's mentioned)?

> And, at this point, you can only respond 'true' if you are doing an
> unchecked conversion. Otherwise (as in the JLS), the check will just fail.

Yes, given the non-conformance issue.

A JBS comment refers to a fix in Valhalla that is no more available [val].
Do you remember what was it?

> Do you agree?

I agree with this reasoning, if clarification of the "obscure" step above.

Of course, the fixes wouldn't be the same.

My suggestion incorporates the return type constraint even if a raw
type is found which seems to be compliant with the JLS 9 Proposed
Final Draft [jls9], §18.5.2.1 on page 715:

"If unchecked conversion was necessary for the method to be applicable
during constraint set reduction in §18.5.1, the constraint formula
<|R| -> T> is reduced and incorporated with B2."

Which would produce the expected error in our example.
What do you think?

Thanks,

Bernard

> Maurizio

[jls9] http://openjdk.java.net/projects/jdk9/spec/
[val] http://hg.openjdk.java.net/valhalla/valhalla/langtools/rev/f0d0dd18b996

>>
>> A simple fix would be to remove this check but I don't know if it
>> would have side-effects, as here under.
>> Removing it produces the expected error:
>>
>> A.java:12: error: incompatible types: inference variable E has
>> incompatible equality constraints G<Object>,G
>>          EnumSet<G<Object>> g3 = EnumSet.allOf(G.class);
>>                                               ^
>>    where E is a type-variable:
>>      E extends Enum<E> declared in method <E>allOf(Class<E>)
>> 1 error
>>
>> What do you think?
>>
>> Bernard
>>
>> [1]
>> http://hg.openjdk.java.net/amber/amber/langtools/file/7a22956a0562/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java#l201
>>
>>> Maurizio
>>
>> diff --git
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>> @@ -198,7 +198,7 @@
>>                   //inject return constraints earlier
>>                   doIncorporation(inferenceContext, warn); //propagation
>>
>> -                if (!warn.hasNonSilentLint(Lint.LintCategory.UNCHECKED))
>> {
>> +//                if
>> (!warn.hasNonSilentLint(Lint.LintCategory.UNCHECKED)) {
>>                       boolean shouldPropagate =
>> shouldPropagate(mt.getReturnType(), resultInfo, inferenceContext);
>>
>>                       InferenceContext minContext = shouldPropagate ?
>> @@ -216,7 +216,7 @@
>>                           deferredAttrContext.complete();
>>                           return mt;
>>                       }
>> -                }
>> +//                }
>>               }
>>
>>               deferredAttrContext.complete();
>
>