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

Mon Sep 18 14:28:25 UTC 2017

On 18/09/17 14:28, B. Blaser wrote:
> 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)?
What isn't clear? That the supertype of a raw type are erased? That part 
is in section 4.4:

"The superclasses (respectively, superinterfaces) of a raw type are the 
erasures of the superclasses (superinterfaces) of any of the 
parameterizations of the generic type. "

>
>> 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?
That specific changeset is here

http://hg.openjdk.java.net/valhalla/valhalla9/langtools/rev/f0d0dd18b996

But I don't think it's 100% related - that is, that changeset took care 
of the well-formedness part, but it didn't touch how inference checked 
constraints.
>
>> 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."
We still do something similar to the above. Note that 
Infer.instantiateMethod is not the code used during a check in a method 
context. This code is responsible for that:

http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java#l325

In any case, changing the compiler behavior in this area is a big 
change, and should be considered carefully, and as part of a more 
holistic effort to bring the compiler in sync with the spec.

Maurizio
>
> 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();
>>