7073296: Review : Bug in Executable.equalParamTypes() results in incorrect comparison for differing number of params
Hello All; A fairly simple bug to review which snuck through testing. http://cr.openjdk.java.net/~mduigou/7073296/0/webrev/ The changes to Class are incidental but so trivial that opted to include them. I can remove if anyone feels strongly (or even weakly) that they should be excluded. Mike
Hi Mike. Thanks for developing the fix for this. Generally looks good to go back; a few minor nits. Personally, I would leave the "// Doesn't use Boolean.getBoolean to avoid class init." note in java.lang.Class unless you know the comment is not relevant any more. Given the default semantics of jtreg, you could omit the @compile and @run lines in the test. Cheers, -Joe On 8/2/2011 12:44 PM, Mike Duigou wrote:
Hello All;
A fairly simple bug to review which snuck through testing.
http://cr.openjdk.java.net/~mduigou/7073296/0/webrev/
The changes to Class are incidental but so trivial that opted to include them. I can remove if anyone feels strongly (or even weakly) that they should be excluded.
Mike
On Aug 2 2011, at 15:27 , joe.darcy@oracle.com wrote:
Hi Mike.
Thanks for developing the fix for this. Generally looks good to go back; a few minor nits. Personally, I would leave the "// Doesn't use Boolean.getBoolean to avoid class init." note in java.lang.Class unless you know the comment is not relevant any more.
I actually added the note. I checked with java -verbose:class on the load order and Class is loaded quite a bit before Boolean. It's a note to future maintainers who, like I was, are tempted to convert to Boolean.getBoolean().
Given the default semantics of jtreg, you could omit the @compile and @run lines in the test.
Will do.
Cheers,
-Joe
On 8/2/2011 12:44 PM, Mike Duigou wrote:
Hello All;
A fairly simple bug to review which snuck through testing.
http://cr.openjdk.java.net/~mduigou/7073296/0/webrev/
The changes to Class are incidental but so trivial that opted to include them. I can remove if anyone feels strongly (or even weakly) that they should be excluded.
Mike
On 8/2/2011 3:32 PM, Mike Duigou wrote:
On Aug 2 2011, at 15:27 , joe.darcy@oracle.com wrote:
Hi Mike.
Thanks for developing the fix for this. Generally looks good to go back; a few minor nits. Personally, I would leave the "// Doesn't use Boolean.getBoolean to avoid class init." note in java.lang.Class unless you know the comment is not relevant any more.
I actually added the note. I checked with java -verbose:class on the load order and Class is loaded quite a bit before Boolean. It's a note to future maintainers who, like I was, are tempted to convert to Boolean.getBoolean().
Read the patch wrong; doh! -Joe
Am 02.08.2011 21:44, schrieb Mike Duigou:
Hello All;
A fairly simple bug to review which snuck through testing.
http://cr.openjdk.java.net/~mduigou/7073296/0/webrev/
The changes to Class are incidental but so trivial that opted to include them. I can remove if anyone feels strongly (or even weakly) that they should be excluded.
Mike Looks good to me
Mike Duigou wrote:
Hello All;
A fairly simple bug to review which snuck through testing.
http://cr.openjdk.java.net/~mduigou/7073296/0/webrev/
The changes to Class are incidental but so trivial that opted to include them. I can remove if anyone feels strongly (or even weakly) that they should be excluded.
Mike While the bug did sneak into the jdk8/tl/jdk repository it didn't get any further so no damage done. Thanks for tracking it down. The fix looks good to me too.
-Alan
participants (4)
-
Alan Bateman
-
joe.darcy＠oracle.com
-
Mike Duigou
-
Sebastian Sickelmann