8013357: javac accepts erroneous binary comparison operations

Summary: javac does not report type errors on illegal Object == primitive comparisons
Reviewed-by: abuckley, mcimadamore

src/share/classes/com/sun/tools/javac/code/Types.java

@@ -1342,6 +1342,26 @@ public class Types {
     }
     // </editor-fold>
 
+    /**
+     * Can t and s be compared for equality?  Any primitive ==
+     * primitive or primitive == object comparisons here are an error.
+     * Unboxing and correct primitive == primitive comparisons are
+     * already dealt with in Attr.visitBinary.
+     *
+     */
+    public boolean isEqualityComparable(Type s, Type t, Warner warn) {
+        if (t.isNumeric() && s.isNumeric())
+            return true;
+
+        boolean tPrimitive = t.isPrimitive();
+        boolean sPrimitive = s.isPrimitive();
+        if (!tPrimitive && !sPrimitive) {
+            return isCastable(s, t, warn) || isCastable(t, s, warn);
+        } else {
+            return false;
+        }
+    }
+
     // <editor-fold defaultstate="collapsed" desc="isCastable">
     public boolean isCastable(Type t, Type s) {
         return isCastable(t, s, noWarnings);

src/share/classes/com/sun/tools/javac/comp/Attr.java

@@ -3010,6 +3010,8 @@ public class Attr extends JCTree.Visitor {
                 !left.isErroneous() &&
                 !right.isErroneous()) {
             owntype = operator.type.getReturnType();
+            // This will figure out when unboxing can happen and
+            // choose the right comparison operator.
             int opc = chk.checkOperator(tree.lhs.pos(),
                                         (OperatorSymbol)operator,
                                         tree.getTag(),
@@ -3037,9 +3039,11 @@ public class Attr extends JCTree.Visitor {
             }
 
             // Check that argument types of a reference ==, != are
-            // castable to each other, (JLS???).
+            // castable to each other, (JLS 15.21).  Note: unboxing
+            // comparisons will not have an acmp* opc at this point.
             if ((opc == ByteCodes.if_acmpeq || opc == ByteCodes.if_acmpne)) {
-                if (!types.isCastable(left, right, new Warner(tree.pos()))) {
+                if (!types.isEqualityComparable(left, right,
+                                                new Warner(tree.pos()))) {
                     log.error(tree.pos(), "incomparable.types", left, right);
                 }
             }

test/tools/javac/lambda/LambdaConv01.java

@@ -67,7 +67,7 @@ public class LambdaConv01 {
         assertTrue(3 == f1.foo());
         //Covariant returns:
         TU<Number, Integer> f2 = (Integer x) -> x;
-        assertTrue(3 == f2.foo(3));
+        assertTrue(3 == f2.foo(3).intValue());
         //Method resolution with boxing:
         int res = LambdaConv01.<Integer,Integer>exec((Integer x) -> x, 3);
         assertTrue(3 == res);
@@ -86,7 +86,7 @@ public class LambdaConv01 {
         assertTrue(3 == f1.foo());
         //Covariant returns:
         TU<Number, Integer> f2 = (Integer x) -> x;
-        assertTrue(3 == f2.foo(3));
+        assertTrue(3 == f2.foo(3).intValue());
         //Method resolution with boxing:
         int res = LambdaConv01.<Integer,Integer>exec((Integer x) -> x, 3);
         assertTrue(3 == res);
@@ -105,7 +105,7 @@ public class LambdaConv01 {
         assertTrue(3 == f1.foo());
         //Covariant returns:
         TU<Number, Integer> f2 = (Integer x) -> x;
-        assertTrue(3 == f2.foo(3));
+        assertTrue(3 == f2.foo(3).intValue());
         //Method resolution with boxing:
         int res = LambdaConv01.<Integer,Integer>exec((Integer x) -> x, 3);
         assertTrue(3 == res);
@@ -124,7 +124,7 @@ public class LambdaConv01 {
         assertTrue(3 == f1.foo());
         //Covariant returns:
         TU<Number, Integer> f2 = (Integer x) -> x;
-        assertTrue(3 == f2.foo(3));
+        assertTrue(3 == f2.foo(3).intValue());
         //Method resolution with boxing:
         int res = LambdaConv01.<Integer,Integer>exec((Integer x) -> x, 3);
         assertTrue(3 == res);

test/tools/javac/lambda/LambdaExpr15.java

@@ -48,7 +48,7 @@ public class LambdaExpr15 {
             new Object() {
                 String get() { return ""; }
             };
-            assertTrue(t == 1);
+            assertTrue((Integer)t == 1);
         };
         ba1.apply(1);
 
@@ -58,7 +58,7 @@ public class LambdaExpr15 {
                 String get() { return ""; }
             };
             new A();
-            assertTrue(t == 2);
+            assertTrue((Integer)t == 2);
         };
         ba2.apply(2);
         assertTrue(assertionCount == 2);

test/tools/javac/lambda/typeInference/InferenceTest2b.java

@@ -64,7 +64,7 @@ public class InferenceTest2b {
 
     void m2(SAM6<? super Integer> s) {
         System.out.println("m2()");
-        assertTrue(s.m6(1, 2) == 1);
+        assertTrue(s.m6(1, 2).equals(Integer.valueOf(1)));
     }
 
     void m3(SAM6<? super Calendar> s) {

test/tools/javac/types/TestComparisons.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8013357
+ * @summary javac should correctly enforce binary comparison rules.
+ */
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.code.Type.*;
+import com.sun.tools.javac.code.Symbol.*;
+import java.io.*;
+import java.lang.reflect.Array;
+import java.util.EnumSet;
+
+public class TestComparisons {
+
+    private int errors = 0;
+    private int testnum = 0;
+
+    static final File testdir = new File("8013357");
+
+    private enum CompareType {
+        BYTE_PRIM("byte"),
+        SHORT_PRIM("short"),
+        CHAR_PRIM("char"),
+        INTEGER_PRIM("int"),
+        LONG_PRIM("long"),
+        FLOAT_PRIM("float"),
+        DOUBLE_PRIM("double"),
+        BOOLEAN_PRIM("boolean"),
+
+        BYTE("Byte"),
+        SHORT("Short"),
+        CHAR("Character"),
+        INTEGER("Integer"),
+        LONG("Long"),
+        FLOAT("Float"),
+        DOUBLE("Double"),
+        BOOLEAN("Boolean"),
+
+        BYTE_SUPER("List<? super Byte>", true),
+        SHORT_SUPER("List<? super Short>", true),
+        CHAR_SUPER("List<? super Character>", true),
+        INTEGER_SUPER("List<? super Integer>", true),
+        LONG_SUPER("List<? super Long>", true),
+        FLOAT_SUPER("List<? super Float>", true),
+        DOUBLE_SUPER("List<? super Double>", true),
+        BOOLEAN_SUPER("List<? super Boolean>", true),
+
+        OBJECT("Object"),
+        NUMBER("Number"),
+        STRING("String");
+
+        public final boolean isList;
+        public final String name;
+
+        private CompareType(final String name, final boolean isList) {
+            this.isList = isList;
+            this.name = name;
+        }
+
+        private CompareType(final String name) {
+            this(name, false);
+        }
+    }
+
+    // The integers here refer to which subsection of JLS 15.21 is in
+    // effect.  0 means no comparison is allowed.
+    private static final int truthtab[][] = {
+        // byte, comparable to itself, any numeric type, or any boxed
+        // numeric type.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          1, 1, 1, 1, 1, 1, 1, 0, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // short, comparable to itself, any numeric type, or any boxed
+        // numeric type.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          1, 1, 1, 1, 1, 1, 1, 0, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // char, comparable to itself, any numeric type, or any boxed
+        // numeric type.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          1, 1, 1, 1, 1, 1, 1, 0, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // int, comparable to itself, any numeric type, or any boxed
+        // numeric type.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          1, 1, 1, 1, 1, 1, 1, 0, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // long, comparable to itself, any numeric type, or any boxed
+        // numeric type.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          1, 1, 1, 1, 1, 1, 1, 0, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // float, comparable to itself, any numeric type, or any boxed
+        // numeric type.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          1, 1, 1, 1, 1, 1, 1, 0, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // double, comparable to itself, any numeric type, or any boxed
+        // numeric type.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          1, 1, 1, 1, 1, 1, 1, 0, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // boolean, comparable only to itself and Boolean.
+        { 0, 0, 0, 0, 0, 0, 0, 2, // Primitives
+          0, 0, 0, 0, 0, 0, 0, 2, // Boxed primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Captures
+          0, 0, 0                 // Reference types
+        },
+        // Byte, comparable to itself, Number, Object, any numeric primitive,
+        // and any captures.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          3, 0, 0, 0, 0, 0, 0, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 0                 // Reference types
+        },
+        // Short, comparable to itself, Number, Object, any numeric primitive,
+        // and any captures.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          0, 3, 0, 0, 0, 0, 0, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 0                 // Reference types
+        },
+        // Character, comparable to itself, Object, any numeric primitive,
+        // and any captures.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          0, 0, 3, 0, 0, 0, 0, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 0, 0                 // Reference types
+        },
+        // Int, comparable to itself, Number, Object, any numeric primitive,
+        // and any captures.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          0, 0, 0, 3, 0, 0, 0, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 0                 // Reference types
+        },
+        // Long, comparable to itself, Number, Object, any numeric primitive,
+        // and any captures.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          0, 0, 0, 0, 3, 0, 0, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 0                 // Reference types
+        },
+        // Float, comparable to itself, Number, Object, any numeric primitive,
+        // and any captures.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          0, 0, 0, 0, 0, 3, 0, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 0                 // Reference types
+        },
+        // Double, comparable to itself, Number, Object, any numeric primitive,
+        // and any captures.
+        { 1, 1, 1, 1, 1, 1, 1, 0, // Primitives
+          0, 0, 0, 0, 0, 0, 3, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 0                 // Reference types
+        },
+        // Boolean, to itself, any capture, Object, and boolean.
+        { 0, 0, 0, 0, 0, 0, 0, 2, // Primitives
+          0, 0, 0, 0, 0, 0, 0, 2, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 0, 0                 // Reference types
+        },
+        // Byte supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Short supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Character supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Integer supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Long supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Float supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Double supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Boolean supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Object, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 3                 // Reference types
+        },
+        // Number, comparable to Object, any of its subclasses.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          3, 3, 0, 3, 3, 3, 3, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 3, 0                 // Reference types
+        },
+        // String supertype wildcard, comparable to any reference type.
+        // and any captures.
+        { 0, 0, 0, 0, 0, 0, 0, 0, // Primitives
+          0, 0, 0, 0, 0, 0, 0, 0, // Boxed primitives
+          3, 3, 3, 3, 3, 3, 3, 3, // Captures
+          3, 0, 3                 // Reference types
+        }
+    };
+
+    private void assert_compile_fail(final File file, final String body) {
+        final String filename = file.getPath();
+        final String[] args = { filename };
+        final StringWriter sw = new StringWriter();
+        final PrintWriter pw = new PrintWriter(sw);
+        final int rc = com.sun.tools.javac.Main.compile(args, pw);
+        pw.close();
+        if (rc == 0) {
+            System.err.println("Compilation of " + file.getName() +
+                               " didn't fail as expected.\nFile:\n" +
+                               body + "\nOutput:\n" + sw.toString());
+            errors++;
+        }
+    }
+
+    private void assert_compile_succeed(final File file, final String body) {
+        final String filename = file.getPath();
+        final String[] args = { filename };
+        final StringWriter sw = new StringWriter();
+        final PrintWriter pw = new PrintWriter(sw);
+        final int rc = com.sun.tools.javac.Main.compile(args, pw);
+        pw.close();
+        if (rc != 0) {
+            System.err.println("Compilation of " + file.getName() +
+                               " didn't succeed as expected.\nFile:\n" +
+                               body + "\nOutput:\n" +
+                               sw.toString());
+            errors++;
+        }
+    }
+
+    private String makeBody(final int num,
+                            final CompareType left,
+                            final CompareType right) {
+        return "import java.util.List;\n" +
+            "public class Test" + num + " {\n" +
+            "    public boolean test(" + left.name +
+            " left, " + right.name + " right) {\n" +
+            "        return left" + (left.isList ? ".get(0)" : "") +
+            " == right" + (right.isList ? ".get(0)" : "") + ";\n" +
+            "    }\n" +
+            "}\n";
+    }
+
+    private File writeFile(final String filename,
+                           final String body)
+        throws IOException {
+        final File f = new File(testdir, filename);
+        f.getParentFile().mkdirs();
+        final FileWriter out = new FileWriter(f);
+        out.write(body);
+        out.close();
+        return f;
+    }
+
+    private void test(final CompareType left, final CompareType right)
+        throws IOException {
+        final int num = testnum++;
+        final String filename = "Test" + num + ".java";
+        final String body = makeBody(num, left, right);
+        final File file = writeFile(filename, body);
+        if (truthtab[left.ordinal()][right.ordinal()] != 0)
+            assert_compile_succeed(file, body);
+        else
+            assert_compile_fail(file, body);
+    }
+
+    void run() throws Exception {
+        testdir.mkdir();
+
+        for(CompareType left : CompareType.values())
+            for(CompareType right : CompareType.values())
+                test(left, right);
+
+        if (errors != 0)
+            throw new Exception("ObjectZeroCompare test failed with " +
+                                errors + " errors.");
+    }
+
+    public static void main(String... args) throws Exception {
+        new TestComparisons().run();
+    }
+}