Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2013-04-12 Thread Joe Darcy
Following up on email from a few months ago On 12/19/2012 01:01 AM, Peter Levart wrote: On 12/19/2012 01:35 AM, David Holmes wrote: On 19/12/2012 10:24 AM, Joe Darcy wrote: On 12/18/2012 04:20 PM, David Holmes wrote: On 19/12/2012 10:16 AM, Joe Darcy wrote: On 12/18/2012 04:12 PM, David

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-19 Thread Peter Levart
Hi Joe, Also, now studying the logic of Class.privateGetPublicMethods(), I don't quite get the reasoning behind current implementation regardless of default interface methods. The javadoc for Class.getMethods() is not very precise about what happens with multiple public methods with same sign

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-19 Thread Peter Levart
Hi Joe, I think that besides this bug, also the Class.privateGetPublicMethods() will need to be revised. Currently it suppresses any methods from direct interfaces that either: - have a non-abstract method with same signature in a superclass - have an abstract or non-abstract method with same

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-19 Thread Peter Levart
On 12/19/2012 10:01 AM, Peter Levart wrote: On 12/19/2012 01:35 AM, David Holmes wrote: On 19/12/2012 10:24 AM, Joe Darcy wrote: On 12/18/2012 04:20 PM, David Holmes wrote: On 19/12/2012 10:16 AM, Joe Darcy wrote: On 12/18/2012 04:12 PM, David Holmes wrote: On 19/12/2012 8:40 AM, Louis Wasse

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-19 Thread Peter Levart
On 12/19/2012 01:35 AM, David Holmes wrote: On 19/12/2012 10:24 AM, Joe Darcy wrote: On 12/18/2012 04:20 PM, David Holmes wrote: On 19/12/2012 10:16 AM, Joe Darcy wrote: On 12/18/2012 04:12 PM, David Holmes wrote: On 19/12/2012 8:40 AM, Louis Wasserman wrote: It's not 100% obvious to me whet

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread David Holmes
On 19/12/2012 10:24 AM, Joe Darcy wrote: On 12/18/2012 04:20 PM, David Holmes wrote: On 19/12/2012 10:16 AM, Joe Darcy wrote: On 12/18/2012 04:12 PM, David Holmes wrote: On 19/12/2012 8:40 AM, Louis Wasserman wrote: It's not 100% obvious to me whether this refers to a default implementation i

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Joe Darcy
On 12/18/2012 04:20 PM, David Holmes wrote: On 19/12/2012 10:16 AM, Joe Darcy wrote: On 12/18/2012 04:12 PM, David Holmes wrote: On 19/12/2012 8:40 AM, Louis Wasserman wrote: It's not 100% obvious to me whether this refers to a default implementation in an interface, a class which inherits tha

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Joe Darcy
On 12/18/2012 04:12 PM, David Holmes wrote: On 19/12/2012 8:40 AM, Louis Wasserman wrote: It's not 100% obvious to me whether this refers to a default implementation in an interface, a class which inherits that default implementation and does not override it, or both. Is that worth clarifying

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread David Holmes
On 19/12/2012 10:16 AM, Joe Darcy wrote: On 12/18/2012 04:12 PM, David Holmes wrote: On 19/12/2012 8:40 AM, Louis Wasserman wrote: It's not 100% obvious to me whether this refers to a default implementation in an interface, a class which inherits that default implementation and does not overrid

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Louis Wasserman
Could we say that, in so many words, in the Javadoc? On Tue, Dec 18, 2012 at 4:12 PM, David Holmes wrote: > On 19/12/2012 8:40 AM, Louis Wasserman wrote: > >> It's not 100% obvious to me whether this refers to a default >> implementation >> in an interface, a class which inherits that default im

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread David Holmes
On 19/12/2012 8:40 AM, Louis Wasserman wrote: It's not 100% obvious to me whether this refers to a default implementation in an interface, a class which inherits that default implementation and does not override it, or both. Is that worth clarifying in the doc, rather than forcing readers to che

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread David Holmes
On 19/12/2012 7:29 AM, Jim Gish wrote: Excuse me, a "comma". On 12/18/2012 04:20 PM, Jim Gish wrote: Code looks good, Joe. There is just one grammatical nit with the comment: * A default method is a non-abstract method, that is a method with 512 * a body, declared in an interface type. Actua

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Louis Wasserman
It's not 100% obvious to me whether this refers to a default implementation in an interface, a class which inherits that default implementation and does not override it, or both. Is that worth clarifying in the doc, rather than forcing readers to check the JLS citation? On Tue, Dec 18, 2012 at 2

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Joe Darcy
Mandy and Jim, I'll correct the typos before I push. Thanks for the careful review, -Joe On 12/18/2012 01:21 PM, Mandy Chung wrote: On 12/18/12 12:43 PM, Joe Darcy wrote: Hello, Please review these changes to add core reflection support to recognize default methods: 8005042 Add Metho

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Joe Darcy
Hello, On 12/18/2012 01:17 PM, Mike Duigou wrote: Looks good to me. Will a JLS reference be added in when defined? It seems kind of odd to mention JLS and not provide a ref. Yes; once there is a particular JLS section available to reference, this documentation should be updated to reference

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Jim Gish
Excuse me, a "comma". On 12/18/2012 04:20 PM, Jim Gish wrote: Code looks good, Joe. There is just one grammatical nit with the comment: * A default method is a non-abstract method, that is a method with 512 * a body, declared in an interface type. "that is" should have a comment

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Mandy Chung
On 12/18/12 12:43 PM, Joe Darcy wrote: Hello, Please review these changes to add core reflection support to recognize default methods: 8005042 Add Method.isDefault to core reflection http://cr.openjdk.java.net/~darcy/8005042.0/ Looks good to me. For the test: 56 System.err.pr

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Jim Gish
Code looks good, Joe. There is just one grammatical nit with the comment: * A default method is a non-abstract method, that is a method with 512 * a body, declared in an interface type. "that is" should have a comment after it. Thanks, Jim On 12/18/2012 03:43 PM, Joe Darcy wrote:

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Alan Bateman
On 18/12/2012 20:43, Joe Darcy wrote: Hello, Please review these changes to add core reflection support to recognize default methods: 8005042 Add Method.isDefault to core reflection http://cr.openjdk.java.net/~darcy/8005042.0/ Thanks, -Joe This looks okay to me except that I would

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Mike Duigou
Looks good to me. Will a JLS reference be added in when defined? It seems kind of odd to mention JLS and not provide a ref. Nice use of annotations in the test. I will steal this technique and can think of tests where I should have used it. Mike On Dec 18 2012, at 12:43 , Joe Darcy wrote: >

Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Remi Forax
On 12/18/2012 09:43 PM, Joe Darcy wrote: Hello, Please review these changes to add core reflection support to recognize default methods: 8005042 Add Method.isDefault to core reflection http://cr.openjdk.java.net/~darcy/8005042.0/ Thanks, -Joe Looks good. RĂ©mi

JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Joe Darcy
Hello, Please review these changes to add core reflection support to recognize default methods: 8005042 Add Method.isDefault to core reflection http://cr.openjdk.java.net/~darcy/8005042.0/ Thanks, -Joe