On Thu, 26 Mar 2026 16:30:38 GMT, Bill Huang <[email protected]> wrote:
>> The idea is to implement the **ValueClass** annotation that can be used in
>> test code to allow to run the same tests with and without --enable-preview.
>>
>> This annotation should be used to mark value classes.
>>
>> The new jtreg test compilation and execution mode that uses javac plugin to
>> process classes during compilation. This plugin change classes with
>> **ValueClass** annotation to value classes.
>> By default, the annotation doesn't have any impact on the tests.
>>
>> Here are the work items:
>>
>> 1. Create ValueClass annotation.
>> 2. Create a javac plugin, ValueClassPlugin, that implements a TaskListener
>> to mark a class as a value class during the PARSE phase.
>> 3. Build the ValueClass plugin.
>> 4. Pass a new plugin option, -Xplugin:ValueClassPlugin, to enable the plugin
>> and enable preview.
>
> Bill Huang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Split AddAll test into separate NoPreview and Preview scenarios using
> @enablePreview tag.
Changes requested by rriggs (Committer).
test/jdk/java/util/Collections/AddAll.java line 45:
> 43: * @enablePreview
> 44: * @compile -XDaccessInternalAPI -Xplugin:ValueClassPlugin AddAll.java
> 45: * @run main AddAll
This doesn't scale well, the mods have to be made to each test.
Separate test blocks are fine but are only 1 use case.
There are entire test runs in which enable-preview is passed in view a test
argument and all test are run with/without enable preview based on the flags.
Since the plugin is inactive unless --enable-preview is present, using the
second @test block but without @enablePreview would allow the tests to be run
either way with enable-preview controlled from outside the test.
test/jtreg_value_class_plugin/plugin/jdk/test/valueclass/ValueClassPlugin.java
line 75:
> 73:
> 74: Preview preview = Preview.instance(ctx);
> 75: new TreeScanner() {
Why scan the tree looking for the annotation unless preview.isEnabled?
Move the `preview.isEnabled` check to early in `finished`.
test/lib/jdk/test/lib/valueclass/ValueClass.java line 47:
> 45: @Retention(RetentionPolicy.SOURCE)
> 46: @Target(ElementType.TYPE)
> 47: public @interface ValueClass {}
The name `ValueClass` does not reflect the conditional or test context use of
the annotation.
It should be named to convey that it is a parameter to the test or condition.
Maybe "TestableAsValueClass", or ...
-------------
PR Review:
https://git.openjdk.org/valhalla/pull/2253#pullrequestreview-4018269221
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2253#discussion_r2998458494
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2253#discussion_r2998441025
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2253#discussion_r2998481786
