On Tue, 7 Apr 2026 06:16:16 GMT, Axel Boldt-Christmas <[email protected]> wrote:
>> One other thing to point out is that running with >> DiagnoseSyncOnValueBasedClasses=2 was removed by >> [JDK-8359437](https://bugs.openjdk.org/browse/JDK-8359437) (@toxaart), but >> then for some reason Alex added it back in for >> [JDK-8377845](https://bugs.openjdk.org/browse/JDK-8377845). I'm not sure why >> he added it back in rather than just remove the comment that was missed by >> [JDK-8359437](https://bugs.openjdk.org/browse/JDK-8359437). > > If the test is no longer using a `@ValueBased` class then there is no need > for this regression test, as it no longer exercises the relevant runtime code > paths. However this means that we also no longer test that EA with > `DiagnoseSyncOnValueBasedClasses` on non-preview runs. > > From what I can see, there is no plan to remove > `DiagnoseSyncOnValueBasedClasses` with JEP 401. So I wonder if we could not > just have this specific `ValueBased` test case be conditional enabled based > on enable-preview. > > Removing the `@ValueBased` testing test case regardless of > `DiagnoseSyncOnValueBasedClasses` means we have less coverage of any > regressions that might be introduced after JEP 401 in non-preview mode w.r.t. > EA and `@ValueBased`. > > But if the consensus is that we want to remove the preview-mode illegal code > from (non-preview mode) tests, then I see no reason to keep this test case > either. Isn't this just a test that should not be run in preview mode? By the time value types are out of preview DiagnoseSyncOnValueBasedClasses should be gone too, at which time the test can be removed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30560#discussion_r3043265844
