RFR: 8351958: Some compile commands should be made diagnostic [v5]

[Vladimir Kozlov]

On Mon, 19 May 2025 10:36:36 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> Error when using a `CompileCommand` that is an alias for a diagnostic option when `-XX:+UnlockDiagnosticVMOptions` is not provided.
>> 
>> The argument processing works this way:
>> 1. Flags are parsed, setting the value accordingly. For `CompileCommand`, each option is added to a  `\n`-separated string. At this step, if a flag is diagnostic but `-XX:+UnlockDiagnosticVMOptions` is not provided, then an error message is emitted, argument parsing fails and the VM terminates. Yet, the value of `CompileCommand` is still an unparsed list of string.
>> 2. Eventually, `CompileCommand` is parsed. For some of them, the value of regular flag is used as the default value, and as far as I know, it's the only mapping between `CompileCommand` and the equivalent flag. Moreover, at this point, the order of the various command line arguments is lost: it is not possible to know which `CompileCommand` comes before or after the `-XX:+UnlockDiagnosticVMOptions`.
>> 
>> Moreover, `CompileCommand` are parsed in the same way as compiler directives coming from a file. If we complain about diagnostic `CompileCommand`, we should also when coming from a directive file, for consistency. But then, while the relative order of `CompileCommand` and `-XX:+UnlockDiagnosticVMOptions` is lost, it simply makes no sense to compare the ordering of command line arguments and directives from a file.
>> 
>> So, before the difficulty and the relative lack of sense, I defaulted to ignore the ordering requirement. And by using the `CompileCommand` error reporting mechanism, we get an error that is consistent with other `CompileCommand`-parsing related errors, e.g.
>> 
>> CompileCommand: An error occurred during parsing
>> Error: VM option 'PrintAssembly' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.
>> Line: 'PrintAssembly,*::*'
>> 
>> Usage: '-XX:CompileCommand=<option>,<method pattern>' - to set boolean option to true
>> Usage: '-XX:CompileCommand=<option>,<method pattern>,<value>'
>> Use:   '-XX:CompileCommand=help' for more information and to list all option.
>> 
>> CompileCommand: compileonly Test.* bool compileonly = true
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>> 
>> 
>> The other problem is how to identify that a `CompileCommand` is an alias for a diagnostic flag. I refrained from hardcoding the list, but there is no nice mapping stating "this is an alias for...", only the default value for some:
>> https://github.com/...
>
> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revert blackhole test change

src/hotspot/share/compiler/compilerOracle.cpp line 1027:

> 1025:         if (option2type(option) == OptionType::Bool) {
> 1026:           register_command(typed_matcher, option, error_buf, sizeof(error_buf), true);
> 1027:           if (*error_buf != '\0') {

@marc-chevalier I know that you follow existing patter. But may be `register_command()` should return boolean result if it  succeeded or not so you can check it instead of loading value from buffer.

`scan_value()`and `scan_option_and_value()` may need to do the same.

Note, `print_parse_error()` has assert to check that error message is not empty.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25150#discussion_r2096015604