8237074: Warning when obj.getClass() == SomeItf.class (from State-of-Valhalla)
Jesper Steen Møller
jesper at selskabet.org
Mon Feb 3 08:09:07 UTC 2020
Hi Srikanth
On 3 Feb 2020, at 07.49, Srikanth <srikanth.adayapalam at oracle.com> wrote:
>
>
> (1) There is a typo "supported om future versions"
>
Fixed, and made comment and lint option text identical
> (2) I am surprised by "return sym.name == names.getClass && !sym.isStatic();" in isInvocationOfGetClass()
>
Fixed by querying the method’s symbol instead, and added your example as a negative test. This approach
>
> (3) I am checking with the right folks on the copyright notice. My understanding is that it should not mention the individual's name.
> But I will confirm as soon as I have heard from the "horse's mouth"
>
No problem, fixed by assigning the copyright to Oracle, as Oracle is free to do under the OCA anyway.
Patch attached.
-Jesper
diff -r e2f1c4d5f39e src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java Tue Jan 28 17:12:01 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java Mon Feb 03 09:03:11 2020 +0100
@@ -216,6 +216,11 @@
MODULE("module"),
/**
+ * Warn about issues related to migration of JDK classes.
+ */
+ MIGRATION("migration"),
+
+ /**
* Warn about issues regarding module opens.
*/
OPENS("opens"),
diff -r e2f1c4d5f39e src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Tue Jan 28 17:12:01 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Mon Feb 03 09:03:11 2020 +0100
@@ -3817,6 +3817,7 @@
if (!types.isCastable(left, right, new Warner(tree.pos()))) {
log.error(tree.pos(), Errors.IncomparableTypes(left, right));
}
+ chk.checkForSuspectClassLiteralComparison(tree, left, right);
}
chk.checkDivZero(tree.rhs.pos(), operator, right);
diff -r e2f1c4d5f39e src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java Tue Jan 28 17:12:01 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java Mon Feb 03 09:03:11 2020 +0100
@@ -1045,6 +1045,39 @@
return varType;
}
+ public void checkForSuspectClassLiteralComparison(
+ final JCBinary tree,
+ final Type leftType,
+ final Type rightType) {
+
+ if (lint.isEnabled(LintCategory.MIGRATION)) {
+ if (isInvocationOfGetClass(tree.lhs) && isClassOfSomeInterface(rightType) ||
+ isInvocationOfGetClass(tree.rhs) && isClassOfSomeInterface(leftType)) {
+ log.warning(LintCategory.MIGRATION, tree.pos(), Warnings.GetClassComparedWithInterface);
+ }
+ }
+ }
+ //where
+ private boolean isClassOfSomeInterface(Type someClass) {
+ if (someClass.tsym.flatName() == names.java_lang_Class) {
+ List<Type> arguments = someClass.getTypeArguments();
+ if (arguments.length() == 1) {
+ return arguments.head.isInterface();
+ }
+ }
+ return false;
+ }
+ //where
+ private boolean isInvocationOfGetClass(JCExpression tree) {
+ tree = TreeInfo.skipParens(tree);
+ if (tree.hasTag(APPLY)) {
+ JCMethodInvocation apply = (JCMethodInvocation)tree;
+ MethodSymbol msym = (MethodSymbol)TreeInfo.symbol(apply.meth);
+ return msym.name == names.getClass && msym.implementedIn(syms.objectType.tsym, types) != null;
+ }
+ return false;
+ }
+
Type checkMethod(final Type mtype,
final Symbol sym,
final Env<AttrContext> env,
diff -r e2f1c4d5f39e src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Tue Jan 28 17:12:01 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Mon Feb 03 09:03:11 2020 +0100
@@ -1778,6 +1778,9 @@
compiler.warn.incubating.modules=\
using incubating module(s): {0}
+compiler.warn.get.class.compared.with.interface=\
+ return value of getClass() can never equal the class literal of an interface
+
# 0: symbol, 1: symbol
compiler.warn.has.been.deprecated=\
{0} in {1} has been deprecated
diff -r e2f1c4d5f39e src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties Tue Jan 28 17:12:01 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties Mon Feb 03 09:03:11 2020 +0100
@@ -206,6 +206,9 @@
javac.opt.Xlint.desc.module=\
Warn about module system related issues.
+javac.opt.Xlint.desc.migration=\
+ Warn about issues related to migration of JDK classes.
+
javac.opt.Xlint.desc.opens=\
Warn about issues regarding module opens.
diff -r e2f1c4d5f39e test/langtools/tools/javac/diags/CheckExamples.java
--- a/test/langtools/tools/javac/diags/CheckExamples.java Tue Jan 28 17:12:01 2020 +0530
+++ b/test/langtools/tools/javac/diags/CheckExamples.java Mon Feb 03 09:03:11 2020 +0100
@@ -45,12 +45,22 @@
/**
* Check invariants for a set of examples.
+ *
+ * READ THIS IF THIS TEST FAILS AFTER ADDING A NEW KEY TO 'compiler.properties':
+ * The 'examples' subdirectory contains a number of examples which provoke
+ * the reporting of most of the compiler message keys.
+ *
* -- each example should exactly declare the keys that will be generated when
* it is run.
+ * -- this is done by the "// key:"-comment in each fine.
* -- together, the examples should cover the set of resource keys in the
* compiler.properties bundle. A list of exceptions may be given in the
* not-yet.txt file. Entries on the not-yet.txt list should not be
* covered by examples.
+ * -- some keys are only reported by the compiler when specific options are
+ * supplied. For the purposes of this test, this can be specified by a
+ * comment e.g. like this: "// options: -Xlint:empty"
+ *
* When new keys are added to the resource bundle, it is strongly recommended
* that corresponding new examples be added here, if at all practical, instead
* of simply and lazily being added to the not-yet.txt list.
diff -r e2f1c4d5f39e test/langtools/tools/javac/diags/examples/CheckGetClassComparedWithInterfaceLiteral.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/diags/examples/CheckGetClassComparedWithInterfaceLiteral.java Mon Feb 03 09:03:11 2020 +0100
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2020, 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.
+ */
+
+// key: compiler.warn.get.class.compared.with.interface
+// options: -Xlint:migration
+
+class CheckGetClassComparedWithInterfaceLiteral {
+ boolean checkObject(Object o) {
+ return o.getClass() == Iterable.class;
+ }
+}
diff -r e2f1c4d5f39e test/langtools/tools/javac/valhalla/lworld-values/CheckInterfaceComparison.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckInterfaceComparison.java Mon Feb 03 09:03:11 2020 +0100
@@ -0,0 +1,72 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8237074
+ * @summary Result of .getClass() should never be compared to an interface class literal
+ *
+ * @compile/ref=CheckInterfaceComparison.out -XDrawDiagnostics -Xlint:migration CheckInterfaceComparison.java
+ */
+public class CheckInterfaceComparison {
+ public boolean bogusCompareLeft(Object o) { // Should be warned against
+ return (o.getClass()) == Runnable.class;
+ }
+
+ public boolean bogusCompareNELeft(Object o) { // Should be warned against
+ return (o.getClass()) != Runnable.class;
+ }
+
+ public boolean bogusCompareRight(Object o) { // Should be warned against
+ return Iterable.class == o.getClass();
+ }
+
+ public boolean bogusCompareNERight(Object o) { // Should be warned against
+ return Iterable.class != o.getClass();
+ }
+
+ public boolean goodCompareLeft(Object o) { // Is fine, no warning required
+ return o.getClass() == Integer.class;
+ }
+
+ public boolean goodCompareNELeft(Object o) { // Is fine, no warning required
+ return o.getClass() != Integer.class;
+ }
+
+ public boolean goodCompareRight(Object o) { // Is fine, no warning required
+ return Long.class == o.getClass();
+ }
+
+ public boolean goodCompareNERight(Object o) { // Is fine, no warning required
+ return Long.class != o.getClass();
+ }
+
+ public boolean rawCompareLeft(Object o, Class<?> clazz) { // Is fine, no warning required
+ return o.getClass() == clazz;
+ }
+
+ public boolean rawCompareNELeft(Object o, Class<?> clazz) { // Is fine, no warning required
+ return o.getClass() != clazz;
+ }
+
+ public boolean rawCompareRight(Object o, Class<?> clazz) { // Is fine, no warning required
+ return clazz == o.getClass();
+ }
+
+ public boolean rawCompareNERight(Object o, Class<?> clazz) { // Is fine, no warning required
+ return clazz != o.getClass();
+ }
+
+ static Class<?> getClass(int x) {
+ return null;
+ }
+
+ public static boolean compare(Object o, Class<?> clazz) {
+ return getClass(0) == Runnable.class; // No warning required for static getClass
+ }
+
+ public Class<?> getClass(String x) {
+ return null;
+ }
+
+ public boolean compare(Object o, String arg, Class<?> clazz) {
+ return getClass(arg) == Runnable.class; // No warning required for non-object.getClass()
+ }
+}
diff -r e2f1c4d5f39e test/langtools/tools/javac/valhalla/lworld-values/CheckInterfaceComparison.out
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckInterfaceComparison.out Mon Feb 03 09:03:11 2020 +0100
@@ -0,0 +1,5 @@
+CheckInterfaceComparison.java:10:31: compiler.warn.get.class.compared.with.interface
+CheckInterfaceComparison.java:14:31: compiler.warn.get.class.compared.with.interface
+CheckInterfaceComparison.java:18:31: compiler.warn.get.class.compared.with.interface
+CheckInterfaceComparison.java:22:31: compiler.warn.get.class.compared.with.interface
+4 warnings
More information about the valhalla-dev
mailing list