RFR: 8073989: Deprecated integer options are now recognized as obsolete.

[David Holmes]

Hi Max,

On 31/03/2015 4:08 AM, Max Ockner wrote:
> Hello all,
> Please review this change involving the handling of obsolete command
> line flags.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8073989
> Webrev: http://cr.openjdk.java.net/~mockner/8073989.2/

Argument processing seems to be a never ending source of bugs.

> Summary: Three key components to this bug:

Thanks for the detailed description!

> (1) When is_newly_obsolete() checks for "SomeObsoleteIntegerFlag=100" in
> the flag table, it is not recognized as a match with
> "SomeObsoleteIntegerFlag" because the lengths are different. This has
> been fixed. Arguments:process_argument() now strips everything except
> for the arguments name. (example: -XX:+SomeBooleanFlag ->
> "SomeBooleanFlag", and -XX:SomeIntegerFlag=100 -> "SomeIntegerFlag")
> This stripped argument name (fittingly called stripped_argname) is now
> passed into is_newly_obsolete, preventing the length check from failing
> on obsolete (but formeerly valid) arguments. It also eliminates the need
> for any case work and other string gymnastics from the the body of
> is_newly_obsolete. If the argument is found to be newly obsolete, the
> warning message now prints stripped_argname instead of argname to avoid
> suggesting that "SomeBooleanFlag=100" was ever a supported option.

So previously is_newly_obsolete handled +/- but despite the comment 
didn't really handle the flag=xxx form. So now before calling 
is_newly_obsolete the leading +/- or trailing = is stripped, so only the 
base argument name is actually checked. Ok.

I think these comments were misplaced originally, and seem more so now:

  874   // For locked flags, report a custom error message if available.
  875   // Otherwise, report the standard unrecognized VM option.

they belong after the is_newly_obsolete calling block, prior to:

  883   Flag* found_flag = Flag::find_flag((const char*)argname, 
arg_len, true, true);
  884   if (found_flag != NULL) {

> (2) Some flags used to be defined in both the flags table and the
> obsolete flags table. Because of this, those obsolete flags which were
> also defined in the flags table could be fuzzy matched to provide better
> error messages. Now that no flag is allowed to be in both tables, it is
> pointless to attempt to fuzzy match an obsolete flag, since fuzzy
> matching only looks in the flags table (not the obsolete flags table).
> The section at the end of Arguments:process_argument in which fuzzy
> matching is attempted on obsolete arguments  no longer makes sense and
> has been removed.

Ok.

> (3) ObsoleteFlagErrorMessage.java has been modified. The existing test
> for an obsolete flag with appended junk no longer tests for fuzzy
> matching, and a second test case has been added for integer valued flags.

Ok. The test will need to use a new "newly obsolete" flag in JDK 10. :)

Reviewed!

Thanks,
David

> Tested with jtreg runtime tests.
>
> Thanks,
> Max Ockner