RFR: JDK-8193877- DateTimeFormatterBuilder throws ClassCastException when using padding
Pallavi Sonal
pallavi.sonal at oracle.com
Mon Apr 30 08:20:32 UTC 2018
Hi Roger/Stephen,
> The fix here seems to make padding to a fixed width incompatible with
> adjacent value parsing.
> That's not intuitive, since adjacent value parsing is intended to
> allow more flexible parsing of a combination leading fixed-width
> fields and subsequent variable length fields.
> The specification of the behavior of padding vs adjacent value parsing
> should be investigated and clarified if necessary.
>
The fix was based on the observation of existing design that whenever a leading variable/fixed length field has subsequent padded fixed length field the setup of adjacent value parsing ends as soon as padding is encountered. For ex. in the below example adjacent value parsing is used and the parser is updated to fixed width.
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("ddM");
formatter.parse("2012");
But in the following case adjacent value parsing mode is not set up , instead for M a new parser is used .
DateTimeFormatter.ofPattern("ddppM");
Similarly, the following is successful, as adjacent value parsing is used and a subsequent width is reserved for M.
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMM");
formatter.parse("201812");
But, the same does not happen with padding in following example and thus parsing fails because year is parsed greedily.
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyppMM");
formatter.parse("201812");
Also, adjacent value parsing is only for NumberPrinterParsers as per the spec which PadPrinterParserDecorator is not.
Based on this, I had added this fix where if a padded field is followed by non-padded fields , the padded field is parsed using a new parser and the adjacent value parsing only happens with another parser for the consecutive non-padded fields.
Should the PadPrinterDecoratorParsers be allowed to participate in adjacent value parsing and should this affect both padded followed by non-padded fields as well as non-padded followed by padded fields ?
Thanks,
Pallavi Sonal
-----Original Message-----
From: core-libs-dev-request at openjdk.java.net <core-libs-dev-request at openjdk.java.net>
Sent: Wednesday, April 25, 2018 7:08 AM
To: core-libs-dev at openjdk.java.net
Subject: core-libs-dev Digest, Vol 132, Issue 84
Send core-libs-dev mailing list submissions to
core-libs-dev at openjdk.java.net
To subscribe or unsubscribe via the World Wide Web, visit
http://mail.openjdk.java.net/mailman/listinfo/core-libs-dev
or, via email, send a message with subject or body 'help' to
core-libs-dev-request at openjdk.java.net
You can reach the person managing the list at
core-libs-dev-owner at openjdk.java.net
When replying, please edit your Subject line so it is more specific than "Re: Contents of core-libs-dev digest..."
----------------------------------------------------------------------
Message: 1
Date: Tue, 24 Apr 2018 23:32:51 +0100
From: Stephen Colebourne <scolebourne at joda.org>
To: core-libs-dev <core-libs-dev at openjdk.java.net>
Subject: Re: RFR: JDK-8193877- DateTimeFormatterBuilder throws
ClassCastException when using padding
Message-ID:
<CACzrW9BbZz82oqLPOwof6k=GLxtcGXvuDPoqYMqO2g=ZhtY5Sg at mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
To add to Roger's comments, the tests should cover parsing as well as formatting. It is the adjacent parsing behaviour that will be most important to test and get right.
For example,
Pattern "dMyyyy" cannot be parsed
(is "1122018" the 1st Dec or the 11th Feb?)
But this one should be parsed "ppdppMyyyy"
(" 1122018" has fixed width day = 1 and fixed width month = 12)
("11 22018" has fixed width day =11 and fixed width month = 2)
And as Roger says, this should be tested using DateTimrFormatterBuilder::padNext as well as the pattern letters.
thanks
Stephen
On 18 April 2018 at 19:34, Roger Riggs <roger.riggs at oracle.com> wrote:
> Hi Pallavi,
>
> The fix here seems to make padding to a fixed width incompatible with
> adjacent value parsing.
> That's not intuitive, since adjacent value parsing is intended to
> allow more flexible parsing of a combination leading fixed-width
> fields and subsequent variable length fields.
> The specification of the behavior of padding vs adjacent value parsing
> should be investigated and clarified if necessary.
>
> The implementation would be better expressed as checking whether the
> active PrinterParser
> *is* a NumberPrinterParser as a precondition for entering the adjacent
> parsing mode block (and the necessary cast).
> And otherwise, fall into the existing code in which the new Parser
> becomes the new active parser.
>
> The tests should be included in the existing test classes for padding,
> and be written using the direct DateTimeFormatterBuilder methods
> (padNext(), instead of the
> patterns) since the patterns
> are just a front end for the builder methods.
> See test/java/time/format/TestPadPrinterDecorator.java
>
> TestDateTimeFormatter.java:
>
> line 96: please keep the static imports for testng together
>
> Line 662: The odd formatting and incorrect indentation should no
> longer be a problem because the indentation will not need to change.
>
> Regards, Roger
>
>
>
> On 4/18/18 8:41 AM, Pallavi Sonal wrote:
>>
>> Hi,
>>
>> Please review the changes to the following issue:
>>
>> Bug :https://bugs.openjdk.java.net/browse/JDK-8193877
>>
>> The proposed fix is located at:
>>
>> Webrev :http://cr.openjdk.java.net/~rpatil/8193877/webrev.00/
>>
>>
>> When padding is used in a pattern where there are unpadded values
>> adjacent to padded ones (like "pdQ") , the previous PrinterParser
>> (used for parsing 'd' in the example ) is fetched to use its settings
>> for parsing the next value('Q' in the example). But , in cases like
>> this , it is a PadPrinterDecoratorParser instead of an assumed
>> NumberPrinterParser and a ClassCastException is thrown.
>>
>> The fix has been done to check such cases where the previous
>> parserprinter is PadPrinterDecoratorParser and use the new
>> NumberPrinterParser instead for parsing the next value.
>>
>>
>> All the tier1 and tier2 Mach 5 tests have passed.
>>
>>
>> Thanks,
>>
>> Pallavi Sonal
>>
>>
>
>
------------------------------
Message: 2
Date: Tue, 24 Apr 2018 23:41:16 +0100
From: Stephen Colebourne <scolebourne at joda.org>
To: core-libs-dev <core-libs-dev at openjdk.java.net>
Subject: Re: [11] RFR: 8181157: CLDR Timezone name fallback
implementation
Message-ID:
<CACzrW9DOKGHBsfzC4JUy6ym19mGeNx0G4GXgQUtpAoykDQpMFw at mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
I had a quick look through and didn't see anything obvious, but its not an area I know well.
Stephen
On 17 April 2018 at 22:28, Naoto Sato <naoto.sato at oracle.com> wrote:
> Hello,
>
> Please review the changes to the following issue:
>
> https://bugs.openjdk.java.net/browse/JDK-8181157
>
> The proposed fix is located at:
>
> http://cr.openjdk.java.net/~naoto/8181157/webrev.05/
>
> This RFE is to implement CLDR time zone names fallback mechanism [1].
> Prior to this fix, time zone names are substituted with English names.
> With this change, it is generated according to the logic defined in TR
> 35. Here are the highlight of the changes:
>
> CLDRConverter:
> - CLDRConverter has changed to not substitute/invent time zone display
> names, except English bundle for compatibility & runtime performance.
> - For English bundle, copy over COMPAT's names as before.
> - CLDRConverer now parses regionFormat/gmtZeroFormat/exemplarCity
>
> CLDR provider:
> - CLDRTimeZoneNameProviderImpl is introduced to generate fallback
> names at runtime.
> - If COMPAT provider is present, use it also as a fallback, before the
> last resort (GMT offset format)
>
> Naoto
>
> [1]
> http://www.unicode.org/reports/tr35/tr35-dates.html#Time_Zone_Names
------------------------------
Message: 3
Date: Tue, 24 Apr 2018 17:30:34 -0700
From: joe darcy <joe.darcy at oracle.com>
To: core-libs-dev <core-libs-dev at openjdk.java.net>
Subject: JDK 11 RFR of 8200478: For boxing conversion javac uses
Long.valueOf which does not guarantee caching according to its javadoc
Message-ID: <e51d4670-e0f4-a48c-d23b-cfe1fc588137 at oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Hello,
Please review the patch below to update the specification of
Long.valueOf(long) to require caching on [-128, 127]. The JDK implementation of this functionality has always cached in that region, even though it is not required.
Additionally, please review the corresponding CSR:
??? ??? JDK-8202231: For boxing conversion javac uses Long.valueOf which does not guarantee caching according to its javadoc
Thanks,
-Joe
diff -r f909f09569ca src/java.base/share/classes/java/lang/Long.java
--- a/src/java.base/share/classes/java/lang/Long.java??? Wed Apr 18
21:10:09 2018 -0700
+++ b/src/java.base/share/classes/java/lang/Long.java??? Tue Apr 24
17:25:24 2018 -0700
@@ -1164,10 +1164,8 @@
????? * significantly better space and time performance by caching ????? * frequently requested values.
????? *
-???? * Note that unlike the {@linkplain Integer#valueOf(int) -???? * corresponding method} in the {@code Integer} class, this method -???? * is <em>not</em> required to cache values within a particular -???? * range.
+???? * This method will always cache values in the range -128 to 127,
+???? * inclusive, and may cache other values outside of this range.
????? *
????? * @param? l a long value.
????? * @return a {@code Long} instance representing {@code l}.
------------------------------
Message: 4
Date: Tue, 24 Apr 2018 17:41:33 -0700
From: Brian Burkhalter <brian.burkhalter at oracle.com>
To: joe darcy <joe.darcy at oracle.com>
Cc: core-libs-dev <core-libs-dev at openjdk.java.net>
Subject: Re: JDK 11 RFR of 8200478: For boxing conversion javac uses
Long.valueOf which does not guarantee caching according to its javadoc
Message-ID: <10B34EE4-9BF5-4319-81BE-60FA69257D7D at oracle.com>
Content-Type: text/plain; charset=us-ascii
Hi Joe,
On Apr 24, 2018, at 5:30 PM, joe darcy <joe.darcy at oracle.com> wrote:
> Please review the patch below to update the specification of Long.valueOf(long) to require caching on [-128, 127]. The JDK implementation of this functionality has always cached in that region, even though it is not required.
Looks fine.
> Additionally, please review the corresponding CSR:
>
> JDK-8202231: For boxing conversion javac uses Long.valueOf
> which does not guarantee caching according to its javadoc
Done.
Brian
------------------------------
Message: 5
Date: Tue, 24 Apr 2018 17:53:11 -0700
From: Jonathan Gibbons <jonathan.gibbons at oracle.com>
To: mandy chung <mandy.chung at oracle.com>
Cc: compiler-dev <compiler-dev at openjdk.java.net>, build-dev
<build-dev at openjdk.java.net>, core-libs-dev
<core-libs-dev at openjdk.java.net>
Subject: Re: RFR: 8201274: Launch Single-File Source-Code Programs
Message-ID: <5ADFD177.7050600 at oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
On 04/12/2018 10:20 PM, mandy chung wrote:
>
>
> On 4/13/18 4:15 AM, Jonathan Gibbons wrote:
>> Please review an initial implementation for the feature described in
>> JEP 330: Launch Single-File Source-Code Programs.
>>
>> The work is described in the JEP and CSR, and falls into various parts:
>>
>> * The part to handle the new command-line options is in the native
>> Java launcher code.
>> * The part to invoke the compiler and subsequently execute the code
>> found in the source file is in a new class in the jdk.compiler
>> module.
>> * There are some minor Makefile changes, to add support for a new
>> resource file.
>>
>> There are no changes to javac itself.
>>
>> JEP: http://openjdk.java.net/jeps/330
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8201274
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8201275
>> Webrev: http://cr.openjdk.java.net/~jjg/8201274/webrev.00/
>
> This looks quite good to me. One small comment on the source launcher
> Main class:
>
> 122 } catch (InvocationTargetException e) {
> 123 // leave VM to handle the stacktrace, in the standard manner
> 124 throw e.getTargetException();
> 125 }
> 387 } catch (InvocationTargetException e) {
> 388 // remove stack frames for source launcher
> 389 int invocationFrames = e.getStackTrace().length;
> 390 Throwable target = e.getTargetException();
> 391 StackTraceElement[] targetTrace = target.getStackTrace();
> 392 target.setStackTrace(Arrays.copyOfRange(targetTrace, 0, targetTrace.length - invocationFrames));
> 393 throw e;
> 394 }
>
> This could simply throw target instead of the
> InvocationTargetException and then the main method can propagate the target, if thrown.
>
> Mandy
Mandy,
Yes, but that would require the execute method and its callers to declare that they throw Throwable, or at least Exception. Since the exception is already wrapped, it seems better to propagate the wrapped exception, and to only unwrap it at the last moment.
-- Jon
------------------------------
Message: 6
Date: Wed, 25 Apr 2018 09:33:51 +0800
From: Leo Jiang <li.jiang at oracle.com>
To: "i18n-dev at openjdk.java.net" <i18n-dev at openjdk.java.net>,
"'core-libs-dev'" <core-libs-dev at openjdk.java.net>
Subject: [11] RFR: 8202026 8193552 : ISO 4217 Amendment #165 # 166
Update
Message-ID: <5d50ddec-884c-e78c-4c2c-b9e11d8c3236 at oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi,
Please review the changes to address the ISO 4217 Amendment 165 166 update.
Bug:
https://bugs.openjdk.java.net/browse/JDK-8193552 165
https://bugs.openjdk.java.net/browse/JDK-8202026 166
CR:
http://cr.openjdk.java.net/~ljiang/8202026/webrev.00/
Detail:
#165
From:
MAURITANIA Ouguiya MRO 478 2
To:
MAURITANIA Ouguiya MRU 929 2
#166
From:
VENEZUELA (BOLIVARIAN REPUBLIC OF) Bol?var VEF 937 2
To:
VENEZUELA (BOLIVARIAN REPUBLIC OF) Bol?var Soberano VES 928 2
Thanks,
Leo
End of core-libs-dev Digest, Vol 132, Issue 84
**********************************************
More information about the core-libs-dev
mailing list