RFR: 8324126: Error message for mistyping -XX:+Unlock...Options is not helpful [v3]

[Thomas Stuefe]

On Mon, 29 Jan 2024 21:05:46 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

>> If we run the VM option to unlock experimental vm options without the preceding ‘+’, the error message is not very helpful: 
>> 
>> ```Error: VM option 'UnlockExperimentalVMOptions' is experimental and must be enabled via -XX:+UnlockExperimentalVMOptions.```
>> 
>> With the proposed change, we should see a more descriptive error message, namely the lack of a +/- sign. 
>> 
>> ```Missing +/- setting for VM option 'UnlockExperimentalVMOptions'```
>> 
>> Testing: runtime/commandLine regression tests pass. Also added a regression test to verify expected behaviour.
>> 
>> Edit (Jan 30): 
>> The expected output is both the unlock message and the well-formed error: 
>> 
>> ```Error: VM option 'UnlockExperimentalVMOptions' is experimental and must be enabled via -XX:+UnlockExperimentalVMOptions.
>> Error: The unlock option must precede 'UnlockExperimentalVMOptions'.
>> Missing +/- setting for VM option 'UnlockExperimentalVMOptions'
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>
> Sonia Zaldana Calles has updated the pull request incrementally with one additional commit since the last revision:
> 
>   restoring end of file

Small nits wrt indentation, otherwise good. If you fix them, I don't need another review - feel free to integrate.

> I expect because it was considered appropriate. If those flags were themselves product flags then they would be subject to product flag rules.

The more I think about this, the less sense this makes. UseExperimentalVMOptions is used to enable experimental VM options. That implies that it is a product flag itself, since you must be able to use in for a product VM that is not yet in ExperimentalVMOptions-allowed-state.

src/hotspot/share/runtime/arguments.cpp line 1130:

> 1128:         return true;
> 1129:       }
> 1130:       #endif

Please revert the indentation (above too) - we don't usually indent preprocessor directives .

src/hotspot/share/runtime/arguments.cpp line 1135:

> 1133:     if (found_flag->is_bool() && !has_plus_minus) {
> 1134:         jio_fprintf(defaultStream::error_stream(),
> 1135:           "Missing +/- setting for VM option '%s'\n", argname);

Fix indentation (2 spaces)

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

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17575#pullrequestreview-1852976985
PR Review Comment: https://git.openjdk.org/jdk/pull/17575#discussion_r1472359252
PR Review Comment: https://git.openjdk.org/jdk/pull/17575#discussion_r1472361645