LUB computation involving NULL (was: switch expression type with 'null' return expression)

[B. Blaser]

Hi,

This one is interesting.

Unless I read JLS15 too quickly, neither §15.28.1 mandates explicitly
to skip null types before computing the LUB:

"Otherwise, boxing conversion (§5.1.7) is applied to each result
expression that has a primitive type, after which the type of the
switch expression is the result of applying capture conversion
(§5.1.10) to the least upper bound (§4.10.4) of the types of the
result expressions."

nor §4.10.4 explicitly specifies how to handle them.

So, ignoring null types not to interfere with this computation seems
implicitly deduced from the fact that they can match any reference
type and Jan's pull request [1] looks therefore reasonable.

However, compilation currently succeeds with the first arm being null
(as initially reported), suggesting probably to fix this more directly
in Types::lub as here under (langtools:tier1 being OK on jdk14u).

What do you think?

Thanks,
Bernard

[1] https://git.openjdk.java.net/jdk/pull/583

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
@@ -3970,33 +3970,29 @@

         case CLASS_BOUND:
             // calculate lub(A, B)
-            int startIdx = 0;
-            for (int i = 0; i < ts.length ; i++) {
-                Type t = ts[i];
-                if (t.hasTag(CLASS) || t.hasTag(TYPEVAR)) {
-                    break;
-                } else {
-                    startIdx++;
-                }
-            }
-            Assert.check(startIdx < ts.length);
             //step 1 - compute erased candidate set (EC)
-            List<Type> cl = erasedSupertypes(ts[startIdx]);
-            for (int i = startIdx + 1 ; i < ts.length ; i++) {
-                Type t = ts[i];
+            List<Type> cl = null;
+            for (Type t: ts) {
                 if (t.hasTag(CLASS) || t.hasTag(TYPEVAR))
-                    cl = intersect(cl, erasedSupertypes(t));
-            }
+                    cl = cl == null ? erasedSupertypes(t) :
intersect(cl, erasedSupertypes(t));
+            }
+            Assert.checkNonNull(cl);
+
             //step 2 - compute minimal erased candidate set (MEC)
             List<Type> mec = closureMin(cl);
             //step 3 - for each element G in MEC, compute lci(Inv(G))
             List<Type> candidates = List.nil();
             for (Type erasedSupertype : mec) {
-                List<Type> lci = List.of(asSuper(ts[startIdx],
erasedSupertype.tsym));
-                for (int i = startIdx + 1 ; i < ts.length ; i++) {
-                    Type superType = asSuper(ts[i], erasedSupertype.tsym);
-                    lci = intersect(lci, superType != null ?
List.of(superType) : List.nil());
+                List<Type> lci = null;
+                for (Type t: ts) {
+                    if (t.hasTag(CLASS) || t.hasTag(TYPEVAR)) {
+                        Type superType = asSuper(t, erasedSupertype.tsym);
+                        List<Type> stl = superType != null ?
List.of(superType) : List.nil();
+                        lci = lci == null ? stl : intersect(lci, stl);
+                    }
                 }
+                Assert.checkNonNull(lci);
+
                 candidates = candidates.appendList(lci);
             }
             //step 4 - let MEC be { G1, G2 ... Gn }, then we have that

On Fri, 9 Oct 2020 at 11:46, Maurizio Cimadamore
<maurizio.cimadamore at oracle.com> wrote:
>
> Filed this:
>
> https://bugs.openjdk.java.net/browse/JDK-8254286
>
> Maurizio
>
> On 09/10/2020 10:42, Maurizio Cimadamore wrote:
>
> Hi Anna,
> yes, this looks like a compiler bug.
>
> Behavior should be the same as:
>
> <Z> Z pick(Z z1, Z z2, Z z3) { .... }
>
> pick(i1, null, i2)
>
> --> lub(I1, I2) --> I
>
> Seems like the `null` is tripping javac up
>
> Maurizio
>
> On 30/09/2020 10:50, Anna Kozlova wrote:
>
> Hi all,
>
> The following code doesn't compile (infers i_ to Object)
>
> interface I {
>     void m();
> }
> interface I1 extends I {}
> interface I2 extends I {}
>
> static void n(I1 i1, I2 i2, int s) {
>     var i_ = switch (s) {
>         case 1 -> i1;
>         case 2 -> null;
>         default -> i2;
>     };
>     if (i_ != null) {
>         i_.m(); //cannot find symbol m()
>     }
> }
>
>
> If you permute "case 1" and "case 2" switch rules, the code starts to compile (infers i_ to I).
>
> Looks like a bug?
>
> Thanks,
> Anna