Re: review of 7117249: java.util warnings patches from LJC/Mike Barker

2011-12-07 Thread David Holmes
On 7/12/2011 9:12 PM, Martijn Verburg wrote: Hi all, Question on contributions, I assume that as the patch is coming from Mike, his OCA covers the rest of us? These patches are simple enough that OCA is not necessary - any Participant can submit simple patches [1] David -- [1]

Re: RFR 7118066: Warnings in java.util.concurrent package

2011-12-07 Thread Chris Hegarty
Thanks David and Doug, So we should be ready to integrate this change, right? NO, not yet! There is a failing regression test, java.util.Collections.EmptyIterator. This test does a reference comparison ( '==' ) on the iterator returned from an empty SynchronousQueue and the iterator returned

Re: Request for Review (XXL): 7104647: Adding a diagnostic command framework

2011-12-07 Thread Paul Hohensee
For the hotspot part: In attachListener.cpp, you might want to delete the first return JNI_OK; line, since the code under HAS_PENDING_EXCEPTION just falls through. In jmm.h, minor formatting nit: be nice to indent (JNIEnv on lines 318, 319 and 325 the same as the existing declarations. Add a

Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow

2011-12-07 Thread John Rose
On Dec 4, 2011, at 6:25 PM, David Holmes wrote: In the mean time, all the non-Groovy, non-JRuby, non-Nashorn, etc. uses of class Class and all the classes not visible in those environments when they are being used will be larger. Adding the fields may be the right time/space trade-off, but

Re: Request for Review (XXL): 7104647: Adding a diagnostic command framework

2011-12-07 Thread Paul Hohensee
This is just the start of a review: fumble-fingers sent it well before it's time. Paul On 12/7/11 4:15 PM, Paul Hohensee wrote: For the hotspot part: In attachListener.cpp, you might want to delete the first return JNI_OK; line, since the code under HAS_PENDING_EXCEPTION just falls

Re: Request for Review (XXL): 7104647: Adding a diagnostic command framework

2011-12-07 Thread Frederic Parain
On 12/7/11 2:32 AM, Mandy Chung wrote: On 12/2/2011 5:57 AM, Frederic Parain wrote: http://cr.openjdk.java.net/~fparain/7104647/webrev.jdk.00/ L112: Since you are in this file, can you fix the typo java.security.SecurityException to java.lang.SecurityException? Fixed. The execute method

Re: review of 7117249: java.util warnings patches from LJC/Mike Barker

2011-12-07 Thread Stuart Marks
On 12/7/11 2:34 AM, Michael Barker wrote: Michael - the Contributed-by line is usually the individual's name (+ mail address) or a list of names (and their mail addresses). I think Stuart is suggesting that this would be better than London Java Community. Okay, no problem: Contributed-by:

Re: Request for Review (XXL): 7104647: Adding a diagnostic command framework

2011-12-07 Thread Mandy Chung
On 12/7/2011 2:04 PM, Mandy Chung wrote: On 12/7/2011 1:30 PM, Frederic Parain wrote: The execute method doesn't throw any exception other than IAE. What happens if the specified command execution fails in the target VM? If no exception is thrown, how does the caller detect if the command

hg: jdk8/tl/langtools: 7086015: fix test/tools/javac/parser/netbeans/JavacParserTest.java

2011-12-07 Thread kumar . x . srinivasan
Changeset: abfa0d8ea803 Author:ksrini Date: 2011-12-07 10:47 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/abfa0d8ea803 7086015: fix test/tools/javac/parser/netbeans/JavacParserTest.java Reviewed-by: ksrini, jjg Contributed-by: matherey.nu...@oracle.com !

Re: RFR 7118066: Warnings in java.util.concurrent package

2011-12-07 Thread David Holmes
Chris, On 7/12/2011 10:09 PM, Chris Hegarty wrote: Thanks David and Doug, So we should be ready to integrate this change, right? NO, not yet! There is a failing regression test, java.util.Collections.EmptyIterator. This test does a reference comparison ( '==' ) on the iterator returned from