Skip to content

8377101: [lworld] Automatically generate value class sources based on source annotations #2180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
david-beaumont wants to merge 17 commits into
base: lworld
Choose a base branch
from
Open

8377101: [lworld] Automatically generate value class sources based on source annotations #2180

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
dff38b3
Use annotation processor to generate value class sources
david-beaumont Feb 27, 2026
166340d
Remove unused file
david-beaumont Feb 27, 2026
6176d2b
Working tests and moved processor
david-beaumont Feb 27, 2026
1816326
More tests and tidying
david-beaumont Feb 27, 2026
64a70a6
rewrite test to allow skipping
david-beaumont Mar 2, 2026
f983aae
Addressing some feedback.
david-beaumont Mar 2, 2026
881192f
remove old version
david-beaumont Mar 2, 2026
5514cc5
better error handling
david-beaumont Mar 2, 2026
8b1bdce
better error handling
david-beaumont Mar 2, 2026
5ac41de
Simpler temp directory
david-beaumont Mar 2, 2026
9f7ba1c
better but still not working
david-beaumont Mar 3, 2026
e6846dc
[[AUTOMATIC FORMATTING]]
david-beaumont Mar 6, 2026
1fe6c3e
[[UNDO FORMATTING IN SQUASH BRANCH]]
david-beaumont Mar 6, 2026
89df018
Latest changes (inc. feecback).
david-beaumont Mar 6, 2026
cd6781d
Finally working
david-beaumont Mar 10, 2026
800d6ff
Merge branch 'lworld' into jdk_8377101_gensrc/squashed
david-beaumont Mar 12, 2026
12a20e1
Remove unwanted lint warning suppression
david-beaumont Mar 12, 2026
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
20 changes: 20 additions & 0 deletions make/CompileJavaModules.gmk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ include MakeFileStart.gmk

include JavaCompilation.gmk
include Modules.gmk
include ToolsJdk.gmk

include CopyFiles.gmk

Expand Down Expand Up @@ -68,6 +69,22 @@ 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.
# TODO: Remove Xlint directives below once the fix in JDK-8378740 is merged into lworld.
Copy link
Copy Markdown
Member

@liach liach Mar 12, 2026

Choose a reason for hiding this comment

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

Let's integrate this after you remove the directives and this comment.

Copy link
Copy Markdown
Contributor Author

@david-beaumont david-beaumont Mar 12, 2026

Choose a reason for hiding this comment

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

I can remove the "removal" directive, but not the "processor" one, since that's "correctly" reporting unused annotations. I'm in discussion with Jan to see if we can do better.


ifeq ($(MODULE), java.base)
Copy link
Copy Markdown
Member

@erikj79 erikj79 Mar 3, 2026

Choose a reason for hiding this comment

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

If this is only ever for java.base, then consider putting the whole thing in make/modules/java.base/Java.gmk, along with the ToolsJdk import. If it's intended to be expanded to a limited set of modules, consider making the conditional on a suitably named variable that can be set from make/modules/<module>/Java.gmk to activate this feature. If it's intended to cover all modules eventually, then leaving it here makes sense.

Copy link
Copy Markdown
Contributor Author

@david-beaumont david-beaumont Mar 6, 2026

Choose a reason for hiding this comment

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

Restricting it to java.base is because it's used for every module, there's a build failure for one of the modules.
Conceptually it should be applied to all modules which can participate in preview behaviour (which is not all modules, but is more than java.base).
I just wanted to set the pieces in the right place, even if I only enable it for java.base for now.

PREPROCESSOR_FLAGS := \
-Xlint:-removal -Xlint:-processing \
Copy link
Copy Markdown
Contributor Author

@david-beaumont david-beaumont Feb 27, 2026

Choose a reason for hiding this comment

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

This is probably contentious, and should be temporary, but there's a bug in javac which this work uncovered, and enabling annotation processing confuses javac into giving out unexpected warnings.

See: https://bugs.openjdk.org/browse/JDK-8378740

Copy link
Copy Markdown
Contributor Author

@david-beaumont david-beaumont Feb 27, 2026

Choose a reason for hiding this comment

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

I'm absolutely fine with not submitting this PR until the warnings are sorted out (esp. for java.base).

-Avalueclasses.outdir=$(SUPPORT_OUTPUTDIR)/gensrc-valueclasses \
-processor $(VALUETYPE_GENSRC_PROCESSOR_NAME)

PROCESSOR_PATH += $(VALUETYPE_GENSRC_PROCESSOR_PATH)
DEPENDS += $(BUILD_VALUETYPE_GENSRC)
endif

################################################################################
# Copy zh_HK properties files from zh_TW (needed by some modules)

Expand Down Expand Up @@ -107,6 +124,7 @@ $(eval $(call SetupJavaCompilation, $(MODULE), \
MODULE := $(MODULE), \
SRC := $(wildcard $(MODULE_SRC_DIRS)), \
INCLUDES := $(JDK_USER_DEFINED_FILTER), \
DEPENDS := $(DEPENDS), \
FAIL_NO_SRC := $(FAIL_NO_SRC), \
BIN := $(COMPILATION_OUTPUTDIR), \
HEADERS := $(SUPPORT_OUTPUTDIR)/headers, \
Expand All @@ -120,9 +138,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, \
Expand Down
14 changes: 13 additions & 1 deletion make/CompileToolsJdk.gmk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 := \
Expand Down Expand Up @@ -155,4 +156,15 @@ endif

################################################################################

$(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 += $(BUILD_VALUETYPE_GENSRC)

################################################################################

include MakeFileEnd.gmk
11 changes: 11 additions & 0 deletions make/ToolsJdk.gmk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,16 @@ 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

# 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
include MakeIncludeEnd.gmk
13 changes: 11 additions & 2 deletions make/common/JavaCompilation.gmk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -288,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

Expand All @@ -302,13 +306,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_AUGMENTED_PROCESSOR_PATH += $$(BUILDTOOLS_OUTPUTDIR)/depend
endif

ifneq ($$($1_AUGMENTED_CLASSPATH), )
$1_FLAGS += -cp $$(call PathList, $$($1_AUGMENTED_CLASSPATH))
endif

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
# exist yet, is in it.
$$(foreach d, $$($1_SRC), \
Expand Down
Loading