Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
950f391
8354556: Expand value-based class warnings to java.lang.ref API
vicente-romero-oracle Apr 17, 2025
4c4aebe
minor fixes
vicente-romero-oracle Apr 21, 2025
0979978
Merge branch 'master' into JDK-8354556
vicente-romero-oracle Apr 21, 2025
d35ef50
fixing bug
vicente-romero-oracle Apr 21, 2025
ed64f22
removing the REQUIRES_IDENTITY flag
vicente-romero-oracle Apr 22, 2025
e6c17f7
removing requiresIdentityInternalType
vicente-romero-oracle Apr 22, 2025
d40e285
removing unnecessary changes
vicente-romero-oracle Apr 22, 2025
5a0b6bf
fixing bug
vicente-romero-oracle Apr 22, 2025
434dda7
only do extra analysis if lint warning enable
vicente-romero-oracle Apr 22, 2025
9f0407a
minor chantes to CreateSymbol
vicente-romero-oracle Apr 22, 2025
2697858
additional coverage for other uses of types
vicente-romero-oracle Apr 23, 2025
b4f5329
additional coverage for other uses of types
vicente-romero-oracle Apr 23, 2025
ac28815
Merge branch 'JDK-8354556' of https://github.com/vicente-romero-oracl…
vicente-romero-oracle Apr 24, 2025
5491f39
covering more cases plus refactoring in order to simplify the code
vicente-romero-oracle Apr 25, 2025
09b8331
refactoring
vicente-romero-oracle Apr 25, 2025
9d6af78
refactoring
vicente-romero-oracle Apr 25, 2025
4a6b9bc
more refactorings and tests
vicente-romero-oracle Apr 25, 2025
1c3cd61
more refactorings
vicente-romero-oracle Apr 25, 2025
84b52a6
removing unneeded changes
vicente-romero-oracle Apr 25, 2025
70e0311
additional refactorings
vicente-romero-oracle Apr 25, 2025
5e3acc9
final adjustments
vicente-romero-oracle Apr 25, 2025
1134ffe
updating test
vicente-romero-oracle May 2, 2025
151908a
Merge branch 'master' into JDK-8354556
vicente-romero-oracle May 2, 2025
60f1f53
changes to test
vicente-romero-oracle May 2, 2025
ad7f617
addressing review comment
vicente-romero-oracle May 5, 2025
5d834ce
Merge branch 'master' into JDK-8354556
vicente-romero-oracle May 8, 2025
d52ec33
documentation and adding alias to lint categories
vicente-romero-oracle May 8, 2025
1ff2f2c
additional documentation changes and bug fixes
vicente-romero-oracle May 9, 2025
ea63116
fixing bugs, removing dead code
vicente-romero-oracle May 9, 2025
a9a5378
integrating code from Archie
vicente-romero-oracle May 10, 2025
3c6dacb
removing dead code
vicente-romero-oracle May 10, 2025
973307e
additional changes from Archie
vicente-romero-oracle May 10, 2025
63fd151
Update src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
vicente-romero-oracle May 10, 2025
22acaf2
Merge branch 'master' into JDK-8354556
vicente-romero-oracle May 13, 2025
bdbcd33
update warning message
vicente-romero-oracle May 14, 2025
6c8d2b1
addressing review comments
vicente-romero-oracle May 15, 2025
def2505
addressing review comments
vicente-romero-oracle May 15, 2025
cf92614
review comments
vicente-romero-oracle May 19, 2025
904bd4c
Merge branch 'master' into JDK-8354556
vicente-romero-oracle May 19, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add handling of RequiresIdentity in createAnnotation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update on CreateSymbols, the complete implementation with the changes for CreateSymbols should be included in the PR for [1]. Which will be developed in parallel to this proposal

[1] https://bugs.openjdk.org/browse/JDK-8356894

Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,18 @@ public void createSymbols(String ctDescriptionFileExtra, String ctDescriptionFil
"Ljdk/internal/ValueBased;";
private static final String VALUE_BASED_ANNOTATION_INTERNAL =
"Ljdk/internal/ValueBased+Annotation;";
private static final String REQUIRES_IDENTITY_ANNOTATION =
"Ljdk/internal/RequiresIdentity;";
private static final String REQUIRES_IDENTITY_ANNOTATION_INTERNAL =
"Ljdk/internal/RequiresIdentity+Annotation;";
public static final Set<String> HARDCODED_ANNOTATIONS = new HashSet<>(
List.of("Ljdk/Profile+Annotation;",
"Lsun/Proprietary+Annotation;",
PREVIEW_FEATURE_ANNOTATION_OLD,
PREVIEW_FEATURE_ANNOTATION_NEW,
VALUE_BASED_ANNOTATION,
RESTRICTED_ANNOTATION));
RESTRICTED_ANNOTATION,
REQUIRES_IDENTITY_ANNOTATION));

private void stripNonExistentAnnotations(LoadDescriptions data) {
Set<String> allClasses = data.classes.name2Class.keySet();
Expand Down Expand Up @@ -1021,6 +1026,12 @@ private Annotation createAnnotation(AnnotationDescription desc) {
annotationType = VALUE_BASED_ANNOTATION_INTERNAL;
}

if (REQUIRES_IDENTITY_ANNOTATION.equals(annotationType)) {
//the non-public RequiresIdentity annotation will not be available in ct.sym,
//replace with purely synthetic javac-internal annotation:
annotationType = REQUIRES_IDENTITY_ANNOTATION_INTERNAL;
}

if (RESTRICTED_ANNOTATION.equals(annotationType)) {
//the non-public Restricted annotation will not be available in ct.sym,
//replace with purely synthetic javac-internal annotation:
Expand Down Expand Up @@ -2202,6 +2213,7 @@ private boolean readAttribute(FeatureDescription feature, Attribute<?> attr) {
chd.permittedSubclasses = a.permittedSubclasses().stream().map(ClassEntry::asInternalName).collect(Collectors.toList());
}
case ModuleMainClassAttribute a -> ((ModuleHeaderDescription) feature).moduleMainClass = a.mainClass().asInternalName();
case RuntimeVisibleTypeAnnotationsAttribute a -> {/* do nothing for now */}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will make createsymbol created older reference/weakhashmap APIs' type parameter for older releases always have this annotation; but this should be fine. We can fix this for 26. The question is that now their type parameter and the actual parameter uses will see some inconsistent values for releases < 25 in the future.

Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jan is taking a look at the changes for CreateSymbols. The change included in this PR is the minimum for the build to pass, I will double check with him if what he is doing will be pushed in a separate PR or as part of this PR, not sure yet. Time is tight and we want this in 25 this is why I decided to move on with this PR even though the CreateSymbols code complete. I forgot to mention this before, sorry

default -> throw new IllegalArgumentException("Unhandled attribute: " + attr.attributeName()); // Do nothing
}

Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/ref/Cleaner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/ref/Reference.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,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) {
Expand Down
6 changes: 3 additions & 3 deletions src/java.base/share/classes/java/lang/ref/SoftReference.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/java.base/share/classes/java/lang/ref/WeakReference.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@
* @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
* reference is not registered with any queue.
*
* @param referent object the new weak reference will refer to
*/
public WeakReference(T referent) {
public WeakReference(@jdk.internal.RequiresIdentity T referent) {
super(referent);
}

Expand All @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/util/WeakHashMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand Down Expand Up @@ -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();
Expand Down
51 changes: 51 additions & 0 deletions src/java.base/share/classes/jdk/internal/RequiresIdentity.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright (c) 2025, 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. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* 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.
*/

package jdk.internal;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.ElementType.TYPE_PARAMETER;

/**
* Indicates that the annotated parameter or type parameter is not expected to be a
* Value Based class.
* Using a parameter or type parameter of a <a href="../lang/doc-files/ValueBased.html">value-based classes</a>
* should produce warnings about behavior that is inconsistent with identity based semantics.
*
* Note this internal annotation is handled specially by the javac compiler.
* To work properly with {@code --release older-release}, it requires special
* handling in {@code make/langtools/src/classes/build/tools/symbolgenerator/CreateSymbols.java}
* and {@code src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java}.
*
* @since 25
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(value={PARAMETER, TYPE_PARAMETER})
public @interface RequiresIdentity {
}
1 change: 0 additions & 1 deletion src/java.base/share/classes/jdk/internal/ValueBased.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,3 @@
@Target(value={TYPE})
public @interface ValueBased {
}

62 changes: 33 additions & 29 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,15 @@

package com.sun.tools.javac.code;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.Set;
import java.util.stream.Stream;

import com.sun.tools.javac.main.Option;
Expand Down Expand Up @@ -119,7 +123,7 @@ public Lint suppress(LintCategory... lc) {
private EnumSet<LintCategory> values;
private EnumSet<LintCategory> suppressedValues;

private static final Map<String, LintCategory> map = new ConcurrentHashMap<>(20);
private static final Map<String, LintCategory> map = new LinkedHashMap<>(40);

@SuppressWarnings("this-escape")
protected Lint(Context context) {
Expand Down Expand Up @@ -149,10 +153,10 @@ private void initializeRootIfNeeded() {
return;

// Initialize enabled categories based on "-Xlint" flags
if (options.isSet(Option.XLINT) || options.isSet(Option.XLINT_CUSTOM, "all")) {
if (options.isSet(Option.XLINT) || options.isSet(Option.XLINT_CUSTOM, Option.LINT_CUSTOM_ALL)) {
// If -Xlint or -Xlint:all is given, enable all categories by default
values = EnumSet.allOf(LintCategory.class);
} else if (options.isSet(Option.XLINT_CUSTOM, "none")) {
} else if (options.isSet(Option.XLINT_CUSTOM, Option.LINT_CUSTOM_NONE)) {
// if -Xlint:none is given, disable all categories by default
values = LintCategory.newEmptySet();
} else {
Expand All @@ -173,15 +177,15 @@ private void initializeRootIfNeeded() {
if (!options.isSet(Option.PREVIEW)) {
values.add(LintCategory.PREVIEW);
}
values.add(LintCategory.SYNCHRONIZATION);
values.add(LintCategory.IDENTITY);
values.add(LintCategory.INCUBATING);
}

// Look for specific overrides
for (LintCategory lc : LintCategory.values()) {
if (options.isSet(Option.XLINT_CUSTOM, lc.option)) {
if (options.isExplicitlyEnabled(Option.XLINT, lc)) {
values.add(lc);
} else if (options.isSet(Option.XLINT_CUSTOM, "-" + lc.option)) {
} else if (options.isExplicitlyDisabled(Option.XLINT, lc)) {
values.remove(lc);
}
}
Expand Down Expand Up @@ -261,6 +265,11 @@ public enum LintCategory {
*/
FINALLY("finally"),

/**
* Warn about uses of @ValueBased classes where an identity class is expected.
*/
IDENTITY("identity", true, "synchronization"),

/**
* Warn about use of incubating modules.
*
Expand Down Expand Up @@ -363,11 +372,6 @@ public enum LintCategory {
*/
STRICTFP("strictfp"),

/**
* Warn about synchronization attempts on instances of @ValueBased classes.
*/
SYNCHRONIZATION("synchronization"),

/**
* Warn about issues relating to use of text blocks
*
Expand Down Expand Up @@ -410,10 +414,14 @@ public enum LintCategory {
this(option, true);
}

LintCategory(String option, boolean annotationSuppression) {
LintCategory(String option, boolean annotationSuppression, String... aliases) {
this.option = option;
this.annotationSuppression = annotationSuppression;
map.put(option, this);
ArrayList<String> optionList = new ArrayList<>(1 + aliases.length);
optionList.add(option);
Collections.addAll(optionList, aliases);
this.optionList = Collections.unmodifiableList(optionList);
this.optionList.forEach(ident -> map.put(ident, this));
}

/**
Expand All @@ -426,13 +434,23 @@ public static Optional<LintCategory> get(String option) {
return Optional.ofNullable(map.get(option));
}

/**
* Get all lint category option strings and aliases.
*/
public static Set<String> options() {
return Collections.unmodifiableSet(map.keySet());
}

public static EnumSet<LintCategory> newEmptySet() {
return EnumSet.noneOf(LintCategory.class);
}

/** Get the string representing this category in @SuppressAnnotations and -Xlint options. */
/** Get the "canonical" string representing this category in @SuppressAnnotations and -Xlint options. */
public final String option;

/** Get a list containing "option" followed by zero or more aliases. */
public final List<String> optionList;

/** Does this category support being suppressed by the {@code @SuppressWarnings} annotation? */
public final boolean annotationSuppression;
}
Expand Down Expand Up @@ -496,20 +514,6 @@ public EnumSet<LintCategory> suppressionsFrom(Symbol symbol) {
return suppressions;
}

/**
* Retrieve the recognized lint categories suppressed by the given @SuppressWarnings annotation.
*
* @param annotation @SuppressWarnings annotation, or null
* @return set of lint categories, possibly empty but never null
*/
private EnumSet<LintCategory> suppressionsFrom(JCAnnotation annotation) {
initializeSymbolsIfNeeded();
if (annotation == null)
return LintCategory.newEmptySet();
Assert.check(annotation.attribute.type.tsym == syms.suppressWarningsType.tsym);
return suppressionsFrom(Stream.of(annotation).map(anno -> anno.attribute));
}

// Find the @SuppressWarnings annotation in the given stream and extract the recognized suppressions
private EnumSet<LintCategory> suppressionsFrom(Stream<Attribute.Compound> attributes) {
initializeSymbolsIfNeeded();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ public static Symtab instance(Context context) {
public final Type constantBootstrapsType;
public final Type valueBasedType;
public final Type valueBasedInternalType;
public final Type requiresIdentityType;
public final Type requiresIdentityInternalType;
public final Type classDescType;
public final Type enumDescType;

Expand Down Expand Up @@ -610,6 +612,8 @@ public <R, P> R accept(ElementVisitor<R, P> v, P p) {
constantBootstrapsType = enterClass("java.lang.invoke.ConstantBootstraps");
valueBasedType = enterClass("jdk.internal.ValueBased");
valueBasedInternalType = enterSyntheticAnnotation("jdk.internal.ValueBased+Annotation");
requiresIdentityType = enterClass("jdk.internal.RequiresIdentity");
requiresIdentityInternalType = enterSyntheticAnnotation("jdk.internal.RequiresIdentity+Annotation");
classDescType = enterClass("java.lang.constant.ClassDesc");
enumDescType = enterClass("java.lang.Enum$EnumDesc");
// For serialization lint checking
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,10 @@ public boolean isFinal() {
return (tsym.flags() & FINAL) != 0;
}

public boolean isValueBased() {
return tsym != null && (tsym.flags_field & VALUE_BASED) != 0;
}

/**
* Does this type contain occurrences of type t?
*/
Expand Down
Loading