On Tue, 2 Apr 2024 16:49:19 GMT, Stefan Karlsson <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Clean up notproduct from tests.
>
> src/hotspot/share/runtime/arguments.cpp line 3420:
>
>> 3418: static void apply_debugger_ergo() {
>> 3419: #ifndef PRODUCT
>> 3420: // UseDebuggerErgo is notproduct
>
> Now that the flag has been changed to a develop flag, it seems wrong that
> these are guarded by "#ifndef PRODUCT". Shouldn't this be changed to check
> for ASSERT instead?
Yes, ifdef ASSERT is more appropriate.
> src/hotspot/share/runtime/flags/jvmFlag.hpp line 118:
>
>> 116: EXPERIMENTAL_FLAG_BUT_LOCKED,
>> 117: DEVELOPER_FLAG_BUT_PRODUCT_BUILD,
>> 118: NOTPRODUCT_FLAG_BUT_PRODUCT_BUILD
>
> Should the ',' on the previous line be removed?
Yes, I guess our compilers don't complain about that anymore.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18541#discussion_r1548261310
PR Review Comment: https://git.openjdk.org/jdk/pull/18541#discussion_r1548269993
