8254274: Development to add 'lint' warning for @valuebased classes

Reviewed-by: dlsmith, jlaskey

Author: Srikanth Adayapalam

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java

@@ -125,6 +125,7 @@ protected Lint(Context context) {
             if (!options.isSet(Option.PREVIEW)) {
                 values.add(LintCategory.PREVIEW);
             }
+            values.add(LintCategory.SYNCHRONIZATION);
         }
 
         // Look for specific overrides
@@ -282,6 +283,11 @@ public enum LintCategory {
          */
         STATIC("static"),
 
+        /**
+         * Warn about synchronization attempts on instances of @ValueBased classes.
+         */
+        SYNCHRONIZATION("synchronization"),
+
         /**
          * Warn about issues relating to use of text blocks
          */

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java

@@ -220,6 +220,7 @@ public static Symtab instance(Context context) {
     public final Type previewFeatureInternalType;
     public final Type typeDescriptorType;
     public final Type recordType;
+    public final Type valueBasedType;
 
     /** The symbol representing the length field of an array.
      */
@@ -584,6 +585,7 @@ public <R, P> R accept(ElementVisitor<R, P> v, P p) {
         previewFeatureInternalType = enterSyntheticAnnotation("jdk.internal.PreviewFeature+Annotation");
         typeDescriptorType = enterClass("java.lang.invoke.TypeDescriptor");
         recordType = enterClass("java.lang.Record");
+        valueBasedType = enterClass("jdk.internal.ValueBased");
 
         synthesizeEmptyInterfaceIfMissing(autoCloseableType);
         synthesizeEmptyInterfaceIfMissing(cloneableType);

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

@@ -1737,9 +1737,24 @@ private Symbol enumConstant(JCTree tree, Type enumType) {
 
     public void visitSynchronized(JCSynchronized tree) {
         chk.checkRefType(tree.pos(), attribExpr(tree.lock, env));
+        if (env.info.lint.isEnabled(LintCategory.SYNCHRONIZATION) && isValueBased(tree.lock.type)) {
+            log.warning(LintCategory.SYNCHRONIZATION, tree.pos(), Warnings.AttemptToSynchronizeOnInstanceOfValueBasedClass);
+        }
         attribStat(tree.body, env);
         result = null;
     }
+        // where
+        private boolean isValueBased(Type t) {
+            if (t != null && t.tsym != null) {
+                for (Attribute.Compound a: t.tsym.getDeclarationAttributes()) {
+                    if (a.type.tsym == syms.valueBasedType.tsym) {
+                        return true;
+                    }
+                }
+            }
+            return false;
+        }
+
 
     public void visitTry(JCTry tree) {
         // Create a new local environment with a local

src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -3742,3 +3742,7 @@ compiler.err.preview.not.latest=\
 
 compiler.err.preview.without.source.or.release=\
     --enable-preview must be used with either -source or --release
+
+compiler.warn.attempt.to.synchronize.on.instance.of.value.based.class=\
+    attempt to synchronize on an instance of a value-based class
+

src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties

@@ -256,10 +256,13 @@ javac.opt.Xlint.desc.unchecked=\
     Warn about unchecked operations.
 
 javac.opt.Xlint.desc.varargs=\
-    Warn about potentially unsafe vararg methods
+    Warn about potentially unsafe vararg methods.
 
 javac.opt.Xlint.desc.preview=\
-    Warn about use of preview language features
+    Warn about use of preview language features.
+
+javac.opt.Xlint.desc.synchronization=\
+    Warn about synchronization attempts on instances of value-based classes.
 
 javac.opt.Xdoclint=\
     Enable recommended checks for problems in javadoc comments

test/langtools/tools/javac/diags/examples/AttemptToSynchronizeOnInstanceOfVbc.java

@@ -0,0 +1,32 @@
+/*
+ * 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.attempt.to.synchronize.on.instance.of.value.based.class
+// options: -Xlint:synchronization
+
+class AttemptToSynchronizeOnInstanceOfVbc {
+    void foo(Integer i) {
+        synchronized(i) {
+        }
+    }
+}

test/langtools/tools/javac/lint/ExternalAbuseOfVbc.java

@@ -0,0 +1,23 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8254274
+ * @summary lint should warn when an instance of a value based class is synchronized upon
+ * @compile/fail/ref=ExternalAbuseOfVbc.out -XDrawDiagnostics -Werror -Xlint ExternalAbuseOfVbc.java
+ * @compile/fail/ref=ExternalAbuseOfVbc.out -XDrawDiagnostics -Werror -Xlint:all ExternalAbuseOfVbc.java
+ * @compile/fail/ref=ExternalAbuseOfVbc.out -XDrawDiagnostics -Werror -Xlint:synchronization ExternalAbuseOfVbc.java
+ * @compile/ref=LintModeOffAbuseOfVbc.out -XDrawDiagnostics -Werror -Xlint:-synchronization ExternalAbuseOfVbc.java
+ */
+
+public final class ExternalAbuseOfVbc {
+
+    final Integer val = Integer.valueOf(42);
+    final String ref = "String";
+
+    void abuseVbc() {
+        synchronized(ref) {      // OK
+            synchronized (val) { // WARN
+            }
+        }
+    }
+}
+

test/langtools/tools/javac/lint/ExternalAbuseOfVbc.out

@@ -0,0 +1,4 @@
+ExternalAbuseOfVbc.java:18:13: compiler.warn.attempt.to.synchronize.on.instance.of.value.based.class
+- compiler.err.warnings.and.werror
+1 error
+1 warning

test/langtools/tools/javac/lint/JdkInternalAbuseOfVbc.java

@@ -0,0 +1,22 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8254274
+ * @summary lint should warn when an instance of a value based class is synchronized upon
+ * @compile/fail/ref=JdkInternalAbuseOfVbc.out --patch-module java.base=${test.src} -XDrawDiagnostics -Werror -Xlint SomeVbc.java JdkInternalAbuseOfVbc.java
+ */
+
+package java.lang;
+
+public final class JdkInternalAbuseOfVbc {
+
+    public JdkInternalAbuseOfVbc() {}
+
+    void abuseVbc(SomeVbc vbc) {
+
+        synchronized(this) {           // OK
+            synchronized (vbc) {       // WARN
+            }
+        }
+    }
+}
+

test/langtools/tools/javac/lint/JdkInternalAbuseOfVbc.out

@@ -0,0 +1,6 @@
+SomeVbc.java:38:13: compiler.warn.attempt.to.synchronize.on.instance.of.value.based.class
+- compiler.err.warnings.and.werror
+SomeVbc.java:49:13: compiler.warn.attempt.to.synchronize.on.instance.of.value.based.class
+JdkInternalAbuseOfVbc.java:17:13: compiler.warn.attempt.to.synchronize.on.instance.of.value.based.class
+1 error
+3 warnings