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-requ...@openjdk.java.net <core-libs-dev-requ...@openjdk.java.net> Sent: Wednesday, April 25, 2018 7:08 AM To: core-libs-dev@openjdk.java.net Subject: core-libs-dev Digest, Vol 132, Issue 84 Send core-libs-dev mailing list submissions to core-libs-dev@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-requ...@openjdk.java.net You can reach the person managing the list at core-libs-dev-ow...@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 <scolebou...@joda.org> To: core-libs-dev <core-libs-dev@openjdk.java.net> Subject: Re: RFR: JDK-8193877- DateTimeFormatterBuilder throws ClassCastException when using padding Message-ID: <CACzrW9BbZz82oqLPOwof6k=GLxtcGXvuDPoqYMqO2g=zhty...@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.ri...@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 <scolebou...@joda.org> To: core-libs-dev <core-libs-dev@openjdk.java.net> Subject: Re: [11] RFR: 8181157: CLDR Timezone name fallback implementation Message-ID: <caczrw9dokghbsfzc4juy6ym19mgenx0g4gxgqutpaoykdqp...@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.s...@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.da...@oracle.com> To: core-libs-dev <core-libs-dev@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-cfe1fc588...@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.burkhal...@oracle.com> To: joe darcy <joe.da...@oracle.com> Cc: core-libs-dev <core-libs-dev@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-60fa69257...@oracle.com> Content-Type: text/plain; charset=us-ascii Hi Joe, On Apr 24, 2018, at 5:30 PM, joe darcy <joe.da...@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.gibb...@oracle.com> To: mandy chung <mandy.ch...@oracle.com> Cc: compiler-dev <compiler-...@openjdk.java.net>, build-dev <build-...@openjdk.java.net>, core-libs-dev <core-libs-dev@openjdk.java.net> Subject: Re: RFR: 8201274: Launch Single-File Source-Code Programs Message-ID: <5adfd177.7050...@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.ji...@oracle.com> To: "i18n-...@openjdk.java.net" <i18n-...@openjdk.java.net>, "'core-libs-dev'" <core-libs-dev@openjdk.java.net> Subject: [11] RFR: 8202026 8193552 : ISO 4217 Amendment #165 # 166 Update Message-ID: <5d50ddec-884c-e78c-4c2c-b9e11d8c3...@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 **********************************************