RFR (XL) 8081519 Split globals.hpp to factor out the Flag class
david.holmes at oracle.com
Sat Mar 31 07:46:08 UTC 2018
Scanning through this it seems okay.
Like Coleen I'm wondering why so many "flag" using .cpp files don't
include jvmFlag.hpp? I expect they transitively included globals.hpp
previously but now will need an explicit include.
The two new files have a slight formatting error with their copyright,
you need a comma after 2018.
The include guards for all the relocated header files and the new files eg:
25 #ifndef SHARE_VM_RUNTIME_JVMFLAG_HPP
need updating now you added the extra directory.
Bikeshed: I'm not sure about calling the directory jvmFlags. First
everything under src/hotspot is "jvm" so it is somewhat redundant to
state that. Second you end up with jvmFlag/jvmFlag*.* which is more
redundancy. I think runtime/flags would suffice.
On 31/03/2018 3:27 AM, Gerard Ziemski wrote:
> Hi all,
> Please review this large and tedious (sorry), but simple fix that accomplishes the following:
> #1 factor out the command option flag related APIs out of globals.hpp/.cpp into its own dedicated files, i.e. jvmFlag.hpp/.cpp
> #2 moved all jvmFlag* files into its own dedicated folder (i.e. src/hotspot/share/runtime/jvmFlag/)
> #3 merge Flag (too generic name) and CommandLineFlag classes and rename them as JVMFlag
> #4 cleanup globals.hpp includes originally added by the JEP-245
> Note: the renamed file retain their history, but one needs to add “follow” flag, ex. “hg log -f file”
> Passes Mach5 hs_tier1-tier5, jtreg/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges tests.
More information about the hotspot-dev