Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2018-06-14 Thread Vicente Romero
I'm not an expert in the area, but the patch looks good to me, Vicente On 05/31/2018 08:39 PM, Liam Miller-Cushon wrote: Hi, Are there any concerns with the patch that I can address? I was hoping to get this in to JDK 11. I confirmed that it still builds and passes all tests at head.

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2018-02-28 Thread Martin Buchholz
I was surprised to learn that junit has a magic assertEquals specifically for Object[] assertEquals(Object[], Object[]) but that is obviously a bad idea and junit eventually deprecated it. https://junit.org/junit4/javadoc/4.12/org/junit/Assert.html#assertEquals(java.lang.Object[],

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2018-02-27 Thread Joseph D. Darcy
Hi Liam, On 2/24/2018 5:14 PM, Liam Miller-Cushon wrote: Hi, thanks for the comments. The updated webrev is at: http://cr.openjdk.java.net/~cushon/7183985/webrev.02/ On Fri, Feb 23, 2018 at 4:29 PM, Joseph D. Darcy

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2018-02-23 Thread Joseph D. Darcy
Hello, A few initial comments, not my final review. Objects implementing the AnnotatedElement interface are also created in javac during annotation processing. As a secondary concern, it would be good to be have behavior between both javac and runtime annotations consistent when possible.

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2018-02-23 Thread joe darcy
On 2/23/2018 9:39 AM, Alan Bateman wrote: On 22/02/2018 23:20, Liam Miller-Cushon wrote: Hello, Please consider this fix for JDK-7183985. bug: https://bugs.openjdk.java.net/browse/JDK-7183985 webrev: http://cr.openjdk.java.net/~cushon/7183985/webrev.01/ I started a CSR for the change:

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2018-02-23 Thread Alan Bateman
On 22/02/2018 23:20, Liam Miller-Cushon wrote: Hello, Please consider this fix for JDK-7183985. bug: https://bugs.openjdk.java.net/browse/JDK-7183985 webrev: http://cr.openjdk.java.net/~cushon/7183985/webrev.01/ I started a CSR for the change:

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2016-02-16 Thread Joel Borggrén-Franck
Hi Liam, Sorry for the delay, On Sat, 30 Jan 2016 at 04:45 Liam Miller-Cushon wrote: > On Thu, Jan 28, 2016 at 11:50 AM, Joel Borggrén-Franck < > joel.borggren.fra...@gmail.com> wrote: > > But, the reason we didn't fix this the last two times we looked at it >> (that I am

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2016-01-29 Thread Liam Miller-Cushon
On Thu, Jan 28, 2016 at 11:50 AM, Joel Borggrén-Franck < joel.borggren.fra...@gmail.com> wrote: > Thanks for the update. I think it makes sense to expand testing > slightly, testing all three parsing clauses that you fixed, and for > all three of them also checking that a "thing" after the broken

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2016-01-28 Thread Joel Borggrén-Franck
Hi, Thanks for the update. I think it makes sense to expand testing slightly, testing all three parsing clauses that you fixed, and for all three of them also checking that a "thing" after the broken item is parsed correctly. But, the reason we didn't fix this the last two times we looked at it

Re: RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

2016-01-05 Thread Joel Borggrén-Franck
Hi Liam, I think you need to do skipMemberValue the correct number of times so that the cursor in buf is correct, or? I modified your test slightly to provoke an AnnotationTypeMismatchException that I think shouldn't be there: @AnnotationAnnotation({@ClassArrayAnnotation({Missing.class,