On Tue, 3 Mar 2026 10:35:12 GMT, David Beaumont <[email protected]> wrote:

>> Adds a new AnnotationProcessor to read the @MigratedValueClass on 
>> prospective value classes and generate the equivalent source file with the 
>> 'value' keyword at each annotated class declaration.
>> 
>> This adds new PROCESSOR_PATH variable to the compilation macro and use it 
>> for annotation processing.
>> This also requires moving the plugin discovery path to the new variable (was 
>> in classpath before) because the addition of a '--processor-path' flag 
>> disables using the classpath for plugin discovery.
>> 
>> For now this is all limited to `java.base` but that's expandable later 
>> easily. There was an issue just enabling it for all modules however (esp. 
>> `jdk.incubator` which might be special).
>> 
>> While there is currently no specific unit test for this code, if it were to 
>> fail to generate the correct value classes, a lot of other downstream tests 
>> would fail.
>
> David Beaumont has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   better but still not working

make/CompileJavaModules.gmk line 77:

> 75: # Temporarily restrict this to java.base, but it can be expanded later.
> 76: 
> 77: ifeq ($(MODULE), java.base)

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.

make/CompileJavaModules.gmk line 176:

> 174:         SRC := $(wildcard $(MODULE_PREVIEW_SRC_DIRS)), \
> 175:         INCLUDES := $(JDK_USER_DEFINED_FILTER), \
> 176:         EXTRA_DEPS := $(VALUETYPE_GENSRC_PROCESSOR_DEP), \

EXTRA_DEPS looks to be internal in SetupJavaCompilation and not meant to be 
used as a parameter. There is a documented parameter `DEPENDS` which seems to 
do what you need.

make/common/JavaCompilation.gmk line 306:

> 304:     # source root, which are not being recompiled in this compilation:
> 305:     $1_AUGMENTED_CLASSPATH += $$($1_BIN)
> 306:     $1_PROCESSOR_PATH += $$(BUILDTOOLS_OUTPUTDIR)/depend

Why this change? Do you really need to mess with the API digest plugin? If this 
is needed, then please document any new parameters on the SetupJavaCompilation 
macro.

-------------

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2180#discussion_r2880576658
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2180#discussion_r2880587543
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2180#discussion_r2880594189

Reply via email to