Re: Review Request -- 5045147 : When TreeMap is empty explicitly check for null keys in put() [updated]

2011-03-15 Thread Steve Poole

On 14/03/11 21:02, Mike Duigou wrote:

I've gotten feedback regarding this issue and I've updated the webrev to use 
the commented out compare(key, key) test rather than the previously committed 
solution. I hadn't looked at the commented out code too carefully and had 
assumed it was pseudo-code rather than an actual solution. It's an improvement 
over the original solution and reads, to me and apparently others, a lot 
simpler.


Hi - can you post the feedback to the mailing list?

http://cr.openjdk.java.net/~mduigou/5045147/1/webrev/

Also now included is a jtreg unit test.

Mike




Re: Review Request -- 5045147 : When TreeMap is empty explicitly check for null keys in put() [updated]

2011-03-15 Thread Mike Duigou

On Mar 15 2011, at 02:36 , Steve Poole wrote:

 On 14/03/11 21:02, Mike Duigou wrote:
 I've gotten feedback regarding this issue and I've updated the webrev to use 
 the commented out compare(key, key) test rather than the previously 
 committed solution. I hadn't looked at the commented out code too carefully 
 and had assumed it was pseudo-code rather than an actual solution. It's an 
 improvement over the original solution and reads, to me and apparently 
 others, a lot simpler.
 
 Hi - can you post the feedback to the mailing list?

Hi Steve;

Sorry, I can't repost them. I'd have preferred that the feedback went to the 
list but for whatever reasons the two respondents chose to send the feedback 
privately and I won't repost their private emails. (They are certainly welcome 
to do so if they feel it matters). Perhaps it was just a case of Reply to 
Sender vs Reply to All. I don't know. To summarize the messages though, 
Just use the commented out code for 504517. It's better than the old patch.

Mike

 http://cr.openjdk.java.net/~mduigou/5045147/1/webrev/
 
 Also now included is a jtreg unit test.
 
 Mike
 



RE: Review Request -- 5045147 : When TreeMap is empty explicitly check for null keys in put() [updated]

2011-03-15 Thread Jason Mehrens

Hi Steve,
 
I was one of the people that provided feedback on Mike's patch.  In my case, it 
was a mishap of reply to sender vs. reply to all.  I don't have the original 
email but, the result are visible in the test case that Mike wrote.  My main 
concern with the old patch that if you use a raw type (think legacy code) you 
can poison a TreeMap/TreeSet with non-null object that cannot be compared.  I 
remembered reviewing that code back in JSR166 maintenance review.  I can't take 
credit for it since the review section of the bug report does a great job of 
explaining the evolution of the correct patch.  
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5045147
 
Jason
 
 Subject: Re: Review Request -- 5045147 : When TreeMap is empty explicitly 
 check for null keys in put() [updated]
 From: mike.dui...@oracle.com
 Date: Tue, 15 Mar 2011 09:34:00 -0700
 To: spo...@linux.vnet.ibm.com
 CC: core-libs-dev@openjdk.java.net
 
 
 On Mar 15 2011, at 02:36 , Steve Poole wrote:
 
  On 14/03/11 21:02, Mike Duigou wrote:
  I've gotten feedback regarding this issue and I've updated the webrev to 
  use the commented out compare(key, key) test rather than the previously 
  committed solution. I hadn't looked at the commented out code too 
  carefully and had assumed it was pseudo-code rather than an actual 
  solution. It's an improvement over the original solution and reads, to me 
  and apparently others, a lot simpler.
  
  Hi - can you post the feedback to the mailing list?
 
 Hi Steve;
 
 Sorry, I can't repost them. I'd have preferred that the feedback went to the 
 list but for whatever reasons the two respondents chose to send the feedback 
 privately and I won't repost their private emails. (They are certainly 
 welcome to do so if they feel it matters). Perhaps it was just a case of 
 Reply to Sender vs Reply to All. I don't know. To summarize the messages 
 though, Just use the commented out code for 504517. It's better than the old 
 patch.
 
 Mike
 
  http://cr.openjdk.java.net/~mduigou/5045147/1/webrev/
  
  Also now included is a jtreg unit test.
  
  Mike
  
 
  

hg: jdk7/tl/langtools: 6993311: annotations on packages are not validated

2011-03-15 Thread jonathan . gibbons
Changeset: c9432f06d9bc
Author:jjg
Date:  2011-03-15 11:04 -0700
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/c9432f06d9bc

6993311: annotations on packages are not validated
Reviewed-by: mcimadamore

! src/share/classes/com/sun/tools/javac/comp/Attr.java
! src/share/classes/com/sun/tools/javac/main/JavaCompiler.java
+ test/tools/javac/annotations/TestAnnotationPackageInfo.java
! test/tools/javac/annotations/pos/package-info.java
! test/tools/javac/processing/filer/TestPackageInfo.java
! test/tools/javac/processing/filer/foo/bar/package-info.java



hg: jdk7/tl/langtools: 6987384: -XprintProcessorRoundsInfo message printed with different timing than previous

2011-03-15 Thread jonathan . gibbons
Changeset: edf03ca74991
Author:jjg
Date:  2011-03-15 11:41 -0700
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/edf03ca74991

6987384: -XprintProcessorRoundsInfo message printed with different timing than 
previous
Reviewed-by: darcy

! test/tools/javac/lib/JavacTestingAbstractProcessor.java
! test/tools/javac/processing/6430209/b6341534.java
! test/tools/javac/processing/environment/round/TestContext.java
+ test/tools/javac/processing/options/testPrintProcessorInfo/Test.java
+ test/tools/javac/processing/options/testPrintProcessorInfo/Test.out



hg: jdk7/tl/langtools: 6988079: Errors reported via Messager.printMessage(ERROR, error message) are not tallied correctly

2011-03-15 Thread jonathan . gibbons
Changeset: 0f9e5b7f0d7e
Author:jjg
Date:  2011-03-15 11:48 -0700
URL:   http://hg.openjdk.java.net/jdk7/tl/langtools/rev/0f9e5b7f0d7e

6988079: Errors reported via Messager.printMessage(ERROR,error message) are 
not tallied correctly
Reviewed-by: darcy

! 
src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
! test/tools/javac/processing/6994946/SemanticErrorTest.2.out
+ test/tools/javac/processing/errors/TestErrorCount.java
+ test/tools/javac/processing/errors/TestErrorCount.out



hg: jdk7/tl/jdk: 7001933: Deadlock in java.lang.classloader.getPackage()

2011-03-15 Thread valerie . peng
Changeset: 4a7da412db38
Author:valeriep
Date:  2011-03-15 18:42 -0700
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/4a7da412db38

7001933: Deadlock in java.lang.classloader.getPackage()
Summary: Modified to not holding the packages lock when calling parent CL.
Reviewed-by: dholmes, alanb

! src/share/classes/java/lang/ClassLoader.java