Re: RFR: JDK-8054987: (reflect) Add sharing of annotations between instances of Executable

2014-08-18 Thread Joel Borggren-Franck
Hi Peter, Joe, On 2014-08-14, Peter Levart wrote: On 08/14/2014 08:47 AM, Joel Borggren-Franck wrote: On 2014-08-13, Joe Darcy wrote: Hi Joel, Does your changeset alter the support (or non-support) of redefining an annotation? Hi Joe, It does not interact with the current non-support

Re: RFR: JDK-8054987: (reflect) Add sharing of annotations between instances of Executable

2014-08-14 Thread Joel Borggren-Franck
On 2014-08-13, Joe Darcy wrote: Hi Joel, Does your changeset alter the support (or non-support) of redefining an annotation? Hi Joe, It does not interact with the current non-support and I am convinced it wont hinder us in improving the situation. cheers /Joel

Re: RFR: JDK-8044629: (reflect) Constructor.getAnnotatedReceiverType() returns wrong value

2014-08-13 Thread Joel Borggren-Franck
Hi Paul, On 2014-06-24, Paul Sandoz wrote: On Jun 17, 2014, at 6:52 PM, Joel Borggrén-Franck joel.fra...@oracle.com wrote: Can I get a review for this fix and javadoc clarification for https://bugs.openjdk.java.net/browse/JDK-8044629 +1 I never quite realised just how

RFR: JDK-8054987: (reflect) Add sharing of annotations between instances of Executable

2014-08-13 Thread Joel Borggren-Franck
Hi all, Cleaning out the patch queue, I found this small patch that adds sharing of conceptually immutable annotation maps between instances of Executable representing the same executable. In short, Method/Constructor contain one bit of mutable state, if they have been set accessible or not.

Re: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

2014-06-12 Thread Joel Borggren-Franck
Andrej, Joe, Peter, Thanks for looking at this, fix pushed yesterday. Peter, I filed a follow up: https://bugs.openjdk.java.net/browse/JDK-804 cheers /Joel

Re: RFR (S) 9 and 8u: JDK-8038994: AnnotatedType.getType() of a TypeVariable boundary without annotations return null

2014-05-16 Thread Joel Borggren-Franck
On 2014-05-15, Paul Sandoz wrote: The non test code looks good to me: Not totally sure about the test approach: 48 @Test(dataProvider = data) 49 public void testClass(Class? c, String method) throws Exception { 50 if (c.getTypeParameters().length == 0) 51

Re: RFR (S) 9 and 8u: JDK-8038994: AnnotatedType.getType() of a TypeVariable boundary without annotations return null

2014-05-16 Thread Joel Borggren-Franck
Thanks for the review Paul. Fix pushed cheers /Joel On 2014-05-16, Paul Sandoz wrote: On May 16, 2014, at 10:53 AM, Joel Borggren-Franck joel.fra...@oracle.com wrote: On 2014-05-15, Paul Sandoz wrote: The non test code looks good to me: Not totally sure about the test approach

RFR (S) 9 and 8u: JDK-8038994: AnnotatedType.getType() of a TypeVariable boundary without annotations return null

2014-05-14 Thread Joel Borggren-Franck
Hi, Here is a fix for: https://bugs.openjdk.java.net/browse/JDK-8038994 In short, getAnnotatedFoo.getType() is supposed to return the same Type as getGenericFoo(). This wasn't the case for a type variable bound without an annotation previously. Also cleaned up an allocation while in the

Re: [7u backport] RFR: 7122142: (ann) Race condition between isAnnotationPresent and getAnnotations

2014-02-27 Thread Joel Borggren-Franck
Hi, I looked at webrev.1. Looks good. cheers /Joel On 2014-02-25, dmeetry degrave wrote: Thanks for looking at this, Peter! On 02/24/2014 04:42 PM, Peter Levart wrote: Hi Dmeetry, On 02/22/2014 01:22 PM, dmeetry degrave wrote: Hi all, I would like to ask for a review of combined

Re: (reflect) Accessing members of inner annotations types

2014-01-08 Thread Joel Borggren-Franck
Hi Peter, On 2014-01-03, Peter Levart wrote: On 01/03/2014 03:52 PM, Peter Levart wrote: This is would be all right until such proxy class (com.sun.proxy.$Proxy1 in our example) has to access some package-private types in some specific package. This happens in your Named.List annotation

Re: (reflect) Accessing members of inner annotations types

2014-01-08 Thread Joel Borggren-Franck
On 2014-01-08, Peter Levart wrote: On 01/08/2014 01:00 PM, Joel Borggren-Franck wrote: Perhaps an alternative would be to check if the interface a proxy is proxying is nested. In that case use the outermost interface's access level for the package calculation. This would probably lead

Re: Review request for 8021368: Launch of Java Web Start app fails with ClassCircularityError exception

2013-12-16 Thread Joel Borggren-Franck
Hi Mandy, Looks good. cheers /Joel On 2013-12-14, Mandy Chung wrote: Hi Peter, Thanks for the review. This code path is critical in this core reflection implementation and I want to resolve this bug with a low risk fix in an update release and thus the proposed fix. Thanks for the

Re: hg: jdk8/tl/jdk: 7194897: JSR 292: Cannot create more than 16 instances of an anonymous class; ...

2013-11-25 Thread Joel Borggren-Franck
Hi Peter, I filed: https://bugs.openjdk.java.net/browse/JDK-8029100 Thanks! cheers /Joel On 2013-11-05, Peter Levart wrote: Hi John, Speaking of names, the following test: package pkg; public class Test { public static void main(String[] args) { Runnable r = () -

Re: RFR: 8027470: AnnotationSupport uses == rather than .equals to compare Class objects

2013-11-17 Thread Joel Borggren-Franck
Hi all, On 2013-11-16, Remi Forax wrote: On 11/15/2013 04:21 AM, Joseph Darcy wrote: Hello, Catching up on email, the specification of java.lang.Class does not explicitly promise that its notion of equality must be identity for all time. Therefore, while not required for today's

RFR: JDK-8027413: Clarify javadoc for j.l.a.Target and j.l.a.ElementType

2013-11-15 Thread Joel Borggren-Franck
Hi Please review this javadoc clarification for j.l.annnotation.Target and j.l.annotation.ElementType as part of the type annotations work. Webrev: http://cr.openjdk.java.net/~jfranck/8027413/webrev.00/ JBS:https://bugs.openjdk.java.net/browse/JDK-8027413 This is based on the update to JLS

Re: JDK 8 RFR: more core libs raw warnings cleanup

2013-11-12 Thread Joel Borggren-Franck
Hi Joe, Looks good, but your mailer trashes the patch. Please fix your mailer linewrap settings. cheers /Joel On 2013-11-12, Joe Darcy wrote: Hello, Please review the patch below which would remove another batch of raw type javac lint warnings from the core libraries. No signatures of

Re: JDK 8 RFR: more core libs raw warnings cleanup

2013-11-12 Thread Joel Borggren-Franck
This also allows you to get rid of the raw type suppression I think, the attached code compiles. Thanks Remi, cheers /Joel diff --git a/src/share/classes/java/lang/reflect/Proxy.java b/src/share/classes/java/lang/reflect/Proxy.java --- a/src/share/classes/java/lang/reflect/Proxy.java +++

Re: declaring class of a default method Was: Bug 8027734

2013-11-12 Thread Joel Borggren-Franck
Hi Yumin, Basically this is due to a difference in declaring class for a Method representing a default method vs a normal Method. On 2013-11-11, Yumin Qi wrote: Hi, Joel This bug is a SQE testing bug, see https://bugs.openjdk.java.net/browse/JDK-8027734

RFR: JDK-8028055: (reflect) invoking Method/Constructor in anonymous classes breaks with -Dsun.reflect.noInflation=true

2013-11-08 Thread Joel Borggren-Franck
Hi Please review fix for: https://bugs.openjdk.java.net/browse/JDK-8028055 (reflect) invoking Method/Constructor in anonymous classes breaks with -Dsun.reflect.noInflation=true As Peter observed here [1] the current fix is incomplete as it does not work when -Dsun.reflect.noInflation=true is

Re: RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

2013-10-10 Thread Joel Borggren-Franck
Hi, Joe, Paul, agreed the test could be better. Improved it (without using streams) and also added a bug id tag to the old Arrays.newInstance test. Thanks for the comments. Webrev here: http://cr.openjdk.java.net/~jfranck/7044282/webrev.01/ cheres /Joel On 2013-10-09, Joel Borggren-Franck

Re: RFR: Lambda 8026213: Reflection support for private methods in interfaces

2013-10-10 Thread Joel Borggren-Franck
Hi Karen, I agree with the others, the code looks good though I like Paul's suggestion. Silly question perhaps, do you know if we have good test coverage on actually reflectively invoking a Method more than 15 times? cheers /Joel On 2013-10-09, Karen Kinnear wrote: Please review: webrev:

RFR: 7044282: (reflect) Class.forName and Array.newInstance are inconsistent regarding multidimensional arrays

2013-10-09 Thread Joel Borggren-Franck
Hi Please review this spec update and test for getting array classes and instances of more dimensions than the class file can express or the VM can handle. Array.newInstance have a test for arrays of more dimensions than 255, this patch adds a test for Class.forName as well. Also the javadoc

Re: JDK-8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions

2013-09-24 Thread Joel Borggren-Franck
Hi Eric, Some feedback: Executable.java: 299 * (i) The number of parameters (parameter_count) is wrong for the method What is wrong in this case? Do you mean inconsistent with the signature? 302 * (iv) A parameter's name is , or contains an illegal character [0] What does [0] mean

Re: RFR: 8009719: core reflection should get type annotation data from the VM lazily

2013-09-23 Thread Joel Borggren-Franck
Hi, On 2013-09-22, Robert Lougher wrote: Hi, Not a reviewer, but in Field.c the parameter to getTypeAnnotationBytes0 is a field not a method. Thanks, will rename that parameter before pushing. cheers /Joel

Re: RFR: 8009719: core reflection should get type annotation data from the VM lazily

2013-09-23 Thread Joel Borggren-Franck
Hi all, Updated webrev: http://cr.openjdk.java.net/~jfranck/8009719/webrev.02/ Adds Field.c to make/java/java/FILES_c.gmk (old build) Renames parameter in Field.c from method to field Thanks for the suggestions/fixes! cheers /Joel On 2013-09-21, Joel Borggren-Franck wrote: Hi A while ago

RFR: 8009719: core reflection should get type annotation data from the VM lazily

2013-09-21 Thread Joel Borggren-Franck
Hi A while ago [1] I introduced an extra field in to j.l.r.Method, j.l.r.Constructor, and j.l.r.Field in order to support reflection for type annotations. These fields were intended to be removed later, they were there to make the coordination between VM and libraries easier when implementing

RFR: 8007072: Update Core Reflection for Type Annotations to match latest spec

2013-09-17 Thread Joel Borggren-Franck
Hi, Here is an update to the javadoc and implementation of reflection for type annotations. The biggest change is an update to the javadoc of Class and Executable to match return types of the getGeneric* functions. That is, when getGenericFoo returns an empty array getAnnotatedFoo should return

Re: RFR: 8009411 : getMethods should not inherit static methods from interfaces

2013-09-11 Thread Joel Borggren-Franck
On 2013-09-09, Remi Forax wrote: On 09/09/2013 07:27 PM, Joel Borggrén-Franck wrote: On 9 sep 2013, at 19:00, Joel Borggrén-Franck joel.fra...@oracle.com wrote: The issue is that since we added static methods to interfaces those have erroneously been reflected in getMethods of implementing

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

2013-09-04 Thread Joel Borggren-Franck
of results -- I also updated the test to cover all four cases. cheers /Joel On 2013-08-26, Joel Borggren-Franck wrote: Hi, Please review doc fix and test for http://bugs.sun.com/view_bug.do?bug_id=5047859 http://cr.openjdk.java.net/~jfranck/5047859/webrev.00/ This is a spec change

RFR: 4987375: (reflect) Class.get{Declared}Method{s} does not return clone() for array type

2013-09-04 Thread Joel Borggren-Franck
Hi, Please review fix for: http://bugs.sun.com/view_bug.do?bug_id=4987375 Webrev: http://cr.openjdk.java.net/~jfranck/4987375/webrev.01/ Specdiff: http://cr.openjdk.java.net/~jfranck/4987375/specdiff/java/lang/Class.html There are two issues here, - First a getInterfaces() call on an array

RFR: 5047859 : (reflect) Class.getField can't find String[].length

2013-08-26 Thread Joel Borggren-Franck
Hi, Please review doc fix and test for http://bugs.sun.com/view_bug.do?bug_id=5047859 http://cr.openjdk.java.net/~jfranck/5047859/webrev.00/ This is a spec change to update the spec to match the long-standing implementation. There is also a clarification of getFields() javadoc without

Re: RFR: 5047859 : (reflect) Class.getField can't find String[].length

2013-08-26 Thread Joel Borggren-Franck
Hi Florian, Thanks for the comments, On 2013-08-26, Florian Weimer wrote: On 08/26/2013 02:39 PM, Joel Borggren-Franck wrote: Hi, Please review doc fix and test for http://bugs.sun.com/view_bug.do?bug_id=5047859 http://cr.openjdk.java.net/~jfranck/5047859/webrev.00/ This is a spec

Re: RFR JDK-8022721 : AnnotationTypeDeadlockTest.java throws java.lang.IllegalStateException: unexpected condition

2013-08-19 Thread Joel Borggren-Franck
Hi Peter, Looks good to me too. cheers /Joel On 2013-08-17, Alan Bateman wrote: On 17/08/2013 14:06, Peter Levart wrote: Hi, Please review the fix for a test that tries to provoke a deadlock when parsing annotations:

RFR: 8022343: j.l.Class.getAnnotatedSuperclass() doesn't return null in some cases

2013-08-16 Thread Joel Borggren-Franck
Hi Please review this small fix for a type annotation reflection issue. The javadoc spec for Class.getAnnotatedSuperclass says: * If this Class represents either the Object class, an interface type, an * array type, a primitive type, or void, the return value is null. The patch fixes this.

Re: RFR JDK-8011940 : java.lang.Class.getAnnotations() always enters synchronized method

2013-08-12 Thread Joel Borggren-Franck
Hi Peter, Thank you for looking in to this! On 2013-08-11, Peter Levart wrote: On 08/07/2013 06:42 PM, Aleksey Shipilev wrote: Hi Peter, On 08/07/2013 08:18 PM, Peter Levart wrote: http://cr.openjdk.java.net/~plevart/jdk8-tl/AnnotationData/webrev.01/ Yeah, looks familiar. The install

Re: Order of annotation declarations

2013-07-22 Thread Joel Borggren-Franck
Hi, On 2013-07-19, Kasper Nielsen wrote: Hi, I haven't been able to find any info on this. but is [Class|AnnotatedMember].getAnnotations() required to return the annotations is order of declaration? For example, if I have the following definition @First @Second public class Foo{}

Re: Code Review Request: More tests for 7184826: (reflect) Add support for Project Lambda concepts in core reflection

2013-07-10 Thread Joel Borggren-Franck
Hi Amy, Tristan, I'm not a Reviewer kind of reviewer, but I've started to look at the code and can sponsor this. Some comments on test/java/lang/reflect/Method/invoke/DefaultStaticTest.java: As there are a lot of non-public top-level classes perhaps this test should be in it own directory. It

Re: Non Inherited repeated annotations should not be searched from child Class

2013-07-08 Thread Joel Borggren-Franck
Hi, Thanks for reporting this. As Alex wrote on the spec list [1] he has clarified the spec. There is a bug filed for this but I don't have time to work on it at the moment. I'll get around to fixing it in the not too distant future though. [1]: