Re: Review request (XS): 7022100: Method annotations are incorrectly set when redefining classes

2012-01-29 Thread Stefan Karlsson 

On 2012-01-27 14:30, David Holmes wrote:

On 27/01/2012 11:12 PM, Stefan Karlsson wrote:

Here's a fix for a simple copy-n-past bug in the handling of
annotations, affecting only class redefinition.

http://cr.openjdk.java.net/~stefank/7022100/webrev.00/

7022100: Method annotations are incorrectly set when redefining classes
Summary: Changed to the correct annotation arrays
Reviewed-by: TBD1, TBD2


Looks perfectly logical, but begs the question as to how this has not 
been discovered. Are there no tests in this area?


I don't think so. This code is only exercised if you redefine a class 
under the following conditions:

1) The class has overloaded functions,
2) these functions have parameter annotations,
3) the redefined class has swapped the order of these functions (or 
added another overloaded function).


I've managed to write a test for the set_method_parameter_annotations, I 
just need to figure out how to push it to a suitable test suite.


I couldn't come up with a test case fore the 
set_method_default_annotations case. These annotations are only used 
together with methods in an Annotation class. Since annotation methods 
are not allowed to have parameters, I can't induce condition (3) above.


thanks,
StefanK



David

Review request (XS): 7022100: Method annotations are incorrectly set when redefining classes

2012-01-27 Thread Stefan Karlsson 
Here's a fix for a simple copy-n-past bug in the handling of 
annotations, affecting only class redefinition.


http://cr.openjdk.java.net/~stefank/7022100/webrev.00/

7022100: Method annotations are incorrectly set when redefining classes
Summary: Changed to the correct annotation arrays
Reviewed-by: TBD1, TBD2

Re: Review request (XS): 7022100: Method annotations are incorrectly set when redefining classes

2012-01-27 Thread Keith McGuigan 


Looks good.

On Jan 27, 2012, at 8:12 AM, Stefan Karlsson wrote:

Here's a fix for a simple copy-n-past bug in the handling of  
annotations, affecting only class redefinition.


http://cr.openjdk.java.net/~stefank/7022100/webrev.00/

7022100: Method annotations are incorrectly set when redefining  
classes

Summary: Changed to the correct annotation arrays
Reviewed-by: TBD1, TBD2

Re: Review request (XS): 7022100: Method annotations are incorrectly set when redefining classes

2012-01-27 Thread David Holmes 

On 27/01/2012 11:12 PM, Stefan Karlsson wrote:

Here's a fix for a simple copy-n-past bug in the handling of
annotations, affecting only class redefinition.

http://cr.openjdk.java.net/~stefank/7022100/webrev.00/

7022100: Method annotations are incorrectly set when redefining classes
Summary: Changed to the correct annotation arrays
Reviewed-by: TBD1, TBD2


Looks perfectly logical, but begs the question as to how this has not 
been discovered. Are there no tests in this area?


David

Re: Review request (XS): 7022100: Method annotations are incorrectly set when redefining classes

2012-01-27 Thread Staffan Larsen 
Looks good.

On 27 jan 2012, at 14:18, Keith McGuigan wrote:

 
 Looks good.
 
 On Jan 27, 2012, at 8:12 AM, Stefan Karlsson wrote:
 
 Here's a fix for a simple copy-n-past bug in the handling of annotations, 
 affecting only class redefinition.
 
 http://cr.openjdk.java.net/~stefank/7022100/webrev.00/
 
 7022100: Method annotations are incorrectly set when redefining classes
 Summary: Changed to the correct annotation arrays
 Reviewed-by: TBD1, TBD2