Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8356539

Expand value-based class warnings to java.lang.ref API

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Unresolved
    • Icon: P4 P4
    • 25
    • tools
    • None
    • low
    • Code compiled with -Xlint:all and -Werror might fail to compile due to the new warnings being issued.
    • Java API, add/remove/modify command line option
    • JDK

      Summary

      JEP 390 introduced the synchronization lint category, now we want to issue additional warnings for inappropriate uses of java.lang.ref and java.util APIs. The existing synchronization category is too narrow to describe the additional warnings we want to introduce, for this reason we are also introducing a new lint warning category named identity. This new category will coexist as an alias of synchronization so enabling or disabling one will imply enabling or disabling the other.

      Problem

      JEP 390 introduced a synchronization lint category for inappropriate uses of value-based class types in synchronized expressions. Now we'd like to produce additional warnings for inappropriate uses of java.lang.ref and java.util APIs, in particular where an identity class is expected instead of a value based class.

      Given that the new warnings won't be related to any synchronization issue, the current name for this category is not general enough. We should also consider that there could be user code making use of the synchronization lint category.

      Solution

      In order to solve the issues mentioned in the Problem section above we propose to:

      • Introduce a new lint warning category named identity which should be an alias of the synchronization category
      • Produce additional warnings for inappropriate uses of java.lang.ref and java.util APIs
      • Keep the synchronization lint category for some time to give user code bases additional time to update to the identity category which should be the only one supported in the future

      In order to indicate where an identity class is expected the new internal annotation jdk.internal.RequiresIdentity will be used.

      Specification

      Changes to core-libs APIs:

      diff --git a/src/java.base/share/classes/java/lang/ref/Cleaner.java b/src/java.base/share/classes/java/lang/ref/Cleaner.java
      index c5a163831c8..20a964be21d 100644
      --- a/src/java.base/share/classes/java/lang/ref/Cleaner.java
      +++ b/src/java.base/share/classes/java/lang/ref/Cleaner.java
      @@ -219,7 +219,7 @@ public static Cleaner create(ThreadFactory threadFactory) {
            * @param action a {@code Runnable} to invoke when the object becomes phantom reachable
            * @return a {@code Cleanable} instance
            */
      -    public Cleanable register(Object obj, Runnable action) {
      +    public Cleanable register(@jdk.internal.RequiresIdentity Object obj, Runnable action) {
               Objects.requireNonNull(obj, "obj");
               Objects.requireNonNull(action, "action");
               return new CleanerImpl.PhantomCleanableRef(obj, this, action);
      diff --git a/src/java.base/share/classes/java/lang/ref/PhantomReference.java b/src/java.base/share/classes/java/lang/ref/PhantomReference.java
      index 3f01144afe6..a6e144dc7b6 100644
      --- a/src/java.base/share/classes/java/lang/ref/PhantomReference.java
      +++ b/src/java.base/share/classes/java/lang/ref/PhantomReference.java
      @@ -51,7 +51,7 @@
        * @since    1.2
        */
      
      -public non-sealed class PhantomReference<T> extends Reference<T> {
      +public non-sealed class PhantomReference<@jdk.internal.RequiresIdentity T> extends Reference<T> {
      
           /**
            * Returns this reference object's referent.  Because the referent of a
      @@ -101,7 +101,7 @@ void clearImpl() {
            * @param q the queue with which the reference is to be registered,
            *          or {@code null} if registration is not required
            */
      -    public PhantomReference(T referent, ReferenceQueue<? super T> q) {
      +    public PhantomReference(@jdk.internal.RequiresIdentity T referent, ReferenceQueue<? super T> q) {
               super(referent, q);
           }
      
      diff --git a/src/java.base/share/classes/java/lang/ref/Reference.java b/src/java.base/share/classes/java/lang/ref/Reference.java
      index e109b974adc..8f8c9ed8227 100644
      --- a/src/java.base/share/classes/java/lang/ref/Reference.java
      +++ b/src/java.base/share/classes/java/lang/ref/Reference.java
      @@ -44,7 +44,7 @@
        * @sealedGraph
        */
      
      -public abstract sealed class Reference<T>
      +public abstract sealed class Reference<@jdk.internal.RequiresIdentity T>
           permits PhantomReference, SoftReference, WeakReference, FinalReference {
      
           /* The state of a Reference object is characterized by two attributes.  It
      diff --git a/src/java.base/share/classes/java/lang/ref/ReferenceQueue.java b/src/java.base/share/classes/java/lang/ref/ReferenceQueue.java
      index 4dcaf796e25..9c620ed8836 100644
      --- a/src/java.base/share/classes/java/lang/ref/ReferenceQueue.java
      +++ b/src/java.base/share/classes/java/lang/ref/ReferenceQueue.java
      @@ -47,7 +47,7 @@
        * @since    1.2
        */
      
      -public class ReferenceQueue<T> {
      +public class ReferenceQueue<@jdk.internal.RequiresIdentity T> {
           private static class Null extends ReferenceQueue<Object> {
               @Override
               boolean enqueue(Reference<?> r) {
      diff --git a/src/java.base/share/classes/java/lang/ref/SoftReference.java b/src/java.base/share/classes/java/lang/ref/SoftReference.java
      index 925f894fdab..0bc372eb105 100644
      --- a/src/java.base/share/classes/java/lang/ref/SoftReference.java
      +++ b/src/java.base/share/classes/java/lang/ref/SoftReference.java
      @@ -62,7 +62,7 @@
        * @since    1.2
        */
      
      -public non-sealed class SoftReference<T> extends Reference<T> {
      +public non-sealed class SoftReference<@jdk.internal.RequiresIdentity T> extends Reference<T> {
      
           /**
            * Timestamp clock, updated by the garbage collector
      @@ -82,7 +82,7 @@ public non-sealed class SoftReference<T> extends Reference<T> {
            *
            * @param referent object the new soft reference will refer to
            */
      -    public SoftReference(T referent) {
      +    public SoftReference(@jdk.internal.RequiresIdentity T referent) {
               super(referent);
               this.timestamp = clock;
           }
      @@ -96,7 +96,7 @@ public SoftReference(T referent) {
            *          or {@code null} if registration is not required
            *
            */
      -    public SoftReference(T referent, ReferenceQueue<? super T> q) {
      +    public SoftReference(@jdk.internal.RequiresIdentity T referent, ReferenceQueue<? super T> q) {
               super(referent, q);
               this.timestamp = clock;
           }
      diff --git a/src/java.base/share/classes/java/lang/ref/WeakReference.java b/src/java.base/share/classes/java/lang/ref/WeakReference.java
      index 1a733bb32b9..4b4f834485e 100644
      --- a/src/java.base/share/classes/java/lang/ref/WeakReference.java
      +++ b/src/java.base/share/classes/java/lang/ref/WeakReference.java
      @@ -46,7 +46,7 @@
        * @since    1.2
        */
      
      -public non-sealed class WeakReference<T> extends Reference<T> {
      +public non-sealed class WeakReference<@jdk.internal.RequiresIdentity T> extends Reference<T> {
      
           /**
            * Creates a new weak reference that refers to the given object.  The new
      @@ -54,7 +54,7 @@ public non-sealed class WeakReference<T> extends Reference<T> {
            *
            * @param referent object the new weak reference will refer to
            */
      -    public WeakReference(T referent) {
      +    public WeakReference(@jdk.internal.RequiresIdentity T referent) {
               super(referent);
           }
      
      @@ -66,7 +66,7 @@ public WeakReference(T referent) {
            * @param q the queue with which the reference is to be registered,
            *          or {@code null} if registration is not required
            */
      -    public WeakReference(T referent, ReferenceQueue<? super T> q) {
      +    public WeakReference(@jdk.internal.RequiresIdentity T referent, ReferenceQueue<? super T> q) {
               super(referent, q);
           }
      
      diff --git a/src/java.base/share/classes/java/util/WeakHashMap.java b/src/java.base/share/classes/java/util/WeakHashMap.java
      index 276e8731d84..7f2960eb2ad 100644
      --- a/src/java.base/share/classes/java/util/WeakHashMap.java
      +++ b/src/java.base/share/classes/java/util/WeakHashMap.java
      @@ -132,7 +132,7 @@
        * @see         java.util.HashMap
        * @see         java.lang.ref.WeakReference
        */
      -public class WeakHashMap<K,V>
      +public class WeakHashMap<@jdk.internal.RequiresIdentity K,V>
           extends AbstractMap<K,V>
           implements Map<K,V> {
      
      @@ -457,7 +457,7 @@ Entry<K,V> getEntry(Object key) {
            *         (A {@code null} return can also indicate that the map
            *         previously associated {@code null} with {@code key}.)
            */
      -    public V put(K key, V value) {
      +    public V put(@jdk.internal.RequiresIdentity K key, V value) {
               Object k = maskNull(key);
               int h = hash(k);
               Entry<K,V>[] tab = getTable();

      Changes to src/jdk.compiler/share/classes/module-info.java:

      diff --git a/src/jdk.compiler/share/classes/module-info.java b/src/jdk.compiler/share/classes/module-info.java
      index 60a9ae0e476..552967159b2 100644
      --- a/src/jdk.compiler/share/classes/module-info.java
      +++ b/src/jdk.compiler/share/classes/module-info.java
      @@ -164,6 +164,7 @@
        * <tr><th scope="row">{@code fallthrough}          <td>falling through from one case of a {@code switch} statement to
        *                                                      the next
        * <tr><th scope="row">{@code finally}              <td>{@code finally} clauses that do not terminate normally
      + * <tr><th scope="row">{@code identity}             <td>use of a value-based class where an identity class is expected
        * <tr><th scope="row">{@code incubating}           <td>use of incubating modules
        * <tr><th scope="row">{@code lossy-conversions}    <td>possible lossy conversions in compound assignment
        * <tr><th scope="row">{@code missing-explicit-ctor} <td>missing explicit constructors in public and protected classes
      @@ -186,7 +187,10 @@
        *                                                      and interfaces
        * <tr><th scope="row">{@code static}               <td>accessing a static member using an instance
        * <tr><th scope="row">{@code strictfp}             <td>unnecessary use of the {@code strictfp} modifier
      - * <tr><th scope="row">{@code synchronization}      <td>synchronization attempts on instances of value-based classes
      + * <tr><th scope="row">{@code synchronization}      <td>synchronization attempts on instances of value-based classes,
      + *                                                      superseded by the {@code identity} warning category which has the same
      + *                                                      uses and effects. Users are encouraged to use the {@code identity}
      + *                                                      category for all future an existing uses of {@code synchronization}
        * <tr><th scope="row">{@code text-blocks}          <td>inconsistent white space characters in text block indentation
        * <tr><th scope="row">{@code this-escape}          <td>superclass constructor leaking {@code this} before subclass initialized
        * <tr><th scope="row">{@code try}                  <td>issues relating to use of {@code try} blocks

      Changes to src/jdk.compiler/share/man/javac.md:

      diff --git a/src/jdk.compiler/share/man/javac.md b/src/jdk.compiler/share/man/javac.md
      index f951ea0def3..8bde0b94f81 100644
      --- a/src/jdk.compiler/share/man/javac.md
      +++ b/src/jdk.compiler/share/man/javac.md
      @@ -596,6 +596,9 @@ ### Extra Options
      
           -   `finally`: Warns about `finally` clauses that do not terminate normally.
      
      +    -   `identity`: Warns about use of a value-based class where an identity
      +        class is expected
      +
           -   `incubating`: Warns about the use of incubating modules.
      
           -   `lossy-conversions`: Warns about possible lossy conversions
      @@ -646,7 +649,9 @@ ### Extra Options
           -   `strictfp`: Warns about unnecessary use of the `strictfp` modifier.
      
           -   `synchronization`: Warns about synchronization attempts on instances
      -        of value-based classes.
      +        of value-based classes. Superseded by the `identity` warning category
      +        which has the same uses and effects. Users are encouraged to use the
      +        `identity` category for all future an existing uses of `synchronization`.
      
           -   `text-blocks`: Warns about inconsistent white space characters in text
               block indentation.

            vromero Vicente Arturo Romero Zaldivar
            dlsmith Dan Smith
            Chen Liang, Dan Smith
            Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: