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
**********************************************

Reply via email to