RFR JDK-8187910: Charset MS950_HKSCS not supported in JDK 9

2017-11-29 Thread Xueming Shen
Hi Please help review the change for JDK-8187910. issue: https://bugs.openjdk.java.net/browse/JDK-8187910 webrev: http://cr.openjdk.java.net/~sherman/8187910/webrev The change is to add charset MS950_HKSCS into sun.nio.cs/"standard charset provider" on Windows platform. It appears it is being

Re: [10] RFR 6354947: [Fmt-*] DecimalFormat ignores FieldPosition settings on input, contrary to Javadocs

2017-11-29 Thread Nishit Jain
Thanks Naoto, Roger for the review. > the summary of the issue does not reflect the issue and implies the implementation is wrong; but it is the javadoc that need clarification. It is probably because when this bug was filed, it was not clear whether implementation will be changed as per javado

Re: RFR(s): 8160406: Collection.toArray() spec should be explicit about returning precisely an Object[]

2017-11-29 Thread Martin Buchholz
Wordsmithing is hard. Which may be why I never tried to improve these specs decades ago. I would make one change. + * @param the static component type of the array to contain the collection Just drop runtime/static * @param the component type of the array to contain the collection t

Re: RFR(s): 8160406: Collection.toArray() spec should be explicit about returning precisely an Object[]

2017-11-29 Thread Stuart Marks
On 11/29/17 5:20 PM, Martin Buchholz wrote: Thanks.  This looks good, and finishes the effort started 12 years ago. Apologies for not having made this spec change myself years ago. I tried to find a less JLSese way to wordsmith it.  Nearby spec talks about the "runtime type" of the array. M

Re: RFR(s): 8160406: Collection.toArray() spec should be explicit about returning precisely an Object[]

2017-11-29 Thread Martin Buchholz
Thanks. This looks good, and finishes the effort started 12 years ago. Apologies for not having made this spec change myself years ago. I tried to find a less JLSese way to wordsmith it. Nearby spec talks about the "runtime type" of the array. Maybe s/component/runtime component/ but I'm not su

Re: RFR(s): 8160406: Collection.toArray() spec should be explicit about returning precisely an Object[]

2017-11-29 Thread Paul Sandoz
+1 Paul. > On 29 Nov 2017, at 16:18, Stuart Marks wrote: > > Hi all, > > Please review this small spec change / clarification regarding > Collection.toArray(). The runtime type of the array returned has always been > intended to be exactly Object[] and not an array of some subtype. This > r

RFR(s): 8160406: Collection.toArray() spec should be explicit about returning precisely an Object[]

2017-11-29 Thread Stuart Marks
Hi all, Please review this small spec change / clarification regarding Collection.toArray(). The runtime type of the array returned has always been intended to be exactly Object[] and not an array of some subtype. This requirement is actually implied by the spec already, but in the wrong place

Re: RFR: JDK-8189611: JarFile versioned stream and real name support

2017-11-29 Thread Martin Buchholz
(How hard is it to rename that hotspot intrinsic to isAscii?) On Wed, Nov 29, 2017 at 4:03 PM, Martin Buchholz wrote: > +private static boolean isASCII(byte[] ba, int off, int len) {+ > for (int i = off; i < off + len; i++) {+if (ba[i] < 0)+ > return false

Re: RFR: JDK-8189611: JarFile versioned stream and real name support

2017-11-29 Thread Martin Buchholz
+private static boolean isASCII(byte[] ba, int off, int len) {+ for (int i = off; i < off + len; i++) {+if (ba[i] < 0)+return false;+}+return true;+} I see that StringCoding has @HotSpotIntrinsicCandidate public static boolean hasNeg

Re: RFR 8186961 Class.getFields() does not return fields of previously visited super interfaces/classes.

2017-11-29 Thread Claes Redestad
Hi Paul, it seems you'll effectively skip processing of the last interface of c in the new code - is this intentional? 3049 Field[] iFields = null; 3050 for (Class c : getInterfaces()) { 3051 if (iFields != null && iFields.length > 0) { ... 3060 } 3061

Re: RFR(m): 8177290 add copy factory methods for unmodifiable List, Set, Map

2017-11-29 Thread Stuart Marks
On 11/29/17 10:09 AM, Martin Buchholz wrote: Guava's "immutable collections" are very popular https://github.com/google/guava/wiki/ImmutableCollectionsExplained and it's not a good idea to fight their advertised notion of "immutable". No generic container class can promise s'marks-style immut

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Vladimir Kozlov
Yes, it was my limited understanding what --limit-modules is. We should not add modules "under cover" when --limit-modules is used. User should known all required modules *explicitly* to create correct image with jlink based on result of runs with --limit-modules flag. Since it is limited set o

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread mandy chung
Vladimir and I discussed this offline. java --limit-modules should have the same set of observable modules as running on an image that is created with jlink.  When running with -XX:+UseJVMCICompiler on a runtime image with just java.base, it will fail in the same way as running with java --limi

Re: RFR (JDK10/JAXP) 8191938: Fix lint warnings in JAXP repo: a few Deprecation warrnings and enable -Xlint:all

2017-11-29 Thread Jonathan Gibbons
Joe, I presume javadoc is OK with this not-quite-a-doc-comment-tag, when you do "make docs" ... -- Jon On 11/29/2017 10:58 AM, Joe Wang wrote: Hi Joe, I moved the LastModified to the bottom of the class comment block. Please let me know what you think: http://cr.openjdk.java.net/~joehw/j

Re: RFR: jsr166 jdk10 integration wave 6

2017-11-29 Thread Doug Lea
On 11/29/2017 01:43 PM, Paul Sandoz wrote: > SubmissionPublisher > -- > > 1004 // Order-sensitive field declarations > > Is this still relevant Not really; layout is now discussed in class-level doc. Removed. Thanks. > > 81 * A single SubmissionPublisher may be shared among multiple >

Re: [10] RFR: 8186441: Change of behavior in the getMessage () method of the SOAPMessageContextImpl class

2017-11-29 Thread Lance Andersen
Hi Aleks, The changes look reasonable Best Lance > On Nov 29, 2017, at 2:48 PM, Aleks Efimov wrote: > > https://bugs.openjdk.java.net/browse/JDK-8159058 >

RFR 8186961 Class.getFields() does not return fields of previously visited super interfaces/classes.

2017-11-29 Thread Paul Sandoz
Hi, Please review: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186961-iface-static-fields-get/webrev/ There is a bug lurking, perhaps for a while, where diamond patterns for interface hierarchies can

Re: [10] RFR: 8186441: Change of behavior in the getMessage () method of the SOAPMessageContextImpl class

2017-11-29 Thread Aleks Efimov
Thank you Lance! Best, Aleksei On 11/29/2017 08:12 PM, Lance Andersen wrote: Hi Aleks, The changes look reasonable Best Lance On Nov 29, 2017, at 2:48 PM, Aleks Efimov > wrote: https://bugs.openjdk.java.net/browse/JDK-8159058

Re: RFR (JDK10/JAXP) 8191938: Fix lint warnings in JAXP repo: a few Deprecation warrnings and enable -Xlint:all

2017-11-29 Thread Joe Wang
Thanks Joe, Roger! On 11/29/17, 11:49 AM, Roger Riggs wrote: +1 On 11/29/2017 2:45 PM, joe darcy wrote: Hi Joe, The new version looks fine to me; thanks, -Joe On 11/29/2017 10:58 AM, Joe Wang wrote: Hi Joe, I moved the LastModified to the bottom of the class comment block. Please let me

Re: RFR (JDK10/JAXP) 8191938: Fix lint warnings in JAXP repo: a few Deprecation warrnings and enable -Xlint:all

2017-11-29 Thread Roger Riggs
+1 On 11/29/2017 2:45 PM, joe darcy wrote: Hi Joe, The new version looks fine to me; thanks, -Joe On 11/29/2017 10:58 AM, Joe Wang wrote: Hi Joe, I moved the LastModified to the bottom of the class comment block. Please let me know what you think: http://cr.openjdk.java.net/~joehw/jdk10/

[10] RFR: 8186441: Change of behavior in the getMessage () method of the SOAPMessageContextImpl class

2017-11-29 Thread Aleks Efimov
Hi, Can I, please, ask for assistance with reviewing the JDK-8186441 fix [1]? Problem description: In JDK9 we've fixed SAAJ bug in JDK:     https://bugs.openjdk.java.net/browse/JDK-8159058 And this fix caused JDK-8186441 regression that resulted in breaking the namespace definitions in SOAP ele

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Jiangli Zhou
> On Nov 29, 2017, at 9:27 AM, Vladimir Kozlov > wrote: > > On 11/29/17 4:37 AM, Alan Bateman wrote: >> On 29/11/2017 11:26, David Holmes wrote: >>> >>> The current approach basically prevents anyone using JVMCI from shooting >>> themselves in the foot by setting --limit-modules in a way that

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Jiangli Zhou
> On Nov 29, 2017, at 9:27 AM, Vladimir Kozlov > wrote: > > On 11/29/17 4:37 AM, Alan Bateman wrote: >> On 29/11/2017 11:26, David Holmes wrote: >>> >>> The current approach basically prevents anyone using JVMCI from shooting >>> themselves in the foot by setting --limit-modules in a way that

Re: RFR (JDK10/JAXP) 8191938: Fix lint warnings in JAXP repo: a few Deprecation warrnings and enable -Xlint:all

2017-11-29 Thread joe darcy
Hi Joe, The new version looks fine to me; thanks, -Joe On 11/29/2017 10:58 AM, Joe Wang wrote: Hi Joe, I moved the LastModified to the bottom of the class comment block. Please let me know what you think: http://cr.openjdk.java.net/~joehw/jdk10/8191938/webrev/index.html Thanks, Joe On 11

Re: RFR (JDK10/JAXP) 8191938: Fix lint warnings in JAXP repo: a few Deprecation warrnings and enable -Xlint:all

2017-11-29 Thread Joe Wang
Hi Joe, I moved the LastModified to the bottom of the class comment block. Please let me know what you think: http://cr.openjdk.java.net/~joehw/jdk10/8191938/webrev/index.html Thanks, Joe On 11/28/17, 10:19 AM, joe darcy wrote: Hi Joe, The code changes look fine, but the copyright blocks sh

Re: RFR: jsr166 jdk10 integration wave 6

2017-11-29 Thread Paul Sandoz
> On 27 Nov 2017, at 20:19, Martin Buchholz wrote: > > http://cr.openjdk.java.net/~martin/webrevs/openjdk10/jsr166-integration/overview.html > > > > Thanks to Dávid Karnok and Pavel Rappo for help with Su

Re: RFR: 8189102: All tools should support -?, -h and --help

2017-11-29 Thread Robert Field
Thumbs up for JShell changes. -Robert > On Nov 28, 2017, at 3:16 AM, Lindenmaier, Goetz > wrote: > > Hi, > > I uploaded a new webrev: > http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.04/ > > This includes the changes > - to jshell requested by Robert > - to the test as

Re: RFR(m): 8177290 add copy factory methods for unmodifiable List, Set, Map

2017-11-29 Thread Martin Buchholz
Guava's "immutable collections" are very popular https://github.com/google/guava/wiki/ImmutableCollectionsExplained and it's not a good idea to fight their advertised notion of "immutable". No generic container class can promise s'marks-style immutability (until valhalla perhaps?) so it's not that

Re: RFR: JDK-8189611: JarFile versioned stream and real name support

2017-11-29 Thread Xueming Shen
On 11/27/17, 5:43 PM, mandy chung wrote: On 11/20/17 6:58 PM, Xueming Shen wrote: http://cr.openjdk.java.net/~sherman/8189611/webrev Just passing comments. 140 * that {@link getName()} returns. should be #getName(). There are a couple other @link missing #. src/java.base/share/classe

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Vladimir Kozlov
On 11/29/17 4:37 AM, Alan Bateman wrote: On 29/11/2017 11:26, David Holmes wrote: The current approach basically prevents anyone using JVMCI from shooting themselves in the foot by setting --limit-modules in a way that excludes the jdk.internal.vm.compiler module. In essence we want to ensure

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Vladimir Kozlov
Hi Alan, On 11/29/17 1:05 AM, Alan Bateman wrote: On 28/11/2017 23:51, Vladimir Kozlov wrote: http://cr.openjdk.java.net/~kvn/8191788/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8191788 This is redo of JDK-8190975 [1] fix which added jdk.internal.vm.compiler to tests which have --limit

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread mandy chung
On 11/29/17 4:37 AM, Alan Bateman wrote: On 29/11/2017 11:26, David Holmes wrote: The current approach basically prevents anyone using JVMCI from shooting themselves in the foot by setting --limit-modules in a way that excludes the jdk.internal.vm.compiler module. In essence we want to ens

Re: [10] RFR 8176841: Additional Unicode Language-Tag Extensions

2017-11-29 Thread Roger Riggs
+1 Thanks, Roger On 11/28/2017 7:00 PM, Lance Andersen wrote: Updates look good :-) On Nov 28, 2017, at 6:27 PM, Naoto Sato wrote: I've got some internal comments (two editorial fixes and java time test location move) and reflected them to the existing fix. Updated webrevs are located at:

Re: [10] RFR 6354947: [Fmt-*] DecimalFormat ignores FieldPosition settings on input, contrary to Javadocs

2017-11-29 Thread Roger Riggs
Hi Nishit, The webrev and updates to the javadoc look fine. However, the summary of the issue does not reflect the issue and implies the implementation is wrong; but it is the javadoc that need clarification. Please change the issue summary (on both the CSR and the issue) to reflect the true

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Alan Bateman
On 29/11/2017 11:26, David Holmes wrote: The current approach basically prevents anyone using JVMCI from shooting themselves in the foot by setting --limit-modules in a way that excludes the jdk.internal.vm.compiler module. In essence we want to ensure that if using JVMCI then all the requisi

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread David Holmes
On 29/11/2017 7:05 PM, Alan Bateman wrote: On 28/11/2017 23:51, Vladimir Kozlov wrote: http://cr.openjdk.java.net/~kvn/8191788/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8191788 This is redo of JDK-8190975 [1] fix which added jdk.internal.vm.compiler to tests which have --limit-module

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Doug Simon
> On 29 Nov 2017, at 10:05, Alan Bateman wrote: > > On 28/11/2017 23:51, Vladimir Kozlov wrote: >> http://cr.openjdk.java.net/~kvn/8191788/webrev.00/ >> https://bugs.openjdk.java.net/browse/JDK-8191788 >> >> This is redo of JDK-8190975 [1] fix which added jdk.internal.vm.compiler to >> tests

Re: [10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

2017-11-29 Thread Alan Bateman
On 28/11/2017 23:51, Vladimir Kozlov wrote: http://cr.openjdk.java.net/~kvn/8191788/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8191788 This is redo of JDK-8190975 [1] fix which added jdk.internal.vm.compiler to tests which have --limit-modules option. Unfortunately tests start failing