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 set.

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

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 +++ b/src/

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-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 f

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

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: JDK 8 RFR for JDK-8023471,,Add compatibility note to AnnotatedElement

2013-12-04 Thread Joel Borggren-Franck
Hi Joe, Stuart, I think this looks good enough but Stuart has a point. Perhaps add that it is a binary compatible change to make an annotation type repeatable to begin with. cheers /Joel On 2013-12-04, Stuart Marks wrote: > Hi Joe, > > The changes look sensible to me. Funnily enough, when I re

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 t

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 annotat

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 pac

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 revie

Re: clarification on docs of Class#getInterfaces() and Class#getGenericInterfaces() needed?

2014-03-14 Thread Joel Borggren-Franck
Hi Jochen, On 2014-03-10, Jochen Theodorou wrote: > > You basically find this sentence for Class#getInterfaces() and > Class#getGenericInterfaces() "If this object represents a class, the > return value is an array containing objects representing all > interfaces implemented by the class." > > T

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 neighbor

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) >

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 > wrote: > > > On 2014-05-15, Paul Sandoz wrote: > >> > >> The non test code looks good to me: > >>

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: 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 > 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 convoluted it

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. Core

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-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? >

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]: http://mail.openjdk.java.net/pipermail/enhanced-m

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 i

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 F

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

2013-07-22 Thread Joel Borggren-Franck
rcontent.com/u/5812451/yl153753/7184826/webrev.01/index.html > > Thanks, > Amy > > > On 7/10/13 10:17 PM, Joel Borggren-Franck wrote: > >Hi Amy, Tristan, > > > >I'm not a Reviewer kind of reviewer, but I've started to look at the > >code and can

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. Th

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-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: > > > > http://cr.openjdk.java.net/~plevart/jdk8-tl/Annotation

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 changin

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.open

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

2013-09-04 Thread Joel Borggren-Franck
, 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 to update the spec to match the long-standing > implement

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 Cla

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

2013-09-05 Thread Joel Borggren-Franck
Hi Florian, On 2013-09-05, Florian Weimer wrote: > On 09/04/2013 03:55 PM, Joel Borggren-Franck wrote: > >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/ >

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 wrote: > > > >>The issue is that since we added static methods to interfaces those have > >>erroneously been reflected in getMethods of implementing classes. Thi

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 a

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 refl

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 > >

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] mea

Re: RFR: 8025502: Exclude tests due to JDK-8025427

2013-09-27 Thread Joel Borggren-Franck
Hi Looks good. I've run the jdk_tools tests and these gets excluded. I'll sponsor this for you. cheers /Joel On 2013-09-27, Erik Helin wrote: > Hi all, > > Joel found a small issue with the patch, the paths were slightly wrong. > > An updated webrev is located at: > http://cr.openjdk.java.net

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

2013-09-30 Thread Joel Borggren-Franck
Erik, Joe, Robert, Thanks for the reviews, just pushed this to TL cheers /Joel

Re: Review request for JDK-8021398: j.l.r.Parameter.getAnnotatedType().getType() for not annotated use of type returns null

2013-10-01 Thread Joel Borggren-Franck
Hi Eric, Thanks for fixing this. On 2013-10-01, Eric McCorkle wrote: > Hello, please review this simple patch which fixes a problem in the type > annotations handling API. This manifests as a problem with both > j.l.r.Parameter.getAnnotatedType().getType() as well as > j.l.r.Executable.getAnnota

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 for

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: > > webr