8380084: [lworld] Implement value classes in test code using annotation by bwhuang-us · Pull Request #2253 · openjdk/valhalla

bwhuang-us · 2026-03-24T01:12:25Z

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:

Create ValueClass annotation.
Create a javac plugin, ValueClassPlugin, that implements a TaskListener to mark a class as a value class during the PARSE phase.
Build the ValueClass plugin.
Pass a new plugin option, -Xplugin:ValueClassPlugin, to enable the plugin and enable preview.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8380084: [lworld] Implement value classes in test code using annotation (Enhancement - P4)

Reviewers

Chen Liang (@liach - Committer) ⚠️ Review applies to 12a77ab3

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2253/head:pull/2253
$ git checkout pull/2253

Update a local copy of the PR:
$ git checkout pull/2253
$ git pull https://git.openjdk.org/valhalla.git pull/2253/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2253

View PR using the GUI difftool:
$ git pr show -t 2253

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2253.diff

Using Webrev

Link to Webrev Comment

…ation

bridgekeeper · 2026-03-24T01:14:09Z

👋 Welcome back bhuang! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-03-24T01:15:18Z

@bwhuang-us This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8380084: [lworld] Implement value classes in test code using annotation

Reviewed-by: liach

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 222 new commits pushed to the lworld branch:

4bdc262: Merge
69deec2: 8378985: serviceability/sa/TestJhsdbJstackMixedWithXComp.java failed if sender frame is return barrier of Continuation
ceb109f: 8380081: jpackage: Incorrect descriptions of win-specific options
... and 219 more: https://git.openjdk.org/valhalla/compare/1257fbdfb3db4bde784c8dab830f2220c2b13544...lworld

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@liach, @lmesnik, @RogerRiggs) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

liach · 2026-03-24T18:38:51Z

Looks good in principle but this means we need extra JTREG flags and need to update CI configuration; wonder if we can just update our @build or @run directives instead.

bwhuang-us · 2026-03-25T01:58:24Z

@liach Could you please elaborate your suggestions? The goal of this task is to run tests with and without enable-preview. The annotation is the indirection that lets the same source file be valid Java in both modes. Without it (or something equivalent), there's no way to write a single source file that compiles cleanly on a normal build and becomes a value class when compiled with the plugin under preview.

liach · 2026-03-25T02:13:02Z

Does 'make test TEST=java/util/Collections/AddAll.java' run the value class patched version of the test?

bwhuang-us · 2026-03-25T17:11:20Z

@liach make test TEST=jdk/java/util/Collections/AddAll.java runs in normal mode. It needs the VALUE_CLASS_PLUGIN option to enable ValueClassPlugin and preview feature: make test TEST=jdk/java/util/Collections/AddAll.java JTREG=VALUE_CLASS_PLUGIN=true

liach · 2026-03-25T17:39:52Z

Yep, so my concern is this requires special command line, which means CI tweaks elsewhere... If Oracle picks it up, github actions or other test runners may not be aware of this still.

…ng explicit VALUE_CLASS_PLUGIN flag

bwhuang-us · 2026-03-25T22:45:16Z

@liach Please review the latest commit, which removes the VALUE_CLASS_PLUGIN flag and adds a test instance with ValueClassPlugin and preview features enabled.

liach

Much better now! Thanks for this update.

liach · 2026-03-26T00:45:28Z

test/jdk/java/util/Collections/AddAll.java

+ * @compile AddAll.java
 * @run main AddAll
+ *
+ * @compile -XDaccessInternalAPI --enable-preview --source ${java.specification.version} -Xplugin:ValueClassPlugin AddAll.java


You might create a separate test scenario like this:

valhalla/test/langtools/tools/javap/T4975569.java

Line 38 in ef301b7

* @test id=Preview

You can use another set of directives.

Alternatively I think you can do a @enablePreview after the old @run to make the subsequent run preview-enabled. But having two tests make the result looks slightly cleaner that you know it's the preview case that fails just in case.

mlbridge · 2026-03-26T16:15:22Z

Webrevs

… @enablePreview tag.

liach

Looks good to me. I think at least @RogerRiggs would like to take a look too.

lmesnik · 2026-03-27T00:39:22Z

make/RunTests.gmk

    ))
  endif

+  ifneq ($$(wildcard $$(JTREG_VALUE_CLASS_ANNOTATION_JAR)), )


Do we need to add this jar into cpa?

The tests should add '/test/lib' library and should compile this annotationati anyway?

Yeah, you're right. This code is redundant after the restructuring. We don't need to add the ValueClass annotation to the cpa.

lmesnik · 2026-03-27T00:49:35Z

test/jdk/java/util/Collections/AddAll.java

 */

+/*
+ * @test id=Preview


I think that is going to fail if test is executed with
jtreg -jdk: -test ....

I think that is better to support
make JTREG_VALUE_CLASS=true run-test TEST=test/jdk/java/util/Collections/AddAll.java
and in this mode test is compiled with this plugin.

So we are not executing the preview mode by default, but can run all our test compiling and executing with --enable-preview. So all JDK build in value classes and these ValueClass marked classes are value classes.

You're right that plain jtreg without cpa will fail with "plugin not found". But this is temporary solution that goes away when value classes graduate from preview - so the failure being loud and obvious is a feature, not a bug. Anyone running these tests without the plugin jar is misconfigured rather than silently getting results that don't test value class behavior.

On the other hand, make test already handles it properly - the plugin jar is built as part of the test image and cpa is added automatically.

I understand your intentions, but not sure it is achievable.
We can't break plain jtreg execution for testing preview features. We should be able to run the default test without any additional requirements. The change of default behavior requires much wider discussion and documentation changes that doesn't worth for temporary solution.

It still think that it should be controlled in the make files. Like adding (the actual options might be updated)

+ ifneq ($$(JTREG_ENABLE_VALUE_CLASS), ) + $1_JTREG_BASIC_OPTIONS += \ + -vmoption:'-Djtreg.value.class=true' \ + -vmoption:'--enable-preview' \ + -javacoption:'-XDaccessInternalAPI' \ + -javacoption:'--source' \ + -javacoption:'27' \ + -javacoption:'--enable-preview' \ + -javacoption:'-cp' \ + -javacoption:'...../public-plugin.jar' \ + -javacoption:'-Xplugin:VallhallaPlugin' + + $1_JTREG_BASIC_OPTIONS += $$(addprefix $$(JTREG_PROBLEM_LIST_PREFIX), $$(wildcard \ + $$(addprefix $$($1_TEST_ROOT)/, ProblemList-enable-preview.txt) \ + )) + endif

So we have this mode JTREG_ENABLE_VALUE_CLASS and use it instead of '--enable-preview' to test value classes. Instead of just running test with value classes enabled, we compile and use test classes as value classes here.

And test should just pass when executed with jtreg -test:...

test/jtreg_value_class_plugin/plugin/jdk/test/valueclass/ValueClassPlugin.java

RogerRiggs · 2026-03-27T01:02:19Z

test/jdk/java/util/Collections/AddAll.java

+ * @test id=NoPreview
 * @bug     4822887
 * @summary Basic test for Collections.addAll
 * @author  Josh Bloch
 * @key randomness
+ * @library /test/lib
+ * @build jdk.test.lib.valueclass.ValueClass
+ * @run main AddAll
 */

+/*
+ * @test id=Preview
+ * @bug     4822887
+ * @summary Basic test for Collections.addAll - Preview
+ * @author  Josh Bloch
+ * @key randomness
+ * @library /test/lib
+ * @build jdk.test.lib.valueclass.ValueClass
+ * @enablePreview
+ * @compile -XDaccessInternalAPI -Xplugin:ValueClassPlugin AddAll.java
+ * @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.

RogerRiggs · 2026-03-27T01:13:01Z

test/lib/jdk/test/lib/valueclass/ValueClass.java

+ */
+@Retention(RetentionPolicy.SOURCE)
+@Target(ElementType.TYPE)
+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 ...

Would it be better to name it AsValueClass?

… drop redundant annotation jar from jtreg -cpa, and collapse AddAll test to a single block with externally-controlled preview.

lmesnik

see my inline comments about test dependency on plugin in the line
@compile -XDaccessInternalAPI -Xplugin:ValueClassPlugin AddAll.java

JDK-8380084 [lworld] Implement value classes in test code using annot…

5edd46a

…ation

Remove trailing whitespaces

573b035

[lworld] Auto-detect ValueClassPlugin JAR presence instead of requiri…

062adfb

…ng explicit VALUE_CLASS_PLUGIN flag

liach reviewed Mar 26, 2026

View reviewed changes

openjdk bot changed the title ~~JDK-8380084 [lworld] Implement value classes in test code using annotation~~ 8380084: [lworld] Implement value classes in test code using annotation Mar 26, 2026

openjdk bot added the rfr Pull request is ready for review label Mar 26, 2026

Split AddAll test into separate NoPreview and Preview scenarios using…

12a77ab

… @enablePreview tag.

liach approved these changes Mar 26, 2026

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 26, 2026

lmesnik suggested changes Mar 27, 2026

View reviewed changes

RogerRiggs suggested changes Mar 27, 2026

View reviewed changes

Simplify ValueClassPlugin to skip tree scan when preview is disabled,…

a3f4b28

… drop redundant annotation jar from jtreg -cpa, and collapse AddAll test to a single block with externally-controlled preview.

bwhuang-us requested review from RogerRiggs and lmesnik March 27, 2026 18:54

Rename ValueClass to AsValueClass.

0c90185

lmesnik suggested changes Mar 27, 2026

View reviewed changes

Conversation

bwhuang-us commented Mar 24, 2026 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Mar 24, 2026

Uh oh!

openjdk bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liach commented Mar 24, 2026

Uh oh!

bwhuang-us commented Mar 25, 2026

Uh oh!

liach commented Mar 25, 2026

Uh oh!

bwhuang-us commented Mar 25, 2026

Uh oh!

liach commented Mar 25, 2026

Uh oh!

bwhuang-us commented Mar 25, 2026

Uh oh!

liach left a comment

Choose a reason for hiding this comment

Uh oh!

liach Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

mlbridge bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

liach left a comment

Choose a reason for hiding this comment

Uh oh!

lmesnik Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

bwhuang-us Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

lmesnik Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

bwhuang-us Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmesnik Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RogerRiggs Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

RogerRiggs Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

bwhuang-us Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

lmesnik left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

bwhuang-us commented Mar 24, 2026 •

edited by openjdk bot

Loading

openjdk bot commented Mar 24, 2026 •

edited

Loading

mlbridge bot commented Mar 26, 2026 •

edited

Loading

bwhuang-us Mar 27, 2026 •

edited

Loading

lmesnik Mar 27, 2026 •

edited

Loading