RFR: JDK-8301990: Separate Arguments::parse into two phases [v2]

[Justin King]

On Thu, 9 Feb 2023 06:55:55 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Justin King has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Updates based on review
>>   
>>   Signed-off-by: Justin King <jcking at google.com>
>
> src/hotspot/share/runtime/arguments.cpp line 3792:
> 
>> 3790:   JVMFlag::check_all_flag_declarations();
>> 3791: 
>> 3792:   // If flag "-XX:Flags=flags-file" is used it will be the first option to be processed.
> 
> Can you restore this comment please.

Done.

> src/hotspot/share/runtime/arguments.cpp line 3798:
> 
>> 3796:   PreprocessedArguments(PreprocessedArguments&&) = delete;
>> 3797:   PreprocessedArguments& operator=(const PreprocessedArguments&) = delete;
>> 3798:   PreprocessedArguments& operator=(PreprocessedArguments&&) = delete;
> 
> We have the ` NONCOPYABLE` macro for this.

Done.

> src/hotspot/share/runtime/arguments.cpp line 3931:
> 
>> 3929:   assert(args != nullptr, "Arguments::parse called before successful Arguments::preprocess");
>> 3930: 
>> 3931:   const char* hotspotrc = ".hotspotrc";
> 
> Can we make this a file static so that it doesn't have to written twice?

Done.

> src/hotspot/share/runtime/arguments.hpp line 278:
> 
>> 276:     friend class Arguments;
>> 277: 
>> 278:     void* _impl = nullptr;
> 
> Is the `void*` typing just to avoid a forward declaration of the `PreprocessedArguments` type?

Its to hide the implementation, so we do not have to pollute the header file. Instead of using void* I switched to using `PreprocessedImpl`.

> src/hotspot/share/runtime/arguments.hpp line 288:
> 
>> 286:     Preprocessed(Preprocessed&&) = delete;
>> 287:     Preprocessed& operator=(const Preprocessed&) = delete;
>> 288:     Preprocessed& operator=(Preprocessed&&) = delete;
> 
> Again ` NONCOPYABLE` macro.

Done.

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

PR: https://git.openjdk.org/jdk/pull/12458