Re: [threeten-dev] ReviewRequest 8011172: JSR 310: DateTime API Updates II,

2013-04-12 Thread Xueming Shen
Good catch. webrev updated. It appears none of the tests catched this error, if I just build the jdk normally (?), either local fresh new build and jprt job. http://cr.openjdk.java.net/~sherman/8011172/webrev/ -Sherman. On 4/11/13 10:57 PM, David Holmes wrote: Hi Sherman, On 12/04/2013

Re: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Peter Levart
Hi Joe, There were certainly debates about why self-suppression is not a good thing when project Coin's try-with-resources has been developed, but I don't quite remember the reason why it was designed this way. Couldn't the logic just make the self-suppression a no-op? The addSuppressed was

Re: [threeten-dev] ReviewRequest 8011172: JSR 310: DateTime API Updates II,

2013-04-12 Thread David Holmes
On 12/04/2013 4:27 PM, Xueming Shen wrote: Good catch. webrev updated. It appears none of the tests catched this error, if I just build the jdk normally (?), either local fresh new build and jprt job. http://cr.openjdk.java.net/~sherman/8011172/webrev/ A profiles build would have been

hg: jdk8/tl/langtools: 7015104: use new subtype of TypeSymbol for type parameters

2013-04-12 Thread joel . franck
Changeset: 137994c189e5 Author:jfranck Date: 2013-04-12 12:05 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/137994c189e5 7015104: use new subtype of TypeSymbol for type parameters Reviewed-by: jjg, mcimadamore ! src/share/classes/com/sun/tools/javac/code/Symbol.java

Re: RFR JDK-8011427: java.util.concurrent collection Spliterator implementations

2013-04-12 Thread Paul Sandoz
On Apr 10, 2013, at 5:03 PM, Peter Levart peter.lev...@gmail.com wrote: On 04/10/2013 03:56 PM, Paul Sandoz wrote: Hi, Following up from JDK-8010096 [1] here is a webrev for spliterator implementations of collection classes in java.util.concurrent. More precisely it represents updates

Re: AWT Dev Swing Dev sun.awt.X11 logs still using String + (waste)

2013-04-12 Thread Anthony Petrov
Thanks Laurent. The fix looks fine to me, too. I've just pushed it to the repository: http://hg.openjdk.java.net/jdk8/awt/jdk/rev/4490ef60ecd3 Currently it's in the AWT forest. I expect it to be integrated to the Master JDK8 repository in a week or two. -- best regards, Anthony On

RFR: [corba] 4504275, 8011986 - two issues with generated code

2013-04-12 Thread Dmeetry Degrave
Hi all, I'm looking for a review for two ancient issues in idlj. First is an issue with uncompilable java code generated for unions with boolean discriminator. 4504275: CORBA boolean type unions do not generate compilable code from idlj bug:

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Jason Mehrens
Joe, Should this same logic be applied to the exceptions thrown from initCause? Seems like that would be consistent with this change. Jason Date: Thu, 11 Apr 2013 18:19:30 -0700 From: joe.da...@oracle.com To: core-libs-dev@openjdk.java.net Subject: Code review request for 8012044: Give more

Re: Codereview Request: 8011647: Add java.time.Instant methods to java.nio.file.attribute.FileTime

2013-04-12 Thread Alan Bateman
On 11/04/2013 21:47, Xueming Shen wrote: Hi As part of the JSR310 Date/Time project, following methods are proposed to be added into java.nio.file.attribute.FileTime to interoperate with the new JSR310 time class Instant. public static FileTime from(Instant instant); public Instant

Re: RFR 8011805: Update sun.tools.java class file reading/writing support to include the new constant pool entries (including invokedynamic)

2013-04-12 Thread Alan Bateman
On 11/04/2013 22:23, Robert Field wrote: Thank you Mike, Alan, and Brian for your reviews, and others for your assistance. Updated webrev: http://cr.openjdk.java.net/~rfield/8011805_2 Changes are all in the test: * Removed unused testWrite and related code. * Used correct copyright. *

hg: jdk8/tl/jdk: 8011172: JSR 310 DateTime API Updates II

2013-04-12 Thread xueming . shen
Changeset: f4d50e8cc9e2 Author:sherman Date: 2013-04-12 07:57 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f4d50e8cc9e2 8011172: JSR 310 DateTime API Updates II Summary: Integration of JSR310 Date/Time API update Reviewed-by: alanb, naoto, dholmes Contributed-by:

Re: RFR: String.join(), StringJoiner additions

2013-04-12 Thread Alan Bateman
On 11/04/2013 23:33, Jim Gish wrote: Please review http://cr.openjdk.java.net/~jgish/Bugs-5015163-7175206-7172553/ http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7175206-7172553/ These are changes that we made in lambda that we're now bringing into JDK8. I've made a couple of additions -

Re: [threeten-dev] ReviewRequest 8011172: JSR 310: DateTime API Updates II,

2013-04-12 Thread Alan Bateman
On 12/04/2013 08:09, David Holmes wrote: On 12/04/2013 4:27 PM, Xueming Shen wrote: Good catch. webrev updated. It appears none of the tests catched this error, if I just build the jdk normally (?), either local fresh new build and jprt job. http://cr.openjdk.java.net/~sherman/8011172/webrev/

Re: [threeten-dev] ReviewRequest 8011172: JSR 310: DateTime API Updates II,

2013-04-12 Thread Alan Bateman
On 12/04/2013 17:04, Xueming Shen wrote: Will add in a separate push/update. I just checked it and the HijrahChronology fails to initialized as expected on profile builds. The attached sorts it out. -Alan. diff -r f4d50e8cc9e2 makefiles/profile-includes.txt ---

Re: [threeten-dev] ReviewRequest 8011172: JSR 310: DateTime API Updates II,

2013-04-12 Thread Alan Bateman
On 12/04/2013 17:17, Xueming Shen wrote: Alan, please help review. http://cr.openjdk.java.net/~sherman/8012123/webrev/ -Sherman Looks good. -Alan

Re: [threeten-dev] ReviewRequest 8011172: JSR 310: DateTime API Updates II,

2013-04-12 Thread Xueming Shen
Alan, please help review. http://cr.openjdk.java.net/~sherman/8012123/webrev/ -Sherman On 4/12/13 8:46 AM, Alan Bateman wrote: On 12/04/2013 08:09, David Holmes wrote: On 12/04/2013 4:27 PM, Xueming Shen wrote: Good catch. webrev updated. It appears none of the tests catched this error, if

hg: jdk8/tl/jdk: 8012123: hijrah-config-umalqura.properties is missing from makefiles/profile-includes.txt

2013-04-12 Thread xueming . shen
Changeset: 035a61c9f981 Author:sherman Date: 2013-04-12 09:51 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/035a61c9f981 8012123: hijrah-config-umalqura.properties is missing from makefiles/profile-includes.txt Summary: added the hijrah-config-umalqura.properties into the

hg: jdk8/tl/jdk: 8011805: Update sun.tools.java class file reading/writing support to include the new constant pool entries

2013-04-12 Thread robert . field
Changeset: e2cd40d7567c Author:rfield Date: 2013-04-12 10:02 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e2cd40d7567c 8011805: Update sun.tools.java class file reading/writing support to include the new constant pool entries Reviewed-by: mduigou, alanb !

Re: Codereview Request: 8011647: Add java.time.Instant methods to java.nio.file.attribute.FileTime

2013-04-12 Thread Xueming Shen
On 04/12/2013 05:55 AM, Alan Bateman wrote: Sherman and I chatted about adding from(Instant)/toInstant() a few times over the last week so most of my comments are already included. Overall I think it has worked out well, just unfortunate that there is a bit of extra complexity because

Re: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Joe Darcy
Hi Jason, Hmm. This is the current initCause implementation from JDK 8: public synchronized Throwable initCause(Throwable cause) { if (this.cause != this) throw new IllegalStateException(Can't overwrite cause); if (cause == this) throw new

RFR (S) 8009531: Crash when redefining class with annotated method

2013-04-12 Thread Coleen Phillimore
Summary: Add annotations to the tests to verify bug above open webrev at http://cr.openjdk.java.net/~coleenp/8009531_jdk/ bug link at http://bugs.sun.com/view_bug.do?bug_id=8009531_jdk The Hotspot change is in tl repository now. Also, this has been reviewed by the hotspot group. Thanks,

Re: RFR (S) 8009531: Crash when redefining class with annotated method

2013-04-12 Thread Coleen Phillimore
Can you reply to me as well as the mailing list since I'm not on this mailing list? thanks, Coleen Original Message Subject: RFR (S) 8009531: Crash when redefining class with annotated method Date: Fri, 12 Apr 2013 13:45:30 -0400 From: Coleen Phillimore

Re: Codereview Request: 8011647: Add java.time.Instant methods to java.nio.file.attribute.FileTime

2013-04-12 Thread Alan Bateman
On 12/04/2013 18:28, Xueming Shen wrote: I'm not sure if the 3-digit unit fitting is better or not. But xml spec cited in the toString() API specifies that the trailing zeros are optional in its 3.2.3.1 lexical representatin section and trailing zeros are prohibited... in 3.2.3.2 canonical

Request for Review : CR#6924259: Remove String.count/String.offset

2013-04-12 Thread wangsheng.0376
hi, all, I agree with you to remove offset, today when I run following code in jdk7(sorry I forget the detail version), my code is like this: Pattern pattern = Pattern.compile(regex); Matcher matcher = pattern.match(content); while (matcher.find()) { String link = matcher.group(1);

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Jason Mehrens
The landmines are the retrofitted exception classes as shown here https://netbeans.org/bugzilla/show_bug.cgi?id=150969 and https://issues.jboss.org/browse/JBREM-552. Really, if the ISE or IAE is thrown it is going to suppress 'this' and 'cause'. It would be nice to see the given 'cause'

Review request: 8004260: dynamic proxy class should have same class access as proxy interfaces

2013-04-12 Thread Mandy Chung
Dynamic proxy class is specified to be public, final, and not abstract. For a proxy class that implements a non-public interface, it will be in the same package as the non-public interface but the proxy class is accessible outside of the non-public interface's runtime package. This change will

Re: RFR: 8010279 - Comparators should throw NPE on null argument

2013-04-12 Thread Henry Jen
On 04/12/2013 11:56 AM, Henry Jen wrote: On 04/10/2013 11:09 AM, Alan Bateman wrote: On 10/04/2013 19:00, Henry Jen wrote: Hi, The bug is reported on lambda repo, part of the fix involved code already integrated into TL/master, thus need help on review and commit. Webrev at:

Re: RFR: 8010279 - Comparators should throw NPE on null argument

2013-04-12 Thread Henry Jen
On 04/10/2013 11:09 AM, Alan Bateman wrote: On 10/04/2013 19:00, Henry Jen wrote: Hi, The bug is reported on lambda repo, part of the fix involved code already integrated into TL/master, thus need help on review and commit. Webrev at:

Re: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Joe Darcy
On 04/12/2013 11:22 AM, Jason Mehrens wrote: The landmines are the retrofitted exception classes as shown here https://netbeans.org/bugzilla/show_bug.cgi?id=150969 and https://issues.jboss.org/browse/JBREM-552. Really, if the ISE or IAE is thrown it is going to suppress 'this' and 'cause'. It

hg: jdk8/tl/jdk: 8011647: Add java.time.Instant methods to java.nio.file.attribute.FileTime

2013-04-12 Thread xueming . shen
Changeset: 6c935c5ac7ff Author:sherman Date: 2013-04-12 12:03 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6c935c5ac7ff 8011647: Add java.time.Instant methods to java.nio.file.attribute.FileTime Summary: added the toInstant()/from(Instant) to FileTime Reviewed-by: alanb !

hg: jdk8/tl/jdk: 8002390: (zipfs) Problems moving files between zip file systems

2013-04-12 Thread xueming . shen
Changeset: 729ca1ef7c75 Author:sherman Date: 2013-04-12 12:12 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/729ca1ef7c75 8002390: (zipfs) Problems moving files between zip file systems Summary: fixed the corner cases in zipfs Reviewed-by: sherman Contributed-by:

Re: RFR: 8004518 8010122 : Default methods on Map

2013-04-12 Thread Akhil Arora
Hi Mike, a few small things - UnmodifiableMap.forEach is missing Objects.requireNonNull(action); EmptyMap.replaceAll(BiFunction) should just return instead of throwing UnsupportedOpEx particularly since EmptyList.replaceAll also returns silently after checking if function is null to

Re: RFR: 8004518 8010122 : Default methods on Map

2013-04-12 Thread Mike Duigou
Thanks for the corrections. I have incorporated all of these into the integration version of the patch. On Apr 12 2013, at 12:50 , Akhil Arora wrote: Hi Mike, a few small things - UnmodifiableMap.forEach is missing Objects.requireNonNull(action); Added.

RFR: 8010939 Deadlock in LogManager

2013-04-12 Thread Jim Gish
Please review: http://cr.openjdk.java.net/~jgish/Bug8010939-LogManager-Deadlock/ http://cr.openjdk.java.net/%7Ejgish/Bug8010939-LogManager-Deadlock/ Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network

Re: Proxy.isProxyClass scalability

2013-04-12 Thread Mandy Chung
Hi Peter, Thank you for rebasing the patch. This is very good work and I hope to make time to work with you to get your patch to jdk8 over the next couple weeks. On 4/10/2013 5:35 AM, Peter Levart wrote: Hi Alan, I have prepared new webrev of the patch rebased to current tip of jdk8/tl

Re: RFR: 8004518 8010122 : Default methods on Map

2013-04-12 Thread Mike Duigou
On Apr 11 2013, at 15:15 , Ulf Zibis wrote: Am 11.04.2013 22:03, schrieb Mike Duigou: Another revision incorporating primarily documentation feedback. http://cr.openjdk.java.net/~mduigou/JDK-8010122/2/webrev/ There is still a yoda style in ConcurrentMap line 72, HashMap line 361 Fixed

Re: RFR: 8004518 8010122 : Default methods on Map

2013-04-12 Thread Mike Duigou
I have updated the webrev with these changes and a few more. merge() required an update to it's specification to correctly account for null values. http://cr.openjdk.java.net/~mduigou/JDK-8010122/5/webrev/ This version is currently undergoing final pre-integration testing. Unless additional

Re: RFR: 8010939 Deadlock in LogManager

2013-04-12 Thread Mandy Chung
Hi Jim, Thanks for fixing this. On 4/12/2013 1:11 PM, Jim Gish wrote: Please review: http://cr.openjdk.java.net/~jgish/Bug8010939-LogManager-Deadlock/ The fix looks okay. I would recommend to move the call to manager.drainLoggerRefQueueBounded() back to LogManager.addLogger which was

Re: Throwable.addSuppressed error conditions -- use the exception as the cause?

2013-04-12 Thread Steven Schlansker
On Apr 11, 2013, at 9:04 PM, Zhong Yu zhong.j...@gmail.com wrote: On Tue, Apr 9, 2013 at 11:54 AM, Steven Schlansker stevenschlans...@gmail.com wrote: I agree, but there is (library) code out there that does this -- the code that caused the problem wasn't even mine. The library should

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2013-04-12 Thread Joe Darcy
Following up on email from a few months ago On 12/19/2012 01:01 AM, Peter Levart wrote: On 12/19/2012 01:35 AM, David Holmes wrote: On 19/12/2012 10:24 AM, Joe Darcy wrote: On 12/18/2012 04:20 PM, David Holmes wrote: On 19/12/2012 10:16 AM, Joe Darcy wrote: On 12/18/2012 04:12 PM, David

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2013-04-12 Thread Krystal Mok
Hi Nazario, That's a typical substring leak case. Matcher.group(n) eventually calls String.substring() to return the result. Before removing the count and offset fields in String, that'll return a String sharing the value char array, which may cause memory leaks if you hold on to a small

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Jason Mehrens
Joe, You'll have guard ise.addSuppressed against null. Looks good otherwise. private static void initCauseNull() { Throwable t1 = new Throwable(); t1.initCause(null); try { t1.initCause(null); } catch(IllegalStateException expect) { } } Jason Date: Fri, 12 Apr

hg: jdk8/tl/jdk: 8010279: java.util.Stream.min/max((Comparator)null) is not consistent in throwing (unspecified) NPE

2013-04-12 Thread jonathan . gibbons
Changeset: d8cae0195fe9 Author:henryjen Date: 2013-04-12 12:41 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d8cae0195fe9 8010279: java.util.Stream.min/max((Comparator)null) is not consistent in throwing (unspecified) NPE Reviewed-by: alanb, mduigou !

hg: jdk8/tl/jdk: 8012028: Metafactory-generated lambda classes should be final; ...

2013-04-12 Thread robert . field
Changeset: 06dfdfa8c3e6 Author:rfield Date: 2013-04-12 20:23 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/06dfdfa8c3e6 8012028: Metafactory-generated lambda classes should be final 8008941: isSynthetic() returns false for lambda instances Reviewed-by: mduigou !