RFR: JDK-8301990: Separate Arguments::parse into two phases

[David Holmes]

On Tue, 7 Feb 2023 16:21:07 GMT, Justin King <jcking at openjdk.org> wrote:

> Separate out some logic from `Arguments::parse` into a separate preceding phase, preprocessing. See JDK-8301990 and JDK-8299196 for more details.

Looks good! No functional changes as you stated. A couple of minor nits below - but pre-approving.

For other reviewers the webrev frames view is the easiest way to see the equivalence.

Thanks.

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.

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.

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?

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?

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.

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

Marked as reviewed by dholmes (Reviewer).

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