RFR: 8066821(S) Enhance command line processing to manage deprecating and obsoleting -XX command line arguments

[Thomas Schatzl]

On Mon, 2015-01-12 at 17:10 -0500, Derek White wrote:
> http://cr.openjdk.java.net/~drwhite/8066821/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8066821
> 
> This webrev adds support for handling "deprecated" -XX options
> (options that still do something but are planned for removal) and
> "alias" options (alternate names for other -XX options) by simply
> adding entries to the deprecated_jvm_flags and/or aliased_jvm_flags
> tables. This follows the
> example of the existing obsolete_jvm_flags table.
> 
> This replaces a lot of ad-hoc and occasionally wrong code in
> arguments.cpp (including Arguments::check_deprecated_gc_flags) as well
> as supporting automatically disabling options after a certain version.
> 
> Removed Code:
>  - Removed global DefaultMaxRAMFraction, which was an "improper" alias
> for "MaxRAMFraction" (two variables that were roughly kept in sync vs.
> two names for the same variable).
>  - Arguments::check_deprecated_gc_flags().
>  - Alias handling code in Arguments::parse_each_vm_init_arg().
> It also avoids future ad-hoc and occasionally wrong code as new
> options get aliased and deprecated.
> 
> Tests:
> Deprecated and alias options can be tested by adding entries to tables
> in new tests:
>   VMAliasOptions.java
>   VMDeprecatedOptions.java
> 
> The new tests subsume these existing tests:
>   test/gc/startup_warnings/TestDefaultMaxRAMFraction.java
>   test/gc/startup_warnings/TestNoParNew.java 


Some comments:

arguments.cpp:
  - in the big comment please avoid adding special headers/footers to
sections of the file. Over time they just get orphaned and waste up
space.

  - I would prefer to use "special" comments like "/**". We "mostly"
also only ever use the "//" comment style for explanation in cpp files.
(Waiting for somebody to mention that in file XY we do differently -
okay just saw one in arguments.cpp, but generally even in that file "//"
is used even for multi-line comments ;)

  - the comment is not about (general) -XX argument processing, but
processing of obsolete and deprecated arguments.

  - note that not only globals.hpp may contain -XX arguments, but also
other files.

  - typo: Obosloete -> Obsolete

  - not sure if the type name "SpecialFlag" is very descriptive.

  - comments always start with upper case (e.g. "// when the
warning ...") with full stop at the end in the SpecialFlag struct.

  - I would prefer to remove the "Declare an" before the description of
SpecialFlag (or "Describes ..."). It does not seem to add anything. Also
I would move part of the description of the deprecated_vm_flags to the
SpecialFlag description, because it seems to describe what a SpecialFlag
is, and not what the deprecated_jvm_flags table contains.

  - maybe add a one-liner what AliasedFlag really is to its description
instead of speaking generally about what an alias is (which has already
been done in the large introductory comment). 

  - please only describe the interface in the definition of a method
(i.e. in the hpp file), not in the implementation (cpp file). If you
have a general implementation specific comment you may put it into the
cpp file. (.e.g is_newly_obsolete)

  - maybe line up the closing braces of the aliased_jvm_flags array too
like the others

arguments.hpp:
  - the comment for is_deprecated_flag() is wrongly indented. Also the
second sentence has odd additional spaces.

  - do not try to repeat the exact same comment for a method in the cpp
file. Actually the one for is_deprecated_flag() in the cpp file is
already different and apparently outdated...

  - extra closing bracket after "(should be ignored)"

  - the comment for handle_aliases_and_deprecation() should start with
upper case.

  - what is a "real" name of an option argument?

TEST.groups:

  - if the copyright for a particular file spans multiple years, the
official format is "X, Z", not "X, Y, Z"

Rest looks okay.

Thanks,
  Thomas