Re: RFR: 8256515: javax.xml.XMLEventReader produces incorrect START_DOCUMENT event

On Thu, 19 Nov 2020 16:34:36 GMT, Marius Volkhart wrote: >> @MariusVolkhart I'll work with you getting the fix in and get the test >> running using jtreg. Give me a few moments to reproduce. > > Thanks @jerboaa > > I have a JBS number now: > [8256515](https://bugs.java.com/bugdatabase/view_b

Re: RFR: 8256839: JavaDoc for java.time.Period.negated() method

On Mon, 23 Nov 2020 19:24:51 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Thanks. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1399

Re: RFR: 8247432: Update IANA Language Subtag Registry to Version 2020-09-29

On Fri, 20 Nov 2020 17:55:55 GMT, Naoto Sato wrote: > Hi, > > Please review the changes to the subject issue. This is to incorporate the > latest language subtag registry definition into the JDK. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/

Re: RFR: 8251317: Support for CLDR version 38

On Tue, 17 Nov 2020 23:19:23 GMT, Naoto Sato wrote: > Hi, > > Please review the changes for upgrading the CLDR data to version 38. The vast > majority of the changes are simply the changes in CLDR upstream, and others > are mainly test changes due to the locale data change. Looks good to me.

Re: Filtered XMLStreamReader Exceptions (java.xml)

). Thank you, Mike On Wed, Oct 28, 2020 at 12:52 PM Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi Mike, As you said, creating a bug report would be a good start. If it involves a signature change, it'd need to go through a proper review (CSR) process.

Re: RFR: 8247781: Day periods support [v7]

On Thu, 5 Nov 2020 17:12:11 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the changes for the subject issue. This is to enhance the >> java.time package to support day periods, such as "in the morning", defined >> in CLDR. It will add a new pattern character 'B' and its supporting builder

Re: RFR: 8247781: Day periods support [v5]

On Fri, 30 Oct 2020 11:42:49 GMT, Stephen Colebourne wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed TCK test failures, added the new pattern char description in >> DateTimeFormatter > > Changes requested by

Re: RFR: 8255671: Bidi.reorderVisually has misleading exception messages

On Fri, 30 Oct 2020 22:23:30 GMT, Naoto Sato wrote: > Hi, > > Please review this simple message fix that follows JDK-8255242. > > Naoto Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/973

Re: Filtered XMLStreamReader Exceptions (java.xml)

Hi Mike, As you said, creating a bug report would be a good start. If it involves a signature change, it'd need to go through a proper review (CSR) process. When you are ready to submit a bug report, please make sure to add a test case to illustrate the use case scenario. Thanks, Joe On 10

Re: RFR: 8255086: Update the root locale display names [v2]

On Thu, 22 Oct 2020 23:20:08 GMT, Naoto Sato wrote: > Hi Joe, > > > Do we need a test similar to ISO3166 where display names of Locale.ROOT and > > Locale.US are compared? I see LocaleEnhanceTest.java has references to > > Locale.ROOT and a few selected names were updated. But we don't seem to

Re: RFR: 8255086: Update the root locale display names [v2]

On Wed, 21 Oct 2020 23:38:23 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the changes to the subject issue. The fix replaces the >> obsolete/incorrect English language/region/script display names in the >> COMPAT root locale bundle. The data are derived from CLDR's English names. > > Naot

Re: RFR: 8255086: Update the root locale display names [v2]

On Thu, 22 Oct 2020 21:40:59 GMT, Brent Christian wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Put a newline at the end. > > Marked as reviewed by bchristi (Reviewer). Hi Naoto, Do we need a test similar to ISO3

Re: RFR: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode [v2]

On Sat, 19 Sep 2020 01:36:38 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the fix to JDK-8253321. As in the issue, uninitialized >> (cached) hash code was incorrectly referenced in >> equals() method. Removing it will correct the problem. Also, unrelated to >> the issue, I fixed a paramet

Re: RFR: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode

On Fri, 18 Sep 2020 23:26:39 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to JDK-8253321. As in the issue, uninitialized (cached) > hash code was incorrectly referenced in > equals() method. Removing it will correct the problem. Also, unrelated to the > issue, I fixed a parameter des

Re: RFR: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode

On Fri, 18 Sep 2020 23:26:39 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to JDK-8253321. As in the issue, uninitialized (cached) > hash code was incorrectly referenced in > equals() method. Removing it will correct the problem. Also, unrelated to the > issue, I fixed a parameter des

Re: RFR: 8253153: Mentioning of "hour-of-minute" in java.time.temporal.TemporalField JavaDoc

On Fri, 18 Sep 2020 01:49:09 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/234

Integrated: 8251495: Remove the implNote in the DOM package description added by JDK-8249643

On Wed, 9 Sep 2020 22:56:14 GMT, Joe Wang wrote: > Revert changes made by JDK-8249643, removing the implNote. This pull request has now been integrated. Changeset: 5191f315 Author: Joe Wang URL: https://git.openjdk.java.net/jdk/commit/5191f315 Stats: 25 lines in 1 file chan

Re: RFR: 8220483: Calendar.setTime(Date date) throws NPE with Date date = null

On Mon, 14 Sep 2020 23:15:10 GMT, Naoto Sato wrote: >> Thanks, Lance, >> >>> The change looks fine. I might have created a CSR just to track the doc >>> change. >> >> Yes, and thanks for the CSR review too! > > Thanks, Joe. > >> The change looks fine. However, are other methods in the same si

Re: RFR: 8220483: Calendar.setTime(Date date) throws NPE with Date date = null

On Mon, 14 Sep 2020 22:18:34 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/159

Re: RFR: 8220483: Calendar.setTime(Date date) throws NPE with Date date = null

On Mon, 14 Sep 2020 22:41:55 GMT, Lance Andersen wrote: >> Hi, >> >> Please review this simple doc fix. > > Hi Naoto, > > The change looks fine. I might have created a CSR just to track the doc > change. The change looks fine. However, are other methods in the same situation, e.g. void setT

Re: RFR: 8251495: Clarify DOM documentation

On Thu, 10 Sep 2020 16:16:57 GMT, Joe Wang wrote: >> Marked as reviewed by alanb (Reviewer). > > Remove the implNote from the package description added by JDK-8249643. > The implementation's deviation from the specification was actually wrong. > It incorrectly identified

Re: RFR: 8251495: Clarify DOM documentation

On Thu, 10 Sep 2020 05:45:59 GMT, Alan Bateman wrote: >> Revert changes made by JDK-8249643, removing the implNote. > > Marked as reviewed by alanb (Reviewer). Remove the implNote from the package description added by JDK-8249643. The implementation's deviation from the specification was actuall

Re: RFR: 8251495: Clarify DOM documentation

On Wed, 9 Sep 2020 23:15:58 GMT, Lance Andersen wrote: >> Revert changes made by JDK-8249643, removing the implNote. > > Based on the spec review and follow-on discussions, this change makes sense An implNote was added to the DOM's package description in an attempt to document the JDK implement

RFR: 8251495: Clarify DOM documentation

Revert changes made by JDK-8249643, removing the implNote. - Commit messages: - 8251495: Clarify DOM documentation Changes: https://git.openjdk.java.net/jdk/pull/100/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=100&range=00 Issue: https://bugs.openjdk.java.net/brow

Re: RFR: 8252740: java/util/Properties/LoadAndStoreXMLWithDefaults.java fails after JDK-8252354

On 9/3/20 2:51 AM, Alan Bateman wrote: On 03/09/2020 10:22, jiefu(傅杰) wrote: Hi all, JBS:    https://bugs.openjdk.java.net/browse/JDK-8252740 Webrev: http://cr.openjdk.java.net/~jiefu/8252740/webrev.00/ After JDK-8252354, properties whose keys or values are none strings won't be skipped an

RFR [16/java.xml] 8252354 : Properties :: storeToXML method does not throw ClassCastException when supplied non strings.

Please review a fix to report CCE when keys or values are not Strings as specified. JBS: https://bugs.openjdk.java.net/browse/JDK-8252354 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8252354/webrev/ Thanks, Joe

Re: RFR: 8252552: DecimalFormat javadoc contains HTML tags in example code

+1 Or, maybe is not needed for code comments -Joe On 8/31/20 10:14 AM, Lance Andersen wrote: Hi Naoto, Looks OK to me. Best Lance On Aug 31, 2020, at 12:22 PM, Naoto Sato wrote: Hello, Please review this simple javadoc fix to: https://bugs.openjdk.java.net/browse/JDK-8252552 The prop

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

On 8/26/20 4:00 AM, Alan Bateman wrote: On 21/08/2020 19:23, Joe Wang wrote: Pelase review a patch to add the missing @return, @throws, @param statements in the java.xml package (excluding the DOM component). JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 CSR: https

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

e warning categories by default? /Erik On 2020-08-25 11:47, Joe Wang wrote: Cc-ing build-...@openjdk.java.net (makefile change: make/Docs.gmk) Updated webrev: http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_04/ Thanks Roger! Please see inline comments. On 8/25/20 8:09 AM, Roger Riggs wrot

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

he javadoc, using @code around true, false, missing periods at the end of first sentences, etc.  But that's a different task. Created a bug to keep track of this: https://bugs.openjdk.java.net/browse/JDK-8252328 Thanks, Joe On 8/24/20 5:44 PM, Joe Wang wrote: Hi all,  adding Ro

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_03/ -Joe On 8/24/20 3:27 PM, Lance Andersen wrote: Looks OK Joe On Aug 24, 2020, at 5:44 PM, Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi all,  adding Roger's comment for the make file to webrev_02 (the only change to we

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

i Joe, This looks OK. On Aug 21, 2020, at 2:23 PM, Joe Wang wrote: Pelase review a patch to add the missing @return, @throws, @param statements in the java.xml package (excluding the DOM component). JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 CSR: https://bugs.openjdk.java.net/brows

RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

Pelase review a patch to add the missing @return, @throws, @param statements in the java.xml package (excluding the DOM component). JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 CSR: https://bugs.openjdk.java.net/browse/JDK-8251995 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8251561

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

sults from the method (cnfMultiplier and whether to return immediately for no-placeholder cases). https://cr.openjdk.java.net/~naoto/8251499/webrev.02/ Naoto On 8/17/20 11:52 PM, Joe Wang wrote: Hi Naoto, Looks good overall. One nit, blocks 1633-1639 and 1642-1649 may share a common private m

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

: http://cr.openjdk.java.net/~naoto/8251499/webrev.01/ I added a new test case (COMPACT_PATTERN14), which actually is extracted from CLDR 38 Somali locale that demonstrates the issue. I'd appreciate your further review. Naoto On 8/14/20 6:21 PM, Joe Wang wrote: Hi Naoto, Looks good

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

Hi Naoto, Looks good to me. While a negative divisor representing no zeros is newly introduced, the "divisor > 0" checks seem to have always been beneficial.  I had to count the number of ""s in COMPACT_PATTERN13 :-) Have a great weekend! Joe On 8/14/2020 3:20 PM, naoto.s...@oracle.com wrot

Re: RFR 8251208: Fix javadoc warnings in java.sql and java.sql.rowsets

o fix the typos (missed I guess due to lack of coffee this am ;-) ) http://cr.openjdk.java.net/~lancea/8251208/webrev.01/index.html is the updated webrev Best Lance On Aug 14, 2020, at 1:20 PM, Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi Lance, Looks good to me overall. * **

Re: RFR 8251208: Fix javadoc warnings in java.sql and java.sql.rowsets

Hi Lance, Looks good to me overall. * **Minor typos in the CSR:* Address the Fix "no comment" warnings in java.sql and java.sql.rowsetsgenerated by javadoc -Xdoclint     ^ remove Fix  ^ missing a spacebetween rowsetsgenerated java.sql and ja

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

. Otherwise, looks good. Roger On 8/7/20 3:13 PM, Joe Wang wrote: Hi Naoto, Why not :-). I kept it to go with equals(). Removing both now. Added a reference comparison as did in the Impl class. http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev_04/ Joe On 8/7/20 10:50 AM, naoto.s

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Thanks Naoto! Joe On 8/7/20 12:45 PM, naoto.s...@oracle.com wrote: Looks good. Thank you for the change. Naoto On 8/7/20 12:13 PM, Joe Wang wrote: Hi Naoto, Why not :-). I kept it to go with equals(). Removing both now. Added a reference comparison as did in the Impl class. http

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

XMLGregorianCalendarImpl.hashCode() just calls super.hashCode(), could it be removed entirely? Also XMLGregorianCalendarImpl.equals() seems to do the same thing as its parent. Could it also be removed? Naoto On 8/7/20 10:16 AM, Joe Wang wrote: Naoto, Roger, Sorry have to come back with another iteration. It looks like

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

pl and XMLGregorianCalendar. webrev_03: http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev_03/ Thanks, Joe On 8/6/20 1:51 PM, Joe Wang wrote: Thanks Naoto, Roger!  I'll prepare push based on version 01. Best, Joe On 8/6/20 1:29 PM, Roger Riggs wrote: +1 On 8/6/20 4:18 PM, naoto.s...

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

rence in two objects. So fine with the version 01. Naoto On 8/6/20 12:18 PM, Joe Wang wrote: Thanks Naoto, Roger for the review! For Naoto's concern about the equals method using EonAndYear and FractionalSecond, I did cut corners using the all int getXXX method. The rational was: it fu

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

ly for the year? Naoto On 8/5/20 4:37 PM, Joe Wang wrote: Hello, Please review a fix for the hashCode generation. JBS: https://bugs.openjdk.java.net/browse/JDK-8246816 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev/ Thanks, Joe

RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Hello, Please review a fix for the hashCode generation. JBS: https://bugs.openjdk.java.net/browse/JDK-8246816 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev/ Thanks, Joe

Re: RFR 8233048: WeekFields.ISO is not a singleton

+1 -Joe On 7/30/20 2:53 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8233048 The proposed chageset is located at: https://cr.openjdk.java.net/~naoto/8233048/webrev.00/ java.time.temporal.WeekFields.ISO is instan

RFR [16/java.xml] 8250638: Address reliance on default constructors in java.xml

Hello, Please review a change to remove reliance of default constructors in java.xml. JBS: https://bugs.openjdk.java.net/browse/JDK-8250638 CSR: https://bugs.openjdk.java.net/browse/JDK-8250800 Patch: diff --git a/src/java.xml/share/classes/org/xml/sax/HandlerBase.java b/src/java.xml/share/

RFR [16/java.xml] 8249643: Clarify DOM documentation

Hello, Please review a doc clarification for the org.w3c.dom package. JBS: https://bugs.openjdk.java.net/browse/JDK-8249643 CSR: https://bugs.openjdk.java.net/browse/JDK-8249904 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8249643/webrev_02/ Thanks, Joe

Re: RFR: 8247546: Pattern matching does not skip correctly over supplementary characters

Hi Naoto, The new webrev looks good. Best, Joe On 7/27/20 7:11 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review! On 7/27/20 6:04 PM, Joe Wang wrote: Hi Naoto, Looks good to me, using the correct supplementary-aware implementation is an improvement for the impl to handle

Re: RFR: 8247546: Pattern matching does not skip correctly over supplementary characters

Hi Naoto, Looks good to me, using the correct supplementary-aware implementation is an improvement for the impl to handle the situation. A note (probably a workaround) for the user's case, to replace invalid surrogate characters, he could have used Unicode escape sequences [\ud800-\udfff] in

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

/20 4:20 PM, Joe Wang wrote: Hi Naoto, The change looks good to me. "supLower" is indeed super slow :-) The only minor comment I have is that the compareCodePointCI method performs toUpperCase unconditionally. That's not a problem for the regular case, where a check on cp1 == cp

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Naoto, The change looks good to me. "supLower" is indeed super slow :-) The only minor comment I have is that the compareCodePointCI method performs toUpperCase unconditionally. That's not a problem for the regular case, where a check on cp1 == cp2 (line 337) is done prior to the method ca

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

On 7/20/2020 8:58 PM, naoto.s...@oracle.com wrote: The short-cut worked well. There's maybe a further optimization we could do to rid us of the performance concern (or the need to run additional performance tests). Consider the cases where strings in comparison don't contain any sup characte

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

ava.net/~naoto/8248655.8248434/webrev.04/ Naoto On 7/20/20 2:39 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your comments. On 7/20/20 11:20 AM, Joe Wang wrote: Hi Naoto, StringUTF16: line 384 - 388 seem unnecessary since you'd only get there if 389:isHighSurrogate is n

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Naoto, StringUTF16: line 384 - 388 seem unnecessary since you'd only get there if 389:isHighSurrogate is not true. But more importantly, StringUTF16 has existing method "codePointAt" you may want to consider instead of adding a new method. Comparing to the base benchmark: StringCompareToI

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

2020, at 3:32 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your review. On 7/15/20 10:57 AM, Joe Wang wrote: Hi Naoto, In StringUTF16.java, if one is isHighSurrogate and the other not, you may quickly return without going through the rest of the process, probably not significant as c

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Naoto, In StringUTF16.java, if one is isHighSurrogate and the other not, you may quickly return without going through the rest of the process, probably not significant as cp1 and cp2 and/or u1 and u2 won't be equal anyways. But it could skip a couple of toCodePoint/toUpperCase/toLowerCase

Re: RFR 824767: Incorrect class name displayed in DriverManager trace output

+1 Joe On 7/14/20 8:41 AM, Lance Andersen wrote: Hi all, Please review this trivial fix for https://bugs.openjdk.java.net/browse/JDK-8247677 , which addresses a couple of typos where the driver class name was not included in the trace output

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

On 7/13/2020 9:04 PM, naoto.s...@oracle.com wrote: Hi Joe, On 7/13/20 7:28 PM, Joe Wang wrote: On 7/13/2020 7:01 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your review. On 7/13/20 3:55 PM, Joe Wang wrote: Hi Naoto, Would it make sense to provide an additional test using

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

On 7/13/2020 7:01 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your review. On 7/13/20 3:55 PM, Joe Wang wrote: Hi Naoto, Would it make sense to provide an additional test using the public APIs similar to the one provided in the bug report? I'm sure yours is correct and c

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

Hi Naoto, Would it make sense to provide an additional test using the public APIs similar to the one provided in the bug report? I'm sure yours is correct and covers more cases than the original, but it would be nice to have an actual use case and use the public APIs. The report showed it was

Re: RFR[15/java.xml] Re: RFR [16/java.xml] 8248486: SafeThread illegal access to java.lang private fields should be removed

Thanks Mandy. Pushed to the mainline/16 repo. -Joe On 7/13/20 10:13 AM, Mandy Chung wrote: Looks okay.  Same as Alan's comment,  I don't see there is any urgency to fix this in 15.   Fixing it in 16 is fine. Mandy On 7/12/20 3:21 PM, Joe Wang wrote: Hi all, Alan updated

RFR[15/java.xml] Re: RFR [16/java.xml] 8248486: SafeThread illegal access to java.lang private fields should be removed

Hi all, Alan updated the bug to indicate it fails since JDK 9. Given we still have a couple days for JDK 15, I've rebased the patch to the jdk15 repo and would like to check in the patch into JDK 15 instead (and let it be sync-ed to 16). Here's the webrev for JDK 15: http://cr.openjdk.java.n

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

On 6/30/2020 7:44 PM, Stuart Marks wrote: An unconventional patch over an unusual hashcode/equals implementation is a bit too controversial. I'd like to propose a new patch that focus on the problem at hand, that is re-setting the opcode causes the item in HashSet to get lost because of th

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

Hi all, (had a nice hike today, and asked the beautiful Lake 22 ;-)) An unconventional patch over an unusual hashcode/equals implementation is a bit too controversial. I'd like to propose a new patch that focus on the problem at hand, that is re-setting the opcode causes the item in HashSet t

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

for the objects. In that case you can implement hashcode uncached and aligned with equals. (And not use it) Gruss Bernd -- http://bernd.eckenfels.net ---- *Von:* Joe Wang *Gesendet:* Friday, June 26, 2020 8:29:41 AM *An:* Bernd E

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

On 6/26/2020 12:46 PM, Stuart Marks wrote: On 6/25/20 4:53 PM, Joe Wang wrote: Please review a fix to a BCEL regression. At issue was the addition of hashCode() method to Instruction.java in BCEL 6.0. This hashCode() method was implemented to return the instruction's opcode

Re: RFR; [docs,15] JDK-8248060 small HTML issues in java.xml package-info.java files

Looks good to me. The upstream SAX hasn't evolved for a long time (and there's no plan to do so). I think we're okay to make small changes. For the 2nd case, I think it's good enough since there are links to the classes at the beginning of the sentence -- a minor inconvenience (vs a straight l

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

(o2) => o1.hashCode() == o2.hashCode(). I see equals() method delegates to a global InstructionComparator instance which can even change. Suppose it does not change. But what does such InstructionComparator compare? opcode (among other things)? Regards, Peter On Fri, 26 Jun 2020, 01:56 Jo

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

ttp://bernd.eckenfels.net Von: core-libs-dev im Auftrag von Joe Wang Gesendet: Friday, June 26, 2020 1:53:08 AM An: Core-Libs-Dev Betreff: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0 Hi, Please review a fix to a BCEL regression. At issue was

RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

Hi, Please review a fix to a BCEL regression. At issue was the addition of hashCode() method to Instruction.java in BCEL 6.0. This hashCode() method was implemented to return the instruction's opcode that unfortunately is mutable. The problem hasn't showed up until the code path led to calls

Re: RFR 15 8247521: (test) jdk/test/lib/hexdump/HexPrinterTest.java fails on windows

+1 -Joe On 6/15/2020 7:09 AM, Roger Riggs wrote: Please review a test change of the test library HexPrinter. It should be comparing against the System.lineSeparator. diff --git a/test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java b/test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java

Re: RFR: 8245448: Remove minimum 4 digit requirement from Year.parse()

+1.  The original poster was probably working on writing history (all the way back to before 1000), I assume ;-)  This enhancement would certainly make his job a bit easier. -Joe On 6/12/2020 11:17 AM, Roger Riggs wrote: Hi Naoto, Yes, looks good. Roger On 6/12/20 1:06 PM, Lance Andersen

Re: RFR 8207936: TestZipFile.java can fail with an OOM error

Looks good to me, Lance. -Joe On 6/10/2020 2:47 PM, Lance Andersen wrote: Hi, Please review the fix for JDK-8207936, which addresses the OOM error when running the manual test TestZipFile.java. The fix also removes the call to "getMetaInfEntryNames” as this method was removed as part of th

Re: [15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

is not depending on identity. Roger On 6/9/20 3:10 PM, naoto.s...@oracle.com wrote: Hi Joe, On 6/9/20 11:54 AM, Joe Wang wrote: Looks fine to me for the implementation of the method. Thank you for the review! It's interesting to observe that the with methods, as demonstrated i

Re: RFR [15/java.xml] 8237456: Transform filtered through SAX filter mishandles character entities

tatic org.testng.Assert.*; — No need to change but typically which get suggested when you run Inspect Code Best Lance On Jun 9, 2020, at 1:54 AM, Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi, Please review a fix to a regression that resulted in invalid output in a transform

Re: [15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

On 6/9/2020 10:01 AM, naoto.s...@oracle.com wrote: Hi Roger, Thanks for the review. On 6/9/20 8:52 AM, Roger Riggs wrote: Hi Naoto, Since the default locale is being changed even briefly, the test should be run in /othervm. Add:  @run testng/othervm ... All java/time/test/java/time/for

RFR [15/java.xml] 8237456: Transform filtered through SAX filter mishandles character entities

Hi, Please review a fix to a regression that resulted in invalid output in a transform that contains entity references. The regression was caused by a JDK9 patch [1]. Adding back the conditional checks fixed the issue. JBS: https://bugs.openjdk.java.net/browse/JDK-8237456 webrevs: http://cr.o

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

match or not does not matter in this test case, so I think we can just eliminate these exact match try loops. I will remove them and do some sniff testing on it. Naoto On 6/1/20 5:58 PM, Joe Wang wrote: Hi Naoto, The patch looks good to fix the failure. I'm just curious whether the 100

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Hi Naoto, The patch looks good to fix the failure. I'm just curious whether the 100-time comparison is necessary because of the existence of this assertion outside the loop that allowed the test to pass if the different was within a certain period of time. None of the tests had commented on t

Re: [15] RFR: 8239480: Support for CLDR version 37

the lines stretched very long <https://cr.openjdk.java.net/~naoto/8239480/webrev.00/src/jdk.localedata/share/legal/cldr.md.html>. But again, the format of the md files might not be that important. Your call. -Joe Naoto On 5/20/20 6:51 PM, Joe Wang wrote: Looks good to me. A minor com

Re: [15] RFR: 8239480: Support for CLDR version 37

Looks good to me. A minor comment: in the license files, Terms of Use lost its original format (e.g. headings and numbering), lines are also super long. But I think we've seen them in the previous update, it's up to you whether you'd want to reformat th

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

Hi Naoto, Thanks for the explanation. I agree on the needs for maintaining backward compatibility.  The patch looks good me then. Best, Joe On 5/20/2020 4:49 PM, naoto.s...@oracle.com wrote: Hi Joe, thanks for the review. On 5/20/20 4:10 PM, Joe Wang wrote: Hi Naoto, I don't seem t

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

Hi Naoto, I don't seem to see the DateFormat class or the getDateInstance methods specify how errors may be handled (or logged). Is that stated somewhere else?  In other cases, I see that you've changed it to throw ServiceConfigurationError, that looks to me may be better than a log as a conf

Re: RFR [15] 8245231 Javadoc for the readObject methods needs to be updated (8244342 followup)

As it appears, I was missing a comma in the header as well (see https://bugs.openjdk.java.net/browse/JDK-8245238) The webrev is updated with the comma added (to PredicatedNodeTest.java) http://cr.openjdk.java.net/~joehw/jdk15/8245231/webrev/ Thanks, Joe On 5/18/2020 1:53 PM, Joe Wang wrote

Re: RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.

Thanks Mark. -Joe On 5/18/2020 1:49 PM, mark sheppard wrote: Hi Joe, all good I think. 👍 regards Mark *From:* Joe Wang *Sent:* Monday 18 May 2020 19:36 *To:* mark sheppard ; core-libs-dev@openjdk.java.net *Cc

Re: RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.

ct in each LocPathIterator PredicatedNodeTest, and UnionPathIterator ? regards Mark *From:* core-libs-dev on behalf of Joe Wang *Sent:* Friday 15 May 2020 17:54 *To:* core-libs-dev@openjdk.java.net *Subject:* RFR [15] 82

RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.

Hi, Please review a fix for the compilation warnings. Thanks Roger for the detailed instructions! If you could verify the fix with the work-in-progress processor, that would be great too. Regular build and test passed. https://bugs.openjdk.java.net/browse/JDK-8244342 http://cr.openjdk.java.n

Re: RFR [15] 8245111: Update doc comments for improved processing by the Standard Doclet

+1 Cheers, Joe On 5/15/2020 7:00 AM, Daniel Fuchs wrote: Hi Pavel, This looks good to me - but English is not my native language ;-) cheers, -- daniel On 15/05/2020 13:35, Pavel Rappo wrote: Hello, Please review this trivial change for https://bugs.openjdk.java.net/browse/JDK-8245111:

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

+1 Joe On 5/14/2020 2:14 AM, Daniel Fuchs wrote: Hi Fernando, That looks good to me. best regards, -- daniel On 12/05/2020 11:46, Fernando Guallini wrote: Thanks for your comments. Below you can find a new web rev version that includes a test description in a comment: http://cr.openjdk.ja

Re: [15] RFR: 8239383: Support for Unicode 13.0

On 5/13/2020 3:24 PM, naoto.s...@oracle.com wrote: Thanks for the review, Joe! On 5/13/20 2:58 PM, Joe Wang wrote: +1 They are from the reference files, but it's interesting to note the ranges in Blocks.txt (that Character.java follows) for Tangut Supplement (18D00..18D8F) an

Re: [15] RFR: 8239383: Support for Unicode 13.0

+1 They are from the reference files, but it's interesting to note the ranges in Blocks.txt (that Character.java follows) for Tangut Supplement (18D00..18D8F) and CJK Unified Ideographs Extension G (3..3134F) are slightly different from those in the online pdf files (http://www.unicode.or

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

path "/patches/java.xml/..”. To sum up, this test creates a temporary java.xml patch that needs to be ignored after compiling *DocumentImpl. *I am using —patch-module because it’s more flexible than @compile/module * * Hope I explained myself! This may be a question for Joe Wang but I&#x

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

Hi Fernando, While you add the comment (about compile/run), you may want to update the original comment about the test as well as it mentioned "see run.sh" that is now gone. A minor comment on the header:  a space between the years would be good, that is "2014,2020," -> 2014, 2020, -Joe O

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

email, Unicode Extensions are correctly dealt in Chronology.ofLocale()/DecimalStyle.of() methods indirectly, so I believe no doc change is warranted. Ok, thanks for the explanation. -Joe Naoto On 5/6/20 11:32 PM, Joe Wang wrote: Hi Naoto, The Javadoc states: If the locale contains the

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

On 5/7/2020 1:14 AM, Stephen Colebourne wrote: On Thu, 7 May 2020 at 07:38, Joe Wang wrote: The Javadoc states: If the locale contains the "ca" (calendar), "nu" (numbering system), "rg" (region override), and/or "tz" (timezone) Unicode extensio

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

Hi Naoto, The Javadoc states:     If the locale contains the "ca" (calendar), "nu" (numbering system), "rg" (region override), and/or "tz" (timezone) Unicode extensions, the chronology, numbering system and/or the zone are overridden. If you remove the two statements that check whether the sp

Re: [15] RFR: 8244459: Optimize the hash map size in LocaleProviderAdapters

On 5/5/2020 2:08 PM, naoto.s...@oracle.com wrote: Thanks, all. I didn't see this coming! +1, just when one might think it was just a minor tweak... ;-) If I understand the discussion correctly, Peter's suggestion is the most optimal (Mark, your formula produces 1 for the expected size is

Re: [15] RFR: 8244459: Optimize the hash map size in LocaleProviderAdapters

Hi Naoto, It seems you've missed the parentheses in : Set tagset = new HashSet<>(tokens.countTokens() * 4 + 2 / 3); vs Set tagset = new HashSet<>((tokens.countTokens() * 4 + 2) / 3); -Joe On 5/5/2020 11:01 AM, naoto.s...@oracle.com wrote: And here is the fix. Please review. http://cr.openjdk.

Re: RFR[8183266] - [TESTBUG]Add test to cover XPathEvaluationResult.XPathResultType.getQNameType method

Hi Fernando, Thanks for the update. Here's the updated webrev. The previous webrev was moved to webrev01. http://cr.openjdk.java.net/~joehw/jdk15/8183266/webrev02/ This change looks good to me. Best, Joe On 4/29/2020 9:38 AM, Joe Wang wrote: Hi Fernando, Thanks for adding covera

<    1   2   3   4   5   6   7   8   9   10   >