> On 27 Jul 2016, at 19:36, Claes Redestad <claes.redes...@oracle.com> wrote:
> 
> On 07/25/2016 08:01 PM, Iris Clark wrote:
>> Hi, Claes.
>> 
>>> Webrev: http://cr.openjdk.java.net/~redestad/8162439/webrev.01/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8162439
>> I think that this change looks good.  We provide a shortcut for the common 
>> case where only the major version number is of interest without introducing 
>> a new API.
> 
> Thanks! Any reviewer want to give this the go-ahead?

Looks ok.

I suppose you could do:

  boolean isSimpleNumber(String s) {
    for (int i = 0; i < s.length(); i++) {
      char c = s.get(i);
      if (c < ((i > 0) ? ‘0’ : ‘1’) || c > ‘9’)
          return false;
    }
    return true;
  }


  if (isSimpleNumber(s)) {
    ...
  } else {
    return VersionBuilder.parse(s);
  }

then let VersionBuilder.parse throw errors in incorrectly formatted version 
strings.

Paul.

> 
>> 
>> Note that this has a minor side-effect that invocations of the form 
>> Runtime.Version.parse("notANumber") will now throw NFE instead of IAE.  I 
>> don't think that this is a problem since NFE extends IAE and the order that 
>> the exceptions will be checked is intentionally unspecified.
> 
> Right, I considered the minor behavior difference and thought it would be 
> considered OK and in accordance with the specification, and unproblematic 
> since this API is as of yet unreleased.
> 
> As an aside: I do think specifying both NFE and IAE to be thrown is 
> suspicious when it's not perfectly clear when which one is to be expected, 
> since it invites to wrong code with strict dependencies on which is currently 
> thrown when, even though it's unspecified, which would mean changing the 
> implementation (like in this patch) would have some possibility of messing 
> with compatibility. I wonder if we should simply specify IAE and drop NFE to 
> allow greater flexibility for future implementations?
> 
> /Claes
> 
>> 
>> Regards,
>> iris
>> (not a Reviewer)
>> 
>> -----Original Message-----
>> From: Claes Redestad
>> Sent: Saturday, July 23, 2016 9:24 AM
>> To: core-libs-dev@openjdk.java.net core-libs-dev
>> Subject: RFR: 8162439: Runtime.Version.parse needs fast-path for major 
>> versions
>> 
>> Hi,
>> 
>> please review this patch to address a startup regression due to use of
>> Runtime.Version.parse("8") etc in JarFile, as introduced by JDK-8150680.
>> This solution introduce a fast-path in case of what appears to be a single 
>> number is sent to Runtime.Version.parse to avoid initializing
>> Runtime.VersionBuilder:
>> 
>> Webrev: http://cr.openjdk.java.net/~redestad/8162439/webrev.01/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8162439
>> 
>> Passes all existing tests.
>> 
>> Thanks!
>> 
>> /Claes
> 

Reply via email to