From ce03d3d5a031311781884a70e9b67c331ba70e4e Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Thu, 26 Feb 2026 17:02:45 +0100 Subject: [PATCH 01/17] Annotation processing for generating value class sources # Conflicts: # make/common/Modules.gmk --- make/CompileJavaModules.gmk | 16 ++ make/common/JavaCompilation.gmk | 7 +- make/common/Modules.gmk | 2 +- .../tools/valueclasses/GenValueClasses.java | 250 ++++++++++++++++++ make/modules/java.base/Gensrc.gmk | 1 - .../java.base/gensrc/GensrcValueClasses.gmk | 76 ------ 6 files changed, 273 insertions(+), 79 deletions(-) create mode 100644 make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java delete mode 100644 make/modules/java.base/gensrc/GensrcValueClasses.gmk diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk index 1b6e95d1afb..a9f32f1ef79 100644 --- a/make/CompileJavaModules.gmk +++ b/make/CompileJavaModules.gmk @@ -68,6 +68,20 @@ MODULESOURCEPATH := $(call GetModuleSrcPath) # Add imported modules to the modulepath MODULEPATH := $(call PathList, $(IMPORT_MODULES_CLASSES)) +################################################################################ +# Setup preprocessor flags +# The output directory must be present in GENERATED_PREVIEW_SUBDIRS in Modules.gmk. +# Temporarily restrict this to java.base, but it can be expanded later. + +ifeq ($(MODULE), java.base) + PREPROCESSOR_FLAGS := \ + -Xlint:-removal -Xlint:-processing \ + -Avalueclasses.outdir=$(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \ + -processor build.tools.valueclasses.GenValueClasses + + PROCESSOR_PATH += $(BUILDTOOLS_OUTPUTDIR)/jdk_tools_classes +endif + ################################################################################ # Copy zh_HK properties files from zh_TW (needed by some modules) @@ -120,9 +134,11 @@ $(eval $(call SetupJavaCompilation, $(MODULE), \ EXCLUDE_PATTERNS := -files, \ KEEP_ALL_TRANSLATIONS := $(KEEP_ALL_TRANSLATIONS), \ TARGET_RELEASE := $(TARGET_RELEASE), \ + PROCESSOR_PATH := $(PROCESSOR_PATH), \ JAVAC_FLAGS := \ $(DOCLINT) \ $(JAVAC_FLAGS) \ + $(PREPROCESSOR_FLAGS) \ --module-source-path $(MODULESOURCEPATH) \ --module-path $(MODULEPATH) \ --system none, \ diff --git a/make/common/JavaCompilation.gmk b/make/common/JavaCompilation.gmk index 33f5d10535a..bde59a2904a 100644 --- a/make/common/JavaCompilation.gmk +++ b/make/common/JavaCompilation.gmk @@ -302,13 +302,18 @@ define SetupJavaCompilationBody # including the compilation output on the classpath, so that incremental # compilations in unnamed module can refer to other classes from the same # source root, which are not being recompiled in this compilation: - $1_AUGMENTED_CLASSPATH += $$(BUILDTOOLS_OUTPUTDIR)/depend $$($1_BIN) + $1_AUGMENTED_CLASSPATH += $$($1_BIN) + $1_PROCESSOR_PATH += $$(BUILDTOOLS_OUTPUTDIR)/depend endif ifneq ($$($1_AUGMENTED_CLASSPATH), ) $1_FLAGS += -cp $$(call PathList, $$($1_AUGMENTED_CLASSPATH)) endif + ifneq ($$($1_PROCESSOR_PATH), ) + $1_FLAGS += --processor-path $$(call PathList, $$($1_PROCESSOR_PATH)) + endif + # Make sure the dirs exist, or that one of the EXTRA_FILES, that may not # exist yet, is in it. $$(foreach d, $$($1_SRC), \ diff --git a/make/common/Modules.gmk b/make/common/Modules.gmk index 6fe6ad80726..cf2af726923 100644 --- a/make/common/Modules.gmk +++ b/make/common/Modules.gmk @@ -75,7 +75,7 @@ GENERATED_SRC_DIRS += \ $(SUPPORT_OUTPUTDIR)/gensrc \ # -# Directories in which generated preview classes may exist. +# Directories in which generated preview classes may exists. # Currently this is restricted to generated value classes, but can be extended. GENERATED_PREVIEW_SUBDIRS += \ $(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \ diff --git a/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java b/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java new file mode 100644 index 00000000000..1be9d12f5e5 --- /dev/null +++ b/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java @@ -0,0 +1,250 @@ +package build.tools.valueclasses; + +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.util.TreePath; +import com.sun.source.util.Trees; + +import javax.annotation.processing.AbstractProcessor; +import javax.annotation.processing.ProcessingEnvironment; +import javax.annotation.processing.RoundEnvironment; +import javax.annotation.processing.SupportedAnnotationTypes; +import javax.annotation.processing.SupportedOptions; +import javax.lang.model.SourceVersion; +import javax.lang.model.element.Element; +import javax.lang.model.element.TypeElement; +import javax.tools.FileObject; +import java.io.File; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStreamWriter; +import java.io.Reader; +import java.io.Writer; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + +import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static java.util.stream.Collectors.groupingBy; + +/** + * Annotation processor for generating preview sources of existing classes which + * annotated as value classes for Valhalla. + * + *

Classes seen by this processor (annotated with {@code @MigratedValueClass} + * will have their source files re-written into the configured output directory + * for compilation as preview classes. Note that more than one class in a given + * source file may be annotated. + * + *

Class re-writing is simply a matter of injecting the new "value" keyword + * into a copy of the original source file after existing modifiers for all + * annotated elements. + * + *

Note that there are two annotations in use for value classes, but since + * we must generate sources for abstract classes, we only care about one of them. + *

+ */ +@SupportedAnnotationTypes({"jdk.internal.MigratedValueClass"}) +@SupportedOptions("valueclasses.outdir") +public final class GenValueClasses extends AbstractProcessor { + // Matches preprocessor option flag in CompileJavaModules.gmk. + private static final String OUTDIR_OPTION_KEY = "valueclasses.outdir"; + + private ProcessingEnvironment processingEnv = null; + private Path outDir = null; + private Trees trees = null; + + @Override + public synchronized void init(ProcessingEnvironment processingEnv) { + super.init(processingEnv); + this.processingEnv = processingEnv; + String outDir = this.processingEnv.getOptions().get(OUTDIR_OPTION_KEY); + if (outDir == null) { + throw new IllegalStateException( + "Must specify -A" + OUTDIR_OPTION_KEY + "=" + + " for annotation processor: " + GenValueClasses.class.getName()); + } + this.outDir = Path.of(outDir.replace('/', File.separatorChar)); + this.trees = Trees.instance(this.processingEnv); + } + + /** + * Override to return latest version, since the runtime in which this is + * compiled doesn't know about development source versions. + */ + @Override + public SourceVersion getSupportedSourceVersion() { + return SourceVersion.latest(); + } + + @Override + public boolean process(Set annotations, RoundEnvironment env) { + // We don't have direct access to MigratedValueClass classes here. + Optional valueClassAnnotation = + getAnnotation(annotations, "jdk.internal.MigratedValueClass"); + if (valueClassAnnotation.isPresent()) { + Set allValueClasses = getAnnotatedTypes(env, valueClassAnnotation.get()); + Map> moduleToType = + allValueClasses.stream().collect(groupingBy(this::getModuleName)); + for (String modName : moduleToType.keySet()) { + moduleToType.get(modName).stream() + .collect(groupingBy(this::getJavaSourceFile)) + .forEach(this::generateValueClassSource); + } + } + return true; + } + + private static Optional getAnnotation(Set annotations, String name) { + return annotations.stream() + .filter(e -> e.getQualifiedName().toString().equals(name)) + .findFirst(); + } + + private static Set getAnnotatedTypes(RoundEnvironment env, TypeElement annotation) { + Set types = new HashSet<>(); + for (Element e : env.getElementsAnnotatedWith(annotation)) { + if (!e.getKind().isClass()) { + throw new IllegalStateException( + "Unexpected element kind (" + e.getKind() + ") for element: " + e); + } + TypeElement type = (TypeElement) e; + if (type.getQualifiedName().isEmpty()) { + throw new IllegalStateException( + "Unexpected empty name for element: " + e); + } + types.add(type); + } + return types; + } + + private void generateValueClassSource(Path srcPath, List classes) { + try { + // We know there's at least one element per source file (by construction). + TypeElement element = classes.getFirst(); + Path relPath = getModuleRelativePath(srcPath, getPackageName(element)); + Path outPath = outDir.resolve(getModuleName(element)).resolve(relPath); + Files.createDirectories(outPath.getParent()); + + List insertPositions = + classes.stream().map(this::getValueKeywordInsertPosition).sorted().toList(); + + try (Reader reader = new InputStreamReader(Files.newInputStream(srcPath)); + Writer output = new OutputStreamWriter(Files.newOutputStream(outPath, CREATE_NEW))) { + long curPos = 0; + for (long nxtPos : insertPositions) { + int nextChunkLen = Math.toIntExact(nxtPos - curPos); + long written = new LimitedReader(reader, nextChunkLen).transferTo(output); + if (written != nextChunkLen) { + throw new IOException("Unexpected number of characters transferred." + + " Expected " + nextChunkLen + " but was " + written); + } + // Position is the end of modifier chars, so we need a leading space. + // pos ------v + // [modifiers] class... -->> [modifiers] value class... + output.write(" value"); + curPos = nxtPos; + } + reader.transferTo(output); + } + } catch (IOException e) { + // TODO: Decide about errors! + throw new RuntimeException(e); + } + } + + private long getValueKeywordInsertPosition(TypeElement classElement) { + TreePath classDecl = trees.getPath(classElement); + ClassTree classTree = (ClassTree) classDecl.getLeaf(); + CompilationUnitTree compilationUnit = classDecl.getCompilationUnit(); + // Since annotations are held as "modifiers", and since we only process + // elements with annotations, the positions for modifiers must be + // well-defined (otherwise we'd get -1 here). + return trees.getSourcePositions().getEndPosition(compilationUnit, classTree.getModifiers()); + } + + private Path getModuleRelativePath(Path srcPath, String pkgName) { + Path relPath = Path.of(pkgName.replace('.', File.separatorChar)).resolve(srcPath.getFileName()); + if (!srcPath.endsWith(relPath)) { + throw new IllegalStateException(String.format( + "Expected trailing path %s for source file %s", relPath, srcPath)); + } + return relPath; + } + + private String getModuleName(TypeElement t) { + return processingEnv.getElementUtils().getModuleOf(t).getQualifiedName().toString(); + } + + private String getPackageName(TypeElement t) { + return processingEnv.getElementUtils().getPackageOf(t).getQualifiedName().toString(); + } + + private Path getJavaSourceFile(TypeElement type) { + return getFilePath(processingEnv.getElementUtils().getFileObjectOf(type)); + } + + private static Path getFilePath(FileObject file) { + return Path.of(file.toUri()); + } + + /** + * A forwarding reader which guarantees to read no more than + * {@code maxCharCount} characters from the underlying stream. + */ + private static final class LimitedReader extends Reader { + // Since these are expected to be short-lived, no need + // to null the delegate when we're closed. + private final Reader delegate; + // This should never go negative. + private int remaining; + + /** + * Creates a limited reader which reads up to {@code maxCharCount} chars + * from the given stream. + * + * @param delegate underlying reader + * @param maxCharCount maximum chars to read (can be 0) + */ + LimitedReader(Reader delegate, int maxCharCount) { + this.delegate = Objects.requireNonNull(delegate); + this.remaining = Math.max(maxCharCount, 0); + } + + @Override + public int read(char[] cbuf, int off, int len) throws IOException { + if (remaining == 0) { + return -1; + } + if (remaining > 0) { + int readLimit = Math.min(remaining, len); + int count = delegate.read(cbuf, off, readLimit); + // Only update remaining if something was read. + if (count > 0) { + if (count > remaining) { + throw new IOException( + "Underlying Reader exceeded requested read limit." + + " Expected at most " + readLimit + " but read " + count); + } + remaining -= count; + } + return count; + } + throw new IllegalStateException("Remaining count should never be negative!"); + } + + @Override + public void close() { + // Do not close the delegate since this is conceptually just a view. + } + } +} diff --git a/make/modules/java.base/Gensrc.gmk b/make/modules/java.base/Gensrc.gmk index 0406c5b7033..e8236f0b0e4 100644 --- a/make/modules/java.base/Gensrc.gmk +++ b/make/modules/java.base/Gensrc.gmk @@ -37,7 +37,6 @@ include gensrc/GensrcMisc.gmk include gensrc/GensrcModuleLoaderMap.gmk include gensrc/GensrcRegex.gmk include gensrc/GensrcScopedMemoryAccess.gmk -include gensrc/GensrcValueClasses.gmk include gensrc/GensrcVarHandles.gmk ################################################################################ diff --git a/make/modules/java.base/gensrc/GensrcValueClasses.gmk b/make/modules/java.base/gensrc/GensrcValueClasses.gmk deleted file mode 100644 index 6a5b6864b67..00000000000 --- a/make/modules/java.base/gensrc/GensrcValueClasses.gmk +++ /dev/null @@ -1,76 +0,0 @@ -# -# Copyright (c) 2023, 2024, 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. -# - -################################################################################ -# Generate the value class replacements for selected java.base source files - -java.base-VALUE_CLASS-REPLACEMENTS := \ - java/lang/Byte.java \ - java/lang/Short.java \ - java/lang/Integer.java \ - java/lang/Long.java \ - java/lang/Float.java \ - java/lang/Double.java \ - java/lang/Boolean.java \ - java/lang/Character.java \ - java/lang/Number.java \ - java/lang/Record.java \ - java/util/Optional.java \ - java/util/OptionalInt.java \ - java/util/OptionalLong.java \ - java/util/OptionalDouble.java \ - java/time/LocalDate.java \ - java/time/LocalDateTime.java \ - java/time/LocalTime.java \ - java/time/Duration.java \ - java/time/Instant.java \ - java/time/MonthDay.java \ - java/time/ZonedDateTime.java \ - java/time/OffsetDateTime.java \ - java/time/OffsetTime.java \ - java/time/YearMonth.java \ - java/time/Year.java \ - java/time/Period.java \ - java/time/chrono/ChronoLocalDateImpl.java \ - java/time/chrono/MinguoDate.java \ - java/time/chrono/HijrahDate.java \ - java/time/chrono/JapaneseDate.java \ - java/time/chrono/ThaiBuddhistDate.java \ - # - -java.base-VALUE-CLASS-FILES := \ - $(foreach f, $(java.base-VALUE_CLASS-REPLACEMENTS), $(addprefix $(TOPDIR)/src/java.base/share/classes/, $(f))) - -$(eval $(call SetupTextFileProcessing, JAVA_BASE_VALUECLASS_REPLACEMENTS, \ - SOURCE_FILES := $(java.base-VALUE-CLASS-FILES), \ - SOURCE_BASE_DIR := $(TOPDIR)/src/java.base/share/classes, \ - OUTPUT_DIR := $(SUPPORT_OUTPUTDIR)/gensrc-valueclasses/java.base/, \ - REPLACEMENTS := \ - public final class => public final value class ; \ - public abstract class => public abstract value class ; \ - abstract class ChronoLocalDateImpl => abstract value class ChronoLocalDateImpl, \ -)) - -TARGETS += $(JAVA_BASE_VALUECLASS_REPLACEMENTS) From 1151df66b20bdfb99f18a95399519898d956b31b Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Thu, 26 Feb 2026 18:50:18 +0100 Subject: [PATCH 02/17] Tidying up --- make/common/Modules.gmk | 2 +- .../tools/valueclasses/GenValueClasses.java | 87 +++++++++++-------- 2 files changed, 50 insertions(+), 39 deletions(-) diff --git a/make/common/Modules.gmk b/make/common/Modules.gmk index cf2af726923..6fe6ad80726 100644 --- a/make/common/Modules.gmk +++ b/make/common/Modules.gmk @@ -75,7 +75,7 @@ GENERATED_SRC_DIRS += \ $(SUPPORT_OUTPUTDIR)/gensrc \ # -# Directories in which generated preview classes may exists. +# Directories in which generated preview classes may exist. # Currently this is restricted to generated value classes, but can be extended. GENERATED_PREVIEW_SUBDIRS += \ $(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \ diff --git a/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java b/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java index 1be9d12f5e5..158572e4fad 100644 --- a/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java @@ -13,6 +13,7 @@ import javax.lang.model.SourceVersion; import javax.lang.model.element.Element; import javax.lang.model.element.TypeElement; +import javax.tools.Diagnostic; import javax.tools.FileObject; import java.io.File; import java.io.IOException; @@ -24,7 +25,6 @@ import java.nio.file.Path; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -33,20 +33,19 @@ import static java.util.stream.Collectors.groupingBy; /** - * Annotation processor for generating preview sources of existing classes which - * annotated as value classes for Valhalla. + * Annotation processor for generating preview sources of classes annotated as + * value classes for preview mode. * *

Classes seen by this processor (annotated with {@code @MigratedValueClass} - * will have their source files re-written into the configured output directory + * will have their source files re-written into the specified output directory * for compilation as preview classes. Note that more than one class in a given * source file may be annotated. * - *

Class re-writing is simply a matter of injecting the new "value" keyword - * into a copy of the original source file after existing modifiers for all - * annotated elements. + *

Class re-writing is achieved by injecting the "value" keyword in front of + * class declarations for all annotated elements in the original source file. * *

Note that there are two annotations in use for value classes, but since - * we must generate sources for abstract classes, we only care about one of them. + * we must generate sources for abstract classes, we only process one of them. *

    *
  • {@code @jdk.internal.ValueBased} appears on concrete value classes. *
  • {@code @jdk.internal.MigratedValueClass} appears on concrete and @@ -92,24 +91,22 @@ public boolean process(Set annotations, RoundEnvironment Optional valueClassAnnotation = getAnnotation(annotations, "jdk.internal.MigratedValueClass"); if (valueClassAnnotation.isPresent()) { - Set allValueClasses = getAnnotatedTypes(env, valueClassAnnotation.get()); - Map> moduleToType = - allValueClasses.stream().collect(groupingBy(this::getModuleName)); - for (String modName : moduleToType.keySet()) { - moduleToType.get(modName).stream() - .collect(groupingBy(this::getJavaSourceFile)) - .forEach(this::generateValueClassSource); - } + getAnnotatedTypes(env, valueClassAnnotation.get()).stream() + .collect(groupingBy(this::getJavaSourceFile)) + .forEach(this::generateValueClassSource); } - return true; + // We may not be the only annotation processor to consume this annotation. + return false; } + /** Find the annotation element by name in the given set. */ private static Optional getAnnotation(Set annotations, String name) { return annotations.stream() .filter(e -> e.getQualifiedName().toString().equals(name)) .findFirst(); } + /** Find the type elements (classes) annotated with the given annotation element. */ private static Set getAnnotatedTypes(RoundEnvironment env, TypeElement annotation) { Set types = new HashSet<>(); for (Element e : env.getElementsAnnotatedWith(annotation)) { @@ -127,6 +124,11 @@ private static Set getAnnotatedTypes(RoundEnvironment env, TypeElem return types; } + /** + * Write a transformed version of the given Java source file with the + * {@code value} keyword inserted before the class declaration of each + * annotated type element. + */ private void generateValueClassSource(Path srcPath, List classes) { try { // We know there's at least one element per source file (by construction). @@ -148,28 +150,37 @@ private void generateValueClassSource(Path srcPath, List classes) { throw new IOException("Unexpected number of characters transferred." + " Expected " + nextChunkLen + " but was " + written); } - // Position is the end of modifier chars, so we need a leading space. - // pos ------v - // [modifiers] class... -->> [modifiers] value class... - output.write(" value"); curPos = nxtPos; + // curPos is at the end of the modifier section, so add a leading space. + // curPos ---v + // [modifiers] class... -->> [modifiers] value class... + output.write(" value"); } + // Trailing section to end-of-file transferred from original reader. reader.transferTo(output); } } catch (IOException e) { - // TODO: Decide about errors! throw new RuntimeException(e); } } + /** + * Returns the character offset in the original source file at which to insert + * the {@code value} keyword. The offset is the end of the modifiers section, + * which must immediately precede the class declaration. + */ private long getValueKeywordInsertPosition(TypeElement classElement) { TreePath classDecl = trees.getPath(classElement); ClassTree classTree = (ClassTree) classDecl.getLeaf(); CompilationUnitTree compilationUnit = classDecl.getCompilationUnit(); // Since annotations are held as "modifiers", and since we only process - // elements with annotations, the positions for modifiers must be - // well-defined (otherwise we'd get -1 here). - return trees.getSourcePositions().getEndPosition(compilationUnit, classTree.getModifiers()); + // elements with annotations, the positions of the modifiers section must + // be well-defined. + long pos = trees.getSourcePositions().getEndPosition(compilationUnit, classTree.getModifiers()); + if (pos == Diagnostic.NOPOS) { + throw new IllegalStateException("Missing position information: " + classElement); + } + return pos; } private Path getModuleRelativePath(Path srcPath, String pkgName) { @@ -202,11 +213,10 @@ private static Path getFilePath(FileObject file) { * {@code maxCharCount} characters from the underlying stream. */ private static final class LimitedReader extends Reader { - // Since these are expected to be short-lived, no need - // to null the delegate when we're closed. + // These are short-lived, no need to null the delegate when closed. private final Reader delegate; // This should never go negative. - private int remaining; + private int remainingChars; /** * Creates a limited reader which reads up to {@code maxCharCount} chars @@ -217,29 +227,30 @@ private static final class LimitedReader extends Reader { */ LimitedReader(Reader delegate, int maxCharCount) { this.delegate = Objects.requireNonNull(delegate); - this.remaining = Math.max(maxCharCount, 0); + this.remainingChars = Math.max(maxCharCount, 0); } @Override public int read(char[] cbuf, int off, int len) throws IOException { - if (remaining == 0) { - return -1; - } - if (remaining > 0) { - int readLimit = Math.min(remaining, len); + if (remainingChars > 0) { + int readLimit = Math.min(remainingChars, len); int count = delegate.read(cbuf, off, readLimit); - // Only update remaining if something was read. + // Only update remainingChars if something was read. if (count > 0) { - if (count > remaining) { + if (count > remainingChars) { throw new IOException( "Underlying Reader exceeded requested read limit." + " Expected at most " + readLimit + " but read " + count); } - remaining -= count; + remainingChars -= count; } + // Can return 0 or -1 here (the underlying reader could finish first). return count; + } else if (remainingChars == 0) { + return -1; + } else { + throw new AssertionError("Remaining character count should never be negative!"); } - throw new IllegalStateException("Remaining count should never be negative!"); } @Override From 89841e153880c85daad451d61de9b340d38d6692 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Thu, 26 Feb 2026 19:02:32 +0100 Subject: [PATCH 03/17] Add copyright --- .../tools/valueclasses/GenValueClasses.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java b/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java index 158572e4fad..8508dc0c8a8 100644 --- a/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java @@ -1,3 +1,28 @@ +/* + * Copyright (c) 2026, 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 build.tools.valueclasses; import com.sun.source.tree.ClassTree; From 8f9c78ca087d2e188acb3f4bff46dbf28148e963 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Fri, 27 Feb 2026 18:51:18 +0100 Subject: [PATCH 04/17] Working tests and moved processor --- make/CompileJavaModules.gmk | 2 +- .../valuetypes}/GenValueClasses.java | 9 +- .../valuetypes/GenValueClassesTest.java | 158 ++++++++++++++++++ 3 files changed, 165 insertions(+), 4 deletions(-) rename make/jdk/src/classes/build/tools/{valueclasses => valhalla/valuetypes}/GenValueClasses.java (96%) create mode 100644 test/jdk/valhalla/valuetypes/GenValueClassesTest.java diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk index a9f32f1ef79..0eaa0d089f2 100644 --- a/make/CompileJavaModules.gmk +++ b/make/CompileJavaModules.gmk @@ -77,7 +77,7 @@ ifeq ($(MODULE), java.base) PREPROCESSOR_FLAGS := \ -Xlint:-removal -Xlint:-processing \ -Avalueclasses.outdir=$(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \ - -processor build.tools.valueclasses.GenValueClasses + -processor build.tools.valhalla.valuetypes.GenValueClasses PROCESSOR_PATH += $(BUILDTOOLS_OUTPUTDIR)/jdk_tools_classes endif diff --git a/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java similarity index 96% rename from make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java rename to make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java index 8508dc0c8a8..1f48aa30575 100644 --- a/make/jdk/src/classes/build/tools/valueclasses/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java @@ -23,7 +23,7 @@ * questions. */ -package build.tools.valueclasses; +package build.tools.valhalla.valuetypes; import com.sun.source.tree.ClassTree; import com.sun.source.tree.CompilationUnitTree; @@ -54,7 +54,8 @@ import java.util.Optional; import java.util.Set; -import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static java.nio.file.StandardOpenOption.CREATE; +import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; import static java.util.stream.Collectors.groupingBy; /** @@ -165,8 +166,10 @@ private void generateValueClassSource(Path srcPath, List classes) { List insertPositions = classes.stream().map(this::getValueKeywordInsertPosition).sorted().toList(); + // For partial rebuilds, generated sources may still exist, so we overwrite them. try (Reader reader = new InputStreamReader(Files.newInputStream(srcPath)); - Writer output = new OutputStreamWriter(Files.newOutputStream(outPath, CREATE_NEW))) { + Writer output = new OutputStreamWriter( + Files.newOutputStream(outPath, CREATE, TRUNCATE_EXISTING))) { long curPos = 0; for (long nxtPos : insertPositions) { int nextChunkLen = Math.toIntExact(nxtPos - curPos); diff --git a/test/jdk/valhalla/valuetypes/GenValueClassesTest.java b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java new file mode 100644 index 00000000000..d05b5b48f04 --- /dev/null +++ b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java @@ -0,0 +1,158 @@ +/* + * Copyright (c) 2026, 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. + */ + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.spi.ToolProvider; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/* + * @test + * @summary Ensure annotation processing generates expected source files + * @library /tools/lib + * @compile ../../../../make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java + * @run junit GenValueClassesTest + */ +public class GenValueClassesTest { + private static final ToolProvider JAVAC_TOOL = ToolProvider.findFirst("javac") + .orElseThrow(() -> new RuntimeException("javac tool not found")); + + private static final String SIMPLE_VALUE_CLASS = + """ + package test; + + @jdk.internal.MigratedValueClass + public /*VALUE*/ class SimpleValueClass { + int value; + } + """; + + private static final String NESTED_VALUE_CLASS = + """ + package test; + + public class NestedValueClass { + @jdk.internal.MigratedValueClass + private static final /*VALUE*/ class Nested { + int value; + } + } + """; + + private static final String MULTIPLE_VALUE_CLASSES = + """ + package test; + + @jdk.internal.MigratedValueClass + public /*VALUE*/ class MultipleValueClasses { + + @jdk.internal.MigratedValueClass + private static final /*VALUE*/ class First { } + + static final class Second { } + + @jdk.internal.MigratedValueClass + private static /*VALUE*/ class Third { } + } + """; + + @TempDir + Path testDir = null; + Path srcDir = null; + Path outDir = null; + + @BeforeEach + void setupDirs() throws IOException { + this.srcDir = testDir.resolve("src"); + Files.createDirectories(srcDir); + this.outDir = testDir.resolve("out"); + Files.createDirectories(outDir); + } + + @Test + public void simpleValueClass() throws IOException { + Path relPath = writeTestSource("SimpleValueClass", SIMPLE_VALUE_CLASS); + compileTestClass(relPath); + String transformedSrc = Files.readString(outDir.resolve(relPath)); + assertEquals(SIMPLE_VALUE_CLASS.replace("/*VALUE*/", "value"), transformedSrc); + assertTrue(transformedSrc.contains(" value class SimpleValueClass ")); + } + + @Test + public void nestedValueClass() throws IOException { + Path relPath = writeTestSource("NestedValueClass", NESTED_VALUE_CLASS); + compileTestClass(relPath); + String transformedSrc = Files.readString(outDir.resolve(relPath)); + assertEquals(NESTED_VALUE_CLASS.replace("/*VALUE*/", "value"), transformedSrc); + assertTrue(transformedSrc.contains(" value class Nested ")); + } + + @Test + public void multipleValueClasses() throws IOException { + Path relPath = writeTestSource("MultipleValueClasses", MULTIPLE_VALUE_CLASSES); + compileTestClass(relPath); + String transformedSrc = Files.readString(outDir.resolve(relPath)); + assertEquals(MULTIPLE_VALUE_CLASSES.replace("/*VALUE*/", "value"), transformedSrc); + assertTrue(transformedSrc.contains(" value class MultipleValueClasses ")); + assertTrue(transformedSrc.contains(" value class First ")); + assertFalse(transformedSrc.contains(" value class Second ")); + assertTrue(transformedSrc.contains(" value class Third ")); + } + + private Path writeTestSource(String className, String source) throws IOException { + // Remove the VALUE tokens to leave "clean" source (otherwise the + // token will persist in the transformed output and get messy). + assertTrue(source.contains("/*VALUE*/ "), "invalid test source"); + String actualSrc = source.replace("/*VALUE*/ ", ""); + + Path relPath = Path.of("test", className + ".java"); + Path srcFile = srcDir.resolve(relPath); + Files.createDirectories(srcFile.getParent()); + Files.writeString(srcFile, actualSrc); + return relPath; + } + + // NOTE: Since the in-memory compiler is limited and doesn't support getting + // the Path of the "memo:" URIs is uses, it cannot be used for testing this + // annotation processor. Thus, we must perform on-disk compilation. + private void compileTestClass(Path srcPath) { + int exitValue = JAVAC_TOOL.run(System.out, System.err, + "-d", testDir.resolve("compiled").toString(), + "--add-exports", "java.base/jdk.internal=ALL-UNNAMED", + "-Avalueclasses.outdir=" + outDir, + "--processor-path", System.getProperty("test.classes"), + "-processor", "build.tools.valhalla.valuetypes.GenValueClasses", + srcDir.resolve(srcPath).toString()); + assertEquals(0, exitValue); + } +} \ No newline at end of file From e73f51640cc78463b3cdddcb99f7e915c96b0f20 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Fri, 27 Feb 2026 19:40:49 +0100 Subject: [PATCH 05/17] More tests and tidying --- .../valhalla/valuetypes/GenValueClasses.java | 22 ++++++------ .../valuetypes/GenValueClassesTest.java | 35 ++++++++++++++++--- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java index 1f48aa30575..02b2ed7147c 100644 --- a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java @@ -118,7 +118,7 @@ public boolean process(Set annotations, RoundEnvironment getAnnotation(annotations, "jdk.internal.MigratedValueClass"); if (valueClassAnnotation.isPresent()) { getAnnotatedTypes(env, valueClassAnnotation.get()).stream() - .collect(groupingBy(this::getJavaSourceFile)) + .collect(groupingBy(this::javaSourceFile)) .forEach(this::generateValueClassSource); } // We may not be the only annotation processor to consume this annotation. @@ -159,12 +159,12 @@ private void generateValueClassSource(Path srcPath, List classes) { try { // We know there's at least one element per source file (by construction). TypeElement element = classes.getFirst(); - Path relPath = getModuleRelativePath(srcPath, getPackageName(element)); - Path outPath = outDir.resolve(getModuleName(element)).resolve(relPath); + Path relPath = moduleRelativePath(srcPath, packageName(element)); + Path outPath = outDir.resolve(moduleName(element)).resolve(relPath); Files.createDirectories(outPath.getParent()); List insertPositions = - classes.stream().map(this::getValueKeywordInsertPosition).sorted().toList(); + classes.stream().map(this::valueKeywordInsertPosition).sorted().toList(); // For partial rebuilds, generated sources may still exist, so we overwrite them. try (Reader reader = new InputStreamReader(Files.newInputStream(srcPath)); @@ -197,7 +197,7 @@ private void generateValueClassSource(Path srcPath, List classes) { * the {@code value} keyword. The offset is the end of the modifiers section, * which must immediately precede the class declaration. */ - private long getValueKeywordInsertPosition(TypeElement classElement) { + private long valueKeywordInsertPosition(TypeElement classElement) { TreePath classDecl = trees.getPath(classElement); ClassTree classTree = (ClassTree) classDecl.getLeaf(); CompilationUnitTree compilationUnit = classDecl.getCompilationUnit(); @@ -211,7 +211,7 @@ private long getValueKeywordInsertPosition(TypeElement classElement) { return pos; } - private Path getModuleRelativePath(Path srcPath, String pkgName) { + private Path moduleRelativePath(Path srcPath, String pkgName) { Path relPath = Path.of(pkgName.replace('.', File.separatorChar)).resolve(srcPath.getFileName()); if (!srcPath.endsWith(relPath)) { throw new IllegalStateException(String.format( @@ -220,19 +220,19 @@ private Path getModuleRelativePath(Path srcPath, String pkgName) { return relPath; } - private String getModuleName(TypeElement t) { + private String moduleName(TypeElement t) { return processingEnv.getElementUtils().getModuleOf(t).getQualifiedName().toString(); } - private String getPackageName(TypeElement t) { + private String packageName(TypeElement t) { return processingEnv.getElementUtils().getPackageOf(t).getQualifiedName().toString(); } - private Path getJavaSourceFile(TypeElement type) { - return getFilePath(processingEnv.getElementUtils().getFileObjectOf(type)); + private Path javaSourceFile(TypeElement type) { + return filePath(processingEnv.getElementUtils().getFileObjectOf(type)); } - private static Path getFilePath(FileObject file) { + private static Path filePath(FileObject file) { return Path.of(file.toUri()); } diff --git a/test/jdk/valhalla/valuetypes/GenValueClassesTest.java b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java index d05b5b48f04..63503dd8160 100644 --- a/test/jdk/valhalla/valuetypes/GenValueClassesTest.java +++ b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java @@ -53,7 +53,6 @@ public class GenValueClassesTest { @jdk.internal.MigratedValueClass public /*VALUE*/ class SimpleValueClass { - int value; } """; @@ -64,7 +63,6 @@ public class GenValueClassesTest { public class NestedValueClass { @jdk.internal.MigratedValueClass private static final /*VALUE*/ class Nested { - int value; } } """; @@ -86,6 +84,24 @@ static final class Second { } } """; + // A slightly extreme case to show that the annotation processor can cope + // with multiline class declarations and interleaved comments. The value + // keyword is insert after the last modifier with a leading space. + private static final String MULTILINE_CLASS_DECLARATION = + """ + package test; + + @jdk.internal.MigratedValueClass + public + /* Some comment */ + final /*VALUE*/ + /* Some other comment */ + class + + MultilineClassDeclaration { + } + """; + @TempDir Path testDir = null; Path srcDir = null; @@ -129,11 +145,20 @@ public void multipleValueClasses() throws IOException { assertTrue(transformedSrc.contains(" value class Third ")); } + @Test + public void multilineClassDeclaration() throws IOException { + Path relPath = writeTestSource("MultilineClassDeclaration", MULTILINE_CLASS_DECLARATION); + compileTestClass(relPath); + String transformedSrc = Files.readString(outDir.resolve(relPath)); + assertEquals(MULTILINE_CLASS_DECLARATION.replace("/*VALUE*/", "value"), transformedSrc); + } + private Path writeTestSource(String className, String source) throws IOException { // Remove the VALUE tokens to leave "clean" source (otherwise the // token will persist in the transformed output and get messy). - assertTrue(source.contains("/*VALUE*/ "), "invalid test source"); - String actualSrc = source.replace("/*VALUE*/ ", ""); + // Tokens have a leading space to match how the keyword is injected. + assertTrue(source.contains(" /*VALUE*/"), "invalid test source"); + String actualSrc = source.replace(" /*VALUE*/", ""); Path relPath = Path.of("test", className + ".java"); Path srcFile = srcDir.resolve(relPath); @@ -155,4 +180,4 @@ private void compileTestClass(Path srcPath) { srcDir.resolve(srcPath).toString()); assertEquals(0, exitValue); } -} \ No newline at end of file +} From 9fd0a04e1924ac3f2b8f36aaa1d38441b205c540 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Mon, 2 Mar 2026 13:28:41 +0100 Subject: [PATCH 06/17] rewrite test to allow skipping --- .../valuetypes/GenValueClassesTest.java | 235 +++++++++++------- 1 file changed, 149 insertions(+), 86 deletions(-) diff --git a/test/jdk/valhalla/valuetypes/GenValueClassesTest.java b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java index 63503dd8160..f2599c642b1 100644 --- a/test/jdk/valhalla/valuetypes/GenValueClassesTest.java +++ b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java @@ -23,13 +23,22 @@ * questions. */ +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.io.TempDir; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.spi.ToolProvider; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -38,107 +47,99 @@ /* * @test - * @summary Ensure annotation processing generates expected source files + * @summary Tests build tool GenValueClasses annotation processor * @library /tools/lib - * @compile ../../../../make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java * @run junit GenValueClassesTest */ public class GenValueClassesTest { private static final ToolProvider JAVAC_TOOL = ToolProvider.findFirst("javac") .orElseThrow(() -> new RuntimeException("javac tool not found")); - private static final String SIMPLE_VALUE_CLASS = - """ - package test; + // We cannot access compiled build tools in JTREG tests, but we can find the + // sources and compile them. See findBuildToolsSrcRoot() for more details. + private static final Path SRC_ROOT = Path.of("make", "jdk", "src", "classes"); + private static final Path PROCESSOR_SRC = Path.of( + "build", "tools", "valhalla", "valuetypes", "GenValueClasses.java"); - @jdk.internal.MigratedValueClass - public /*VALUE*/ class SimpleValueClass { - } - """; - - private static final String NESTED_VALUE_CLASS = - """ - package test; - - public class NestedValueClass { - @jdk.internal.MigratedValueClass - private static final /*VALUE*/ class Nested { - } - } - """; - - private static final String MULTIPLE_VALUE_CLASSES = - """ - package test; - - @jdk.internal.MigratedValueClass - public /*VALUE*/ class MultipleValueClasses { - - @jdk.internal.MigratedValueClass - private static final /*VALUE*/ class First { } - - static final class Second { } + // By being on a static member, this is shared across all tests. + @TempDir + private static Path sharedTestDir = null; - @jdk.internal.MigratedValueClass - private static /*VALUE*/ class Third { } - } - """; - - // A slightly extreme case to show that the annotation processor can cope - // with multiline class declarations and interleaved comments. The value - // keyword is insert after the last modifier with a leading space. - private static final String MULTILINE_CLASS_DECLARATION = - """ - package test; - - @jdk.internal.MigratedValueClass - public - /* Some comment */ - final /*VALUE*/ - /* Some other comment */ - class - - MultilineClassDeclaration { - } - """; + @BeforeAll + static void compileAnnotationProcessor() { + compile(findBuildToolsSrcRoot().resolve(PROCESSOR_SRC), sharedTestDir.resolve("processor")); + } - @TempDir - Path testDir = null; - Path srcDir = null; - Path outDir = null; + Path perTestDir = null; @BeforeEach - void setupDirs() throws IOException { - this.srcDir = testDir.resolve("src"); - Files.createDirectories(srcDir); - this.outDir = testDir.resolve("out"); - Files.createDirectories(outDir); + void setupDirs(TestInfo testInfo) throws IOException { + // Since the test directory is shared, we make per-test directories + // based on the test's method name to keep everything separate. + String testName = testInfo.getTestMethod().orElseThrow().getName(); + this.perTestDir = sharedTestDir.resolve(testName); + Files.createDirectories(perTestDir); } @Test public void simpleValueClass() throws IOException { - Path relPath = writeTestSource("SimpleValueClass", SIMPLE_VALUE_CLASS); + String simpleValueClass = + """ + package test; + + @jdk.internal.MigratedValueClass + public /*VALUE*/ class SimpleValueClass { + } + """; + Path relPath = writeTestSource("SimpleValueClass", simpleValueClass); compileTestClass(relPath); - String transformedSrc = Files.readString(outDir.resolve(relPath)); - assertEquals(SIMPLE_VALUE_CLASS.replace("/*VALUE*/", "value"), transformedSrc); + String transformedSrc = readTransformedSource(relPath); + assertEquals(simpleValueClass.replace("/*VALUE*/", "value"), transformedSrc); assertTrue(transformedSrc.contains(" value class SimpleValueClass ")); } @Test public void nestedValueClass() throws IOException { - Path relPath = writeTestSource("NestedValueClass", NESTED_VALUE_CLASS); + String nestedValueClass = + """ + package test; + + public class NestedValueClass { + @jdk.internal.MigratedValueClass + private static final /*VALUE*/ class Nested { + } + } + """; + Path relPath = writeTestSource("NestedValueClass", nestedValueClass); compileTestClass(relPath); - String transformedSrc = Files.readString(outDir.resolve(relPath)); - assertEquals(NESTED_VALUE_CLASS.replace("/*VALUE*/", "value"), transformedSrc); + String transformedSrc = readTransformedSource(relPath); + assertEquals(nestedValueClass.replace("/*VALUE*/", "value"), transformedSrc); assertTrue(transformedSrc.contains(" value class Nested ")); } @Test public void multipleValueClasses() throws IOException { - Path relPath = writeTestSource("MultipleValueClasses", MULTIPLE_VALUE_CLASSES); + String multipleValueClasses = + """ + package test; + + @jdk.internal.MigratedValueClass + public /*VALUE*/ class MultipleValueClasses { + + @jdk.internal.MigratedValueClass + private static final /*VALUE*/ class First { } + + static final class Second { } + + @jdk.internal.MigratedValueClass + private static /*VALUE*/ class Third { } + } + """; + + Path relPath = writeTestSource("MultipleValueClasses", multipleValueClasses); compileTestClass(relPath); - String transformedSrc = Files.readString(outDir.resolve(relPath)); - assertEquals(MULTIPLE_VALUE_CLASSES.replace("/*VALUE*/", "value"), transformedSrc); + String transformedSrc = readTransformedSource(relPath); + assertEquals(multipleValueClasses.replace("/*VALUE*/", "value"), transformedSrc); assertTrue(transformedSrc.contains(" value class MultipleValueClasses ")); assertTrue(transformedSrc.contains(" value class First ")); assertFalse(transformedSrc.contains(" value class Second ")); @@ -147,10 +148,27 @@ public void multipleValueClasses() throws IOException { @Test public void multilineClassDeclaration() throws IOException { - Path relPath = writeTestSource("MultilineClassDeclaration", MULTILINE_CLASS_DECLARATION); + // A slightly extreme case to show that the annotation processor can cope + // with multiline class declarations and interleaved comments. The value + // keyword is inserted after the last modifier with a leading space. + String multilineClassDeclaration = + """ + package test; + + @jdk.internal.MigratedValueClass + public + /* Some comment */ + final /*VALUE*/ + /* Some other comment */ + class + + MultilineClassDeclaration { + } + """; + Path relPath = writeTestSource("MultilineClassDeclaration", multilineClassDeclaration); compileTestClass(relPath); - String transformedSrc = Files.readString(outDir.resolve(relPath)); - assertEquals(MULTILINE_CLASS_DECLARATION.replace("/*VALUE*/", "value"), transformedSrc); + String transformedSrc = readTransformedSource(relPath); + assertEquals(multilineClassDeclaration.replace("/*VALUE*/", "value"), transformedSrc); } private Path writeTestSource(String className, String source) throws IOException { @@ -161,23 +179,68 @@ private Path writeTestSource(String className, String source) throws IOException String actualSrc = source.replace(" /*VALUE*/", ""); Path relPath = Path.of("test", className + ".java"); - Path srcFile = srcDir.resolve(relPath); + Path srcFile = perTestDir.resolve("src").resolve(relPath); Files.createDirectories(srcFile.getParent()); Files.writeString(srcFile, actualSrc); return relPath; } - // NOTE: Since the in-memory compiler is limited and doesn't support getting - // the Path of the "memo:" URIs is uses, it cannot be used for testing this - // annotation processor. Thus, we must perform on-disk compilation. + private String readTransformedSource(Path relPath) throws IOException { + return Files.readString(perTestDir.resolve("out").resolve(relPath)); + } + private void compileTestClass(Path srcPath) { - int exitValue = JAVAC_TOOL.run(System.out, System.err, - "-d", testDir.resolve("compiled").toString(), + compile(perTestDir.resolve("src").resolve(srcPath), sharedTestDir.resolve("compiled"), "--add-exports", "java.base/jdk.internal=ALL-UNNAMED", - "-Avalueclasses.outdir=" + outDir, - "--processor-path", System.getProperty("test.classes"), - "-processor", "build.tools.valhalla.valuetypes.GenValueClasses", - srcDir.resolve(srcPath).toString()); - assertEquals(0, exitValue); + "-Avalueclasses.outdir=" + perTestDir.resolve("out"), + "--processor-path", sharedTestDir.resolve("processor").toString(), + "-processor", "build.tools.valhalla.valuetypes.GenValueClasses"); + } + + // NOTE: Since the in-memory compiler is limited and doesn't support getting + // the Path of the "memo:" URIs is uses, it cannot be used for testing this + // annotation processor, so we must perform on-disk compilation. + private static void compile(Path srcFile, Path outDir, String... extraArgs) { + StringWriter out = new StringWriter(); + StringWriter err = new StringWriter(); + List allArgs = new ArrayList<>(); + allArgs.add("-d"); + allArgs.add(outDir.toString()); + allArgs.addAll(Arrays.asList(extraArgs)); + allArgs.add(srcFile.toString()); + int exitValue = JAVAC_TOOL.run( + new PrintWriter(out), + new PrintWriter(err), + allArgs.toArray(String[]::new)); + Assertions.assertEquals(0, exitValue, String.format( + """ + Compilation failed: %s + Stdout: %s + Stderr: %s + """, srcFile, out, err)); + } + + /** + * The source root is {@code make/jdk/src/classes} from the JDK root, but + * this may not be available in all test environments (and if it isn't, the + * test should be skipped). + * + *

    Similar to {@code test/langtools/tools/all/RunCodingRules.java}, we + * attempt to locate this directory by walking from the {@code test.src} + * directory. + */ + private static Path findBuildToolsSrcRoot() { + Path testSrc = Path.of(System.getProperty("test.src", ".")); + for (Path d = testSrc; d != null; d = d.getParent()) { + if (Files.exists(d.resolve("TEST.ROOT"))) { + d = d.getParent(); + Path srcRoot = d.resolve(SRC_ROOT); + if (!Files.isDirectory(srcRoot)) { + break; + } + return srcRoot; + } + } + return Assumptions.abort("Build tools source root not found; skipping test"); } } From 10c3b2847aa6cd9e63e939d574a0dcecb0820e94 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Mon, 2 Mar 2026 13:41:26 +0100 Subject: [PATCH 07/17] feedback --- .../build/tools/valhalla/valuetypes/GenValueClasses.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java index 02b2ed7147c..4a95658c0ec 100644 --- a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java @@ -78,9 +78,11 @@ * abstract value classes. *

*/ -@SupportedAnnotationTypes({"jdk.internal.MigratedValueClass"}) +@SupportedAnnotationTypes(GenValueClasses.MIGRATED_VALUE_CLASS_ANNOTATION) @SupportedOptions("valueclasses.outdir") public final class GenValueClasses extends AbstractProcessor { + static final String MIGRATED_VALUE_CLASS_ANNOTATION = "jdk.internal.MigratedValueClass"; + // Matches preprocessor option flag in CompileJavaModules.gmk. private static final String OUTDIR_OPTION_KEY = "valueclasses.outdir"; @@ -115,7 +117,7 @@ public SourceVersion getSupportedSourceVersion() { public boolean process(Set annotations, RoundEnvironment env) { // We don't have direct access to MigratedValueClass classes here. Optional valueClassAnnotation = - getAnnotation(annotations, "jdk.internal.MigratedValueClass"); + getAnnotation(annotations, MIGRATED_VALUE_CLASS_ANNOTATION); if (valueClassAnnotation.isPresent()) { getAnnotatedTypes(env, valueClassAnnotation.get()).stream() .collect(groupingBy(this::javaSourceFile)) From a2d1dd816bee168e7d4278bc32d9fd280fdb65a2 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Mon, 2 Mar 2026 14:34:18 +0100 Subject: [PATCH 08/17] better error handling --- .../valhalla/valuetypes/GenValueClasses.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java index 4a95658c0ec..00e5faf6070 100644 --- a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java @@ -95,13 +95,14 @@ public synchronized void init(ProcessingEnvironment processingEnv) { super.init(processingEnv); this.processingEnv = processingEnv; String outDir = this.processingEnv.getOptions().get(OUTDIR_OPTION_KEY); - if (outDir == null) { - throw new IllegalStateException( - "Must specify -A" + OUTDIR_OPTION_KEY + "=" - + " for annotation processor: " + GenValueClasses.class.getName()); + if (outDir != null) { + // No need to convert '/' for Windows in build tools. + this.outDir = Path.of(outDir); + this.trees = Trees.instance(this.processingEnv); + } else { + processingEnv.getMessager().printError( + "Must specify -A" + OUTDIR_OPTION_KEY + "="); } - this.outDir = Path.of(outDir.replace('/', File.separatorChar)); - this.trees = Trees.instance(this.processingEnv); } /** @@ -115,13 +116,16 @@ public SourceVersion getSupportedSourceVersion() { @Override public boolean process(Set annotations, RoundEnvironment env) { - // We don't have direct access to MigratedValueClass classes here. - Optional valueClassAnnotation = - getAnnotation(annotations, MIGRATED_VALUE_CLASS_ANNOTATION); - if (valueClassAnnotation.isPresent()) { - getAnnotatedTypes(env, valueClassAnnotation.get()).stream() - .collect(groupingBy(this::javaSourceFile)) - .forEach(this::generateValueClassSource); + // Don't do anything if there was an error in init(). + if (outDir != null) { + // We don't have direct access to MigratedValueClass classes here. + Optional valueClassAnnotation = + getAnnotation(annotations, MIGRATED_VALUE_CLASS_ANNOTATION); + if (valueClassAnnotation.isPresent()) { + getAnnotatedTypes(env, valueClassAnnotation.get()).stream() + .collect(groupingBy(this::javaSourceFile)) + .forEach(this::generateValueClassSource); + } } // We may not be the only annotation processor to consume this annotation. return false; From e27349ee34552d828201f274e3cc8662be355d20 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Mon, 2 Mar 2026 15:04:03 +0100 Subject: [PATCH 09/17] better error handling --- .../valhalla/valuetypes/GenValueClasses.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java index 00e5faf6070..a00857bb2af 100644 --- a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java @@ -132,24 +132,25 @@ public boolean process(Set annotations, RoundEnvironment } /** Find the annotation element by name in the given set. */ - private static Optional getAnnotation(Set annotations, String name) { + private static Optional getAnnotation( + Set annotations, String name) { return annotations.stream() .filter(e -> e.getQualifiedName().toString().equals(name)) .findFirst(); } /** Find the type elements (classes) annotated with the given annotation element. */ - private static Set getAnnotatedTypes(RoundEnvironment env, TypeElement annotation) { + private Set getAnnotatedTypes(RoundEnvironment env, TypeElement annotation) { Set types = new HashSet<>(); for (Element e : env.getElementsAnnotatedWith(annotation)) { if (!e.getKind().isClass()) { - throw new IllegalStateException( - "Unexpected element kind (" + e.getKind() + ") for element: " + e); + processingEnv.getMessager().printError("Unexpected element kind (" + e.getKind() + ")", e); + continue; } TypeElement type = (TypeElement) e; if (type.getQualifiedName().isEmpty()) { - throw new IllegalStateException( - "Unexpected empty name for element: " + e); + processingEnv.getMessager().printError("Unexpected empty name", e); + continue; } types.add(type); } @@ -162,11 +163,11 @@ private static Set getAnnotatedTypes(RoundEnvironment env, TypeElem * annotated type element. */ private void generateValueClassSource(Path srcPath, List classes) { + // We know there's at least one element per source file (by construction). + TypeElement classElement = classes.getFirst(); + Path relPath = moduleRelativePath(srcPath, packageName(classElement)); + Path outPath = outDir.resolve(moduleName(classElement)).resolve(relPath); try { - // We know there's at least one element per source file (by construction). - TypeElement element = classes.getFirst(); - Path relPath = moduleRelativePath(srcPath, packageName(element)); - Path outPath = outDir.resolve(moduleName(element)).resolve(relPath); Files.createDirectories(outPath.getParent()); List insertPositions = @@ -194,6 +195,7 @@ private void generateValueClassSource(Path srcPath, List classes) { reader.transferTo(output); } } catch (IOException e) { + processingEnv.getMessager().printError("Failed to write value class source: " + outPath); throw new RuntimeException(e); } } @@ -226,12 +228,12 @@ private Path moduleRelativePath(Path srcPath, String pkgName) { return relPath; } - private String moduleName(TypeElement t) { - return processingEnv.getElementUtils().getModuleOf(t).getQualifiedName().toString(); + private String moduleName(TypeElement type) { + return processingEnv.getElementUtils().getModuleOf(type).getQualifiedName().toString(); } - private String packageName(TypeElement t) { - return processingEnv.getElementUtils().getPackageOf(t).getQualifiedName().toString(); + private String packageName(TypeElement type) { + return processingEnv.getElementUtils().getPackageOf(type).getQualifiedName().toString(); } private Path javaSourceFile(TypeElement type) { From 60cf8918d19b30a1901c5cda18e2cd42efe9fe83 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Mon, 2 Mar 2026 18:20:14 +0100 Subject: [PATCH 10/17] Simpler temp directory --- .../valuetypes/GenValueClassesTest.java | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/test/jdk/valhalla/valuetypes/GenValueClassesTest.java b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java index f2599c642b1..4e098b15391 100644 --- a/test/jdk/valhalla/valuetypes/GenValueClassesTest.java +++ b/test/jdk/valhalla/valuetypes/GenValueClassesTest.java @@ -26,9 +26,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.io.TempDir; import java.io.IOException; @@ -61,25 +59,17 @@ public class GenValueClassesTest { private static final Path PROCESSOR_SRC = Path.of( "build", "tools", "valhalla", "valuetypes", "GenValueClasses.java"); - // By being on a static member, this is shared across all tests. + // Compile the annotation processor once for all test cases. @TempDir - private static Path sharedTestDir = null; + private static Path processorDir = null; @BeforeAll static void compileAnnotationProcessor() { - compile(findBuildToolsSrcRoot().resolve(PROCESSOR_SRC), sharedTestDir.resolve("processor")); + compile(findBuildToolsSrcRoot().resolve(PROCESSOR_SRC), processorDir); } - Path perTestDir = null; - - @BeforeEach - void setupDirs(TestInfo testInfo) throws IOException { - // Since the test directory is shared, we make per-test directories - // based on the test's method name to keep everything separate. - String testName = testInfo.getTestMethod().orElseThrow().getName(); - this.perTestDir = sharedTestDir.resolve(testName); - Files.createDirectories(perTestDir); - } + @TempDir + Path testDir = null; @Test public void simpleValueClass() throws IOException { @@ -179,21 +169,21 @@ private Path writeTestSource(String className, String source) throws IOException String actualSrc = source.replace(" /*VALUE*/", ""); Path relPath = Path.of("test", className + ".java"); - Path srcFile = perTestDir.resolve("src").resolve(relPath); + Path srcFile = testDir.resolve("src").resolve(relPath); Files.createDirectories(srcFile.getParent()); Files.writeString(srcFile, actualSrc); return relPath; } private String readTransformedSource(Path relPath) throws IOException { - return Files.readString(perTestDir.resolve("out").resolve(relPath)); + return Files.readString(testDir.resolve("out").resolve(relPath)); } private void compileTestClass(Path srcPath) { - compile(perTestDir.resolve("src").resolve(srcPath), sharedTestDir.resolve("compiled"), + compile(testDir.resolve("src").resolve(srcPath), processorDir.resolve("compiled"), "--add-exports", "java.base/jdk.internal=ALL-UNNAMED", - "-Avalueclasses.outdir=" + perTestDir.resolve("out"), - "--processor-path", sharedTestDir.resolve("processor").toString(), + "-Avalueclasses.outdir=" + testDir.resolve("out"), + "--processor-path", processorDir.toString(), "-processor", "build.tools.valhalla.valuetypes.GenValueClasses"); } From 2aeca48d4b645ce259721e2221448e29d2878174 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Tue, 3 Mar 2026 11:27:16 +0100 Subject: [PATCH 11/17] better but still not working --- make/CompileJavaModules.gmk | 8 ++++++-- make/CompileToolsJdk.gmk | 13 ++++++++++++- make/ToolsJdk.gmk | 8 ++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk index 0eaa0d089f2..1da369162b1 100644 --- a/make/CompileJavaModules.gmk +++ b/make/CompileJavaModules.gmk @@ -29,6 +29,7 @@ include MakeFileStart.gmk include JavaCompilation.gmk include Modules.gmk +include ToolsJdk.gmk include CopyFiles.gmk @@ -77,9 +78,10 @@ ifeq ($(MODULE), java.base) PREPROCESSOR_FLAGS := \ -Xlint:-removal -Xlint:-processing \ -Avalueclasses.outdir=$(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \ - -processor build.tools.valhalla.valuetypes.GenValueClasses + -processor $(VALUETYPE_GENSRC_PROCESSOR_NAME) - PROCESSOR_PATH += $(BUILDTOOLS_OUTPUTDIR)/jdk_tools_classes + PROCESSOR_PATH += $(VALUETYPE_GENSRC_PROCESSOR_PATH) + EXTRA_DEPS += $(VALUETYPE_GENSRC_PROCESSOR_DEP) endif ################################################################################ @@ -121,6 +123,7 @@ $(eval $(call SetupJavaCompilation, $(MODULE), \ MODULE := $(MODULE), \ SRC := $(wildcard $(MODULE_SRC_DIRS)), \ INCLUDES := $(JDK_USER_DEFINED_FILTER), \ + EXTRA_DEPS := $(EXTRA_DEPS), \ FAIL_NO_SRC := $(FAIL_NO_SRC), \ BIN := $(COMPILATION_OUTPUTDIR), \ HEADERS := $(SUPPORT_OUTPUTDIR)/headers, \ @@ -170,6 +173,7 @@ ifneq ($(COMPILER), bootjdk) MODULE := $(MODULE), \ SRC := $(wildcard $(MODULE_PREVIEW_SRC_DIRS)), \ INCLUDES := $(JDK_USER_DEFINED_FILTER), \ + EXTRA_DEPS := $(VALUETYPE_GENSRC_PROCESSOR_DEP), \ FAIL_NO_SRC := $(FAIL_NO_SRC), \ BIN := $(TEMP_OUTPUTDIR)/, \ HEADERS := $(SUPPORT_OUTPUTDIR)/headers, \ diff --git a/make/CompileToolsJdk.gmk b/make/CompileToolsJdk.gmk index c291dbdba0a..a5bb701c169 100644 --- a/make/CompileToolsJdk.gmk +++ b/make/CompileToolsJdk.gmk @@ -45,7 +45,8 @@ $(eval $(call SetupJavaCompilation, BUILD_TOOLS_JDK, \ build/tools/deps \ build/tools/docs \ build/tools/jigsaw \ - build/tools/depend, \ + build/tools/depend \ + build/tools/valhalla, \ BIN := $(BUILDTOOLS_OUTPUTDIR)/jdk_tools_classes, \ DISABLED_WARNINGS := dangling-doc-comments options, \ JAVAC_FLAGS := \ @@ -155,4 +156,14 @@ endif ################################################################################ +$(eval $(call SetupJavaCompilation, COMPILE_VALUETYPE_GENSRC, \ + TARGET_RELEASE := $(TARGET_RELEASE_BOOTJDK), \ + SRC := $(TOPDIR)/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java, \ + BIN := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc, \ +)) + +TARGETS += $(COMPILE_VALUETYPE_GENSRC) + +################################################################################ + include MakeFileEnd.gmk diff --git a/make/ToolsJdk.gmk b/make/ToolsJdk.gmk index b04d7820c91..cf33115c03e 100644 --- a/make/ToolsJdk.gmk +++ b/make/ToolsJdk.gmk @@ -142,5 +142,13 @@ PANDOC_HTML_MANPAGE_FILTER := $(BUILDTOOLS_OUTPUTDIR)/manpages/pandoc-html-manpa ################################################################################ +# Annotation processor for generating preview sources of annotated value classes. + +VALUETYPE_GENSRC_PROCESSOR_NAME := build.tools.valhalla.valuetypes.GenValueClasses +VALUETYPE_GENSRC_PROCESSOR_PATH := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc +VALUETYPE_GENSRC_PROCESSOR_DEP := $(VALUETYPE_GENSRC_PROCESSOR_PATH)/_the.COMPILE_VALUETYPE_GENSRC_batch + +################################################################################ + endif # include guard include MakeIncludeEnd.gmk From d0cc4464f14147056819955ac4a083c3415eaa84 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Tue, 3 Mar 2026 13:15:22 +0100 Subject: [PATCH 12/17] better compile macro call - still not working --- make/CompileToolsJdk.gmk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/make/CompileToolsJdk.gmk b/make/CompileToolsJdk.gmk index a5bb701c169..167f52b2db1 100644 --- a/make/CompileToolsJdk.gmk +++ b/make/CompileToolsJdk.gmk @@ -158,7 +158,8 @@ endif $(eval $(call SetupJavaCompilation, COMPILE_VALUETYPE_GENSRC, \ TARGET_RELEASE := $(TARGET_RELEASE_BOOTJDK), \ - SRC := $(TOPDIR)/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java, \ + SRC := $(TOPDIR)/make/jdk/src/classes, \ + INCLUDES := build/tools/valhalla/valuetypes, \ BIN := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc, \ )) From f12c7127cb3e900770d80ffeee63adc82fac564d Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Fri, 6 Mar 2026 15:07:11 +0100 Subject: [PATCH 13/17] feedback updates and tidying --- make/CompileJavaModules.gmk | 6 +++--- make/ToolsJdk.gmk | 1 - make/common/JavaCompilation.gmk | 5 ++++- .../build/tools/valhalla/valuetypes/GenValueClasses.java | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk index 1da369162b1..7e60b7051d8 100644 --- a/make/CompileJavaModules.gmk +++ b/make/CompileJavaModules.gmk @@ -73,6 +73,7 @@ MODULEPATH := $(call PathList, $(IMPORT_MODULES_CLASSES)) # Setup preprocessor flags # The output directory must be present in GENERATED_PREVIEW_SUBDIRS in Modules.gmk. # Temporarily restrict this to java.base, but it can be expanded later. +# TODO: Remove Xlint directives below once the fix in JDK-8378740 is merged into lworld. ifeq ($(MODULE), java.base) PREPROCESSOR_FLAGS := \ @@ -81,7 +82,7 @@ ifeq ($(MODULE), java.base) -processor $(VALUETYPE_GENSRC_PROCESSOR_NAME) PROCESSOR_PATH += $(VALUETYPE_GENSRC_PROCESSOR_PATH) - EXTRA_DEPS += $(VALUETYPE_GENSRC_PROCESSOR_DEP) + DEPENDS += $(VALUETYPE_GENSRC_PROCESSOR_PATH) endif ################################################################################ @@ -123,7 +124,7 @@ $(eval $(call SetupJavaCompilation, $(MODULE), \ MODULE := $(MODULE), \ SRC := $(wildcard $(MODULE_SRC_DIRS)), \ INCLUDES := $(JDK_USER_DEFINED_FILTER), \ - EXTRA_DEPS := $(EXTRA_DEPS), \ + DEPENDS := $(DEPENDS), \ FAIL_NO_SRC := $(FAIL_NO_SRC), \ BIN := $(COMPILATION_OUTPUTDIR), \ HEADERS := $(SUPPORT_OUTPUTDIR)/headers, \ @@ -173,7 +174,6 @@ ifneq ($(COMPILER), bootjdk) MODULE := $(MODULE), \ SRC := $(wildcard $(MODULE_PREVIEW_SRC_DIRS)), \ INCLUDES := $(JDK_USER_DEFINED_FILTER), \ - EXTRA_DEPS := $(VALUETYPE_GENSRC_PROCESSOR_DEP), \ FAIL_NO_SRC := $(FAIL_NO_SRC), \ BIN := $(TEMP_OUTPUTDIR)/, \ HEADERS := $(SUPPORT_OUTPUTDIR)/headers, \ diff --git a/make/ToolsJdk.gmk b/make/ToolsJdk.gmk index cf33115c03e..d7a4088b978 100644 --- a/make/ToolsJdk.gmk +++ b/make/ToolsJdk.gmk @@ -146,7 +146,6 @@ PANDOC_HTML_MANPAGE_FILTER := $(BUILDTOOLS_OUTPUTDIR)/manpages/pandoc-html-manpa VALUETYPE_GENSRC_PROCESSOR_NAME := build.tools.valhalla.valuetypes.GenValueClasses VALUETYPE_GENSRC_PROCESSOR_PATH := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc -VALUETYPE_GENSRC_PROCESSOR_DEP := $(VALUETYPE_GENSRC_PROCESSOR_PATH)/_the.COMPILE_VALUETYPE_GENSRC_batch ################################################################################ diff --git a/make/common/JavaCompilation.gmk b/make/common/JavaCompilation.gmk index bde59a2904a..6587810c845 100644 --- a/make/common/JavaCompilation.gmk +++ b/make/common/JavaCompilation.gmk @@ -161,7 +161,10 @@ endef # EXTRA_FILES List of extra source files to include in compilation. Can be used to # specify files that need to be generated by other rules first. # HEADERS path to directory where all generated c-headers are written. -# DEPENDS Extra dependency +# DEPENDS Extra dependencies +# PROCESSOR_PATH Annotation processor and plugin path (see --processor-path). +# Needed when specifying annotation processors not found via serivce discovery, +# and required for plugins when they are used alongside annotation processors. # KEEP_DUPS Do not remove duplicate file names from different source roots. # FAIL_NO_SRC Set to false to not fail the build if no source files are found, # default is true. diff --git a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java index a00857bb2af..4d60d4c57d5 100644 --- a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java @@ -189,7 +189,7 @@ private void generateValueClassSource(Path srcPath, List classes) { // curPos is at the end of the modifier section, so add a leading space. // curPos ---v // [modifiers] class... -->> [modifiers] value class... - output.write(" value"); + output.write(" value /*qq*/ "); } // Trailing section to end-of-file transferred from original reader. reader.transferTo(output); From 18a5e21183c669fd7a43524882aae6f0730b3eb3 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Tue, 10 Mar 2026 16:29:48 +0100 Subject: [PATCH 14/17] Finally working --- make/CompileJavaModules.gmk | 2 +- make/CompileToolsJdk.gmk | 4 ++-- make/ToolsJdk.gmk | 4 ++++ make/common/JavaCompilation.gmk | 7 ++++--- .../build/tools/valhalla/valuetypes/GenValueClasses.java | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk index 7e60b7051d8..698664ba58e 100644 --- a/make/CompileJavaModules.gmk +++ b/make/CompileJavaModules.gmk @@ -82,7 +82,7 @@ ifeq ($(MODULE), java.base) -processor $(VALUETYPE_GENSRC_PROCESSOR_NAME) PROCESSOR_PATH += $(VALUETYPE_GENSRC_PROCESSOR_PATH) - DEPENDS += $(VALUETYPE_GENSRC_PROCESSOR_PATH) + DEPENDS += $(BUILD_VALUETYPE_GENSRC) endif ################################################################################ diff --git a/make/CompileToolsJdk.gmk b/make/CompileToolsJdk.gmk index 167f52b2db1..240dc57d93d 100644 --- a/make/CompileToolsJdk.gmk +++ b/make/CompileToolsJdk.gmk @@ -156,14 +156,14 @@ endif ################################################################################ -$(eval $(call SetupJavaCompilation, COMPILE_VALUETYPE_GENSRC, \ +$(eval $(call SetupJavaCompilation, BUILD_VALUETYPE_GENSRC, \ TARGET_RELEASE := $(TARGET_RELEASE_BOOTJDK), \ SRC := $(TOPDIR)/make/jdk/src/classes, \ INCLUDES := build/tools/valhalla/valuetypes, \ BIN := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc, \ )) -TARGETS += $(COMPILE_VALUETYPE_GENSRC) +TARGETS += $(BUILD_VALUETYPE_GENSRC) ################################################################################ diff --git a/make/ToolsJdk.gmk b/make/ToolsJdk.gmk index d7a4088b978..07c066d561f 100644 --- a/make/ToolsJdk.gmk +++ b/make/ToolsJdk.gmk @@ -147,6 +147,10 @@ PANDOC_HTML_MANPAGE_FILTER := $(BUILDTOOLS_OUTPUTDIR)/manpages/pandoc-html-manpa VALUETYPE_GENSRC_PROCESSOR_NAME := build.tools.valhalla.valuetypes.GenValueClasses VALUETYPE_GENSRC_PROCESSOR_PATH := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc +# Same trick as BUILD_TOOLS_JDK but for the annotation processor. +BUILD_VALUETYPE_GENSRC := $(call SetupJavaCompilationCompileTarget, \ + BUILD_VALUETYPE_GENSRC, $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc) + ################################################################################ endif # include guard diff --git a/make/common/JavaCompilation.gmk b/make/common/JavaCompilation.gmk index 6587810c845..f8e4434cbcb 100644 --- a/make/common/JavaCompilation.gmk +++ b/make/common/JavaCompilation.gmk @@ -291,6 +291,7 @@ define SetupJavaCompilationBody endif $1_AUGMENTED_CLASSPATH := $$($1_CLASSPATH) + $1_AUGMENTED_PROCESSOR_PATH := $$($1_PROCESSOR_PATH) $1_API_TARGET := $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1_pubapi $1_API_INTERNAL := $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1_internalapi @@ -306,15 +307,15 @@ define SetupJavaCompilationBody # compilations in unnamed module can refer to other classes from the same # source root, which are not being recompiled in this compilation: $1_AUGMENTED_CLASSPATH += $$($1_BIN) - $1_PROCESSOR_PATH += $$(BUILDTOOLS_OUTPUTDIR)/depend + $1_AUGMENTED_PROCESSOR_PATH += $$(BUILDTOOLS_OUTPUTDIR)/depend endif ifneq ($$($1_AUGMENTED_CLASSPATH), ) $1_FLAGS += -cp $$(call PathList, $$($1_AUGMENTED_CLASSPATH)) endif - ifneq ($$($1_PROCESSOR_PATH), ) - $1_FLAGS += --processor-path $$(call PathList, $$($1_PROCESSOR_PATH)) + ifneq ($$($1_AUGMENTED_PROCESSOR_PATH), ) + $1_FLAGS += --processor-path $$(call PathList, $$($1_AUGMENTED_PROCESSOR_PATH)) endif # Make sure the dirs exist, or that one of the EXTRA_FILES, that may not diff --git a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java index 4d60d4c57d5..a00857bb2af 100644 --- a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java @@ -189,7 +189,7 @@ private void generateValueClassSource(Path srcPath, List classes) { // curPos is at the end of the modifier section, so add a leading space. // curPos ---v // [modifiers] class... -->> [modifiers] value class... - output.write(" value /*qq*/ "); + output.write(" value"); } // Trailing section to end-of-file transferred from original reader. reader.transferTo(output); From 1423745491b50688d382f4860e8e0102122cb59c Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Thu, 12 Mar 2026 17:13:26 +0100 Subject: [PATCH 15/17] Remove unwanted lint warning suppression --- make/CompileJavaModules.gmk | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk index 698664ba58e..61de5d9f3bb 100644 --- a/make/CompileJavaModules.gmk +++ b/make/CompileJavaModules.gmk @@ -73,11 +73,10 @@ MODULEPATH := $(call PathList, $(IMPORT_MODULES_CLASSES)) # Setup preprocessor flags # The output directory must be present in GENERATED_PREVIEW_SUBDIRS in Modules.gmk. # Temporarily restrict this to java.base, but it can be expanded later. -# TODO: Remove Xlint directives below once the fix in JDK-8378740 is merged into lworld. ifeq ($(MODULE), java.base) PREPROCESSOR_FLAGS := \ - -Xlint:-removal -Xlint:-processing \ + -Xlint:-processing \ -Avalueclasses.outdir=$(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \ -processor $(VALUETYPE_GENSRC_PROCESSOR_NAME) From 75aa3b009281a4719ab7496cde2bc734ecb7c164 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Fri, 27 Mar 2026 13:57:46 +0100 Subject: [PATCH 16/17] Temporary plugin work --- make/CompileJavaModules.gmk | 4 +- make/CompileToolsJdk.gmk | 13 +- .../valuetypes/GenValueClassPlugin.java | 225 ++++++++++++++++++ 3 files changed, 238 insertions(+), 4 deletions(-) create mode 100644 make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClassPlugin.java diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk index 61de5d9f3bb..b585fb8d5db 100644 --- a/make/CompileJavaModules.gmk +++ b/make/CompileJavaModules.gmk @@ -76,9 +76,7 @@ MODULEPATH := $(call PathList, $(IMPORT_MODULES_CLASSES)) ifeq ($(MODULE), java.base) PREPROCESSOR_FLAGS := \ - -Xlint:-processing \ - -Avalueclasses.outdir=$(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \ - -processor $(VALUETYPE_GENSRC_PROCESSOR_NAME) + -Xplugin:"GenValueClassPlugin $(SUPPORT_OUTPUTDIR)/gensrc-valueclasses" PROCESSOR_PATH += $(VALUETYPE_GENSRC_PROCESSOR_PATH) DEPENDS += $(BUILD_VALUETYPE_GENSRC) diff --git a/make/CompileToolsJdk.gmk b/make/CompileToolsJdk.gmk index 240dc57d93d..3034718b632 100644 --- a/make/CompileToolsJdk.gmk +++ b/make/CompileToolsJdk.gmk @@ -161,9 +161,20 @@ $(eval $(call SetupJavaCompilation, BUILD_VALUETYPE_GENSRC, \ SRC := $(TOPDIR)/make/jdk/src/classes, \ INCLUDES := build/tools/valhalla/valuetypes, \ BIN := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc, \ + DISABLED_WARNINGS := options, \ + JAVAC_FLAGS := \ + --add-exports jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED \ + --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \ + --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED \ )) -TARGETS += $(BUILD_VALUETYPE_GENSRC) +VALUETYPE_GENSRC_SERVICE_PROVIDER := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc/META-INF/services/com.sun.source.util.Plugin + +$(VALUETYPE_GENSRC_SERVICE_PROVIDER): + $(call MakeDir, $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc/META-INF/services) + $(ECHO) build.tools.depend.Depend > $@ + +TARGETS += $(BUILD_VALUETYPE_GENSRC) $(VALUETYPE_GENSRC_SERVICE_PROVIDER) ################################################################################ diff --git a/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClassPlugin.java b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClassPlugin.java new file mode 100644 index 00000000000..7346dbbc794 --- /dev/null +++ b/make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClassPlugin.java @@ -0,0 +1,225 @@ +package build.tools.valhalla.valuetypes; + +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.util.JavacTask; +import com.sun.source.util.Plugin; +import com.sun.source.util.TaskEvent; +import com.sun.source.util.TaskListener; +import com.sun.tools.javac.tree.JCTree; +import com.sun.tools.javac.tree.JCTree.JCClassDecl; +import com.sun.tools.javac.tree.TreeInfo; +import com.sun.tools.javac.tree.TreeScanner; + +import javax.tools.Diagnostic; +import javax.tools.FileObject; +import java.io.File; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStreamWriter; +import java.io.Reader; +import java.io.Writer; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +import static java.nio.file.StandardOpenOption.CREATE; +import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; + +/** + * Plugin for generating preview sources of classes annotated as value classes + * for preview mode. + * + *

Classes seen by this plugin (annotated with {@code @MigratedValueClass} + * will have their source files re-written into the specified output directory + * for compilation as preview classes. Note that more than one class in a given + * source file may be annotated. + * + *

Class re-writing is achieved by injecting the "value" keyword in front of + * class declarations for all annotated elements in the original source file. + * + *

Note that there are two annotations in use for value classes, but since + * we must generate sources for abstract classes, we only process one of them. + *

    + *
  • {@code @jdk.internal.ValueBased} appears on concrete value classes. + *
  • {@code @jdk.internal.MigratedValueClass} appears on concrete and + * abstract value classes. + *
+ */ +public final class GenValueClassPlugin implements Plugin { + private static final String VALUE_CLASS_ANNOTATION = "@jdk.internal.MigratedValueClass"; + + @Override + public String getName() { + return "GenValueClassPlugin"; + } + + @Override + public void init(JavacTask task, String... args) { + if (args.length == 0) { + throw new IllegalArgumentException("Plugin " + getName() + ": missing output directory argument"); + } + Path outDir = Path.of(args[0]); + if (!Files.isDirectory(outDir)) { + throw new IllegalArgumentException("Plugin " + getName() + ": no such output directory: " + outDir); + } + task.addTaskListener(new ValueClassGenerator(outDir)); + } + + private record ValueClassGenerator(Path outDir) implements TaskListener { + @Override + public void finished(TaskEvent e) { + CompilationUnitTree compilation = e.getCompilationUnit(); + + List classes = new ArrayList<>(); + new TreeScanner() { + @Override + public void visitClassDef(JCClassDecl cls) { + boolean hasAnnotation = cls.getModifiers().getAnnotations().stream() + .peek(a -> System.out.println("--> " + a)) + .anyMatch(a -> a.toString().equals(VALUE_CLASS_ANNOTATION)); + if (hasAnnotation) { + classes.add(cls); + } + super.visitClassDef(cls); + } + }.scan((JCTree) compilation); + + if (!classes.isEmpty()) { + Path srcPath = filePath(compilation.getSourceFile()); + String moduleName = compilation.getModule().getName().toString(); + String packageName = compilation.getPackage().toString(); + generateValueClassSource(srcPath, moduleName, packageName, classes); + } + } + + /** + * Write a transformed version of the given Java source file with the + * {@code value} keyword inserted before the class declaration of each + * annotated type element. + */ + private void generateValueClassSource( + Path srcPath, String moduleName, String packageName, List classes) { + + System.out.println("Module: " + moduleName); + System.out.println("Package: " + packageName); + System.out.println("Classes: " + classes.stream().map(c -> c.type.toString()).toList()); + + Path relPath = moduleRelativePath(srcPath, packageName); + Path outPath = outDir.resolve(moduleName).resolve(relPath); + + System.out.println("Out path: " + outPath); + + try { + Files.createDirectories(outPath.getParent()); + + List insertPositions = + classes.stream().map(this::valueKeywordInsertPosition).sorted().toList(); + + System.out.println("Insert positions: " + insertPositions); + + // For partial rebuilds, generated sources may still exist, so we overwrite them. + try (Reader reader = new InputStreamReader(Files.newInputStream(srcPath)); + Writer output = new OutputStreamWriter( + Files.newOutputStream(outPath, CREATE, TRUNCATE_EXISTING))) { + int curPos = 0; + for (int nxtPos : insertPositions) { + int nextChunkLen = nxtPos - curPos; + long written = new LimitedReader(reader, nextChunkLen).transferTo(output); + if (written != nextChunkLen) { + throw new IOException("Unexpected number of characters transferred." + + " Expected " + nextChunkLen + " but was " + written); + } + curPos = nxtPos; + // curPos is at the end of the modifier section, so add a leading space. + // curPos ---v + // [modifiers] class... -->> [modifiers] value class... + output.write(" value"); + } + // Trailing section to end-of-file transferred from original reader. + reader.transferTo(output); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private Path moduleRelativePath(Path srcPath, String pkgName) { + Path relPath = Path.of(pkgName.replace('.', File.separatorChar)).resolve(srcPath.getFileName()); + if (!srcPath.endsWith(relPath)) { + throw new IllegalStateException(String.format( + "Expected trailing path %s for source file %s", relPath, srcPath)); + } + return relPath; + } + + /** + * Returns the character offset in the original source file at which to insert + * the {@code value} keyword. The offset is the end of the modifiers section, + * which must immediately precede the class declaration. + */ + private int valueKeywordInsertPosition(JCClassDecl classDecl) { + int pos = TreeInfo.getStartPos(classDecl.getModifiers()); + if (pos == Diagnostic.NOPOS) { + throw new IllegalStateException("Missing position information: " + classDecl); + } + return pos; + } + } + + private static Path filePath(FileObject file) { + return Path.of(file.toUri()); + } + + /** + * A forwarding reader which guarantees to read no more than + * {@code maxCharCount} characters from the underlying stream. + */ + private static final class LimitedReader extends Reader { + // These are short-lived, no need to null the delegate when closed. + private final Reader delegate; + // This should never go negative. + private int remainingChars; + + /** + * Creates a limited reader which reads up to {@code maxCharCount} chars + * from the given stream. + * + * @param delegate underlying reader + * @param maxCharCount maximum chars to read (can be 0) + */ + LimitedReader(Reader delegate, int maxCharCount) { + this.delegate = Objects.requireNonNull(delegate); + this.remainingChars = Math.max(maxCharCount, 0); + } + + @Override + public int read(char[] cbuf, int off, int len) throws IOException { + if (remainingChars > 0) { + int readLimit = Math.min(remainingChars, len); + int count = delegate.read(cbuf, off, readLimit); + // Only update remainingChars if something was read. + if (count > 0) { + if (count > remainingChars) { + throw new IOException( + "Underlying Reader exceeded requested read limit." + + " Expected at most " + readLimit + " but read " + count); + } + remainingChars -= count; + } + // Can return 0 or -1 here (the underlying reader could finish first). + return count; + } else if (remainingChars == 0) { + return -1; + } else { + throw new AssertionError("Remaining character count should never be negative!"); + } + } + + @Override + public void close() { + // Do not close the delegate since this is conceptually just a view. + } + } +} From ccf24ac2c82c72e293c3cbfdab6e9347e82802dc Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Fri, 27 Mar 2026 14:19:46 +0100 Subject: [PATCH 17/17] Fix plugin name --- make/CompileToolsJdk.gmk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/make/CompileToolsJdk.gmk b/make/CompileToolsJdk.gmk index 3034718b632..62e59bee734 100644 --- a/make/CompileToolsJdk.gmk +++ b/make/CompileToolsJdk.gmk @@ -172,7 +172,7 @@ VALUETYPE_GENSRC_SERVICE_PROVIDER := $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc/ME $(VALUETYPE_GENSRC_SERVICE_PROVIDER): $(call MakeDir, $(BUILDTOOLS_OUTPUTDIR)/valuetype_gensrc/META-INF/services) - $(ECHO) build.tools.depend.Depend > $@ + $(ECHO) build.tools.valhalla.valuetypes.GenValueClassPlugin > $@ TARGETS += $(BUILD_VALUETYPE_GENSRC) $(VALUETYPE_GENSRC_SERVICE_PROVIDER)