On Mon, 15 Dec 2025 19:50:33 GMT, Daniel D. Daugherty <[email protected]> 
wrote:

>> Paul Hübner has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains five additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'lworld' into JDK-8373575
>>  - Move some tests to Xcomp list.
>>  - Add failing test to problemlist.
>>  - Add one of the AOT tests.
>>  - Problemlist and fix.
>
> test/langtools/tools/javac/AnonymousClass/AnonymousClassFlags.java line 29:
> 
>> 27:  * @summary Verify that anonymous class binaries have the correct flags 
>> set
>> 28:  * @modules java.base/jdk.internal.misc
>> 29:  * @requires !java.enablePreview
> 
> Why is `@requires !java.enablePreview` needed here? The test's id value is 
> NoPreview so that might
> indicate that the test doesn't support --enable-preview bits, but I had to 
> expand the review's context
> to see that id value.

Another thought occurred to me. Perhaps add a comment line above the 
`@requires`:

@comment Test does not like Valhalla value classes.

or something like that. I think it would be good if we added a comment
when we use `@requires !java.enablePreview` to indicate which preview
feature is problematic.

> test/langtools/tools/javap/T4975569.java line 30:
> 
>> 28:  * @modules jdk.jdeps/com.sun.tools.javap
>> 29:  * @modules java.base/jdk.internal.misc
>> 30:  * @requires !java.enablePreview
> 
> Why is  `@requires !java.enablePreview` needed here? The test's `id` value is 
> `NoPreview` so that might
> indicate that the test doesn't support `--enable-preview` bits, but I had to 
> expand the review's context
> to see that `id` value.

Another thought occurred to me. Perhaps add a comment line above the 
`@requires`:

@comment Test does not like Valhalla value classes.

or something like that. I think it would be good if we added a comment
when we use `@requires !java.enablePreview` to indicate which preview
feature is problematic.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1793#discussion_r2620862468
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1793#discussion_r2620862839

Reply via email to