Re: RFR: 8210943: Hiding of inner classes not resolved properly
Huh… so even within the single array returned from a single call to Classes.getClasses() we can have two classes of the same short name? I foolishly presumed this wouldn’t be the case. I guess in this case the full solution would indeed to provide an ordering on the classes, first on the name, then on whether one’s declaring class is a subclass of the other (I guess the number of superclasses is also a good proxy for that.) Attila. > On 2018. Dec 1., at 19:12, Hannes Wallnöfer > wrote: > > Attila, the subclass-to-superclass traversal is actually not done in dynalink > but directly in java.lang.Class.getClasses(). So Sundar has a point in that > my code relies on implementation rather than specified behaviour of > Class.getClasses(). > > Now I do think it is highly unlikely that the order of classes returned by > Classes.getClasses() would change in a future release. That would certainly > break a lot of other code. Furthermore, if the order was reversed > (superclasses to subclasses) that change would be caught by the test. The > only change that could go unnoticed would be classes returned in random > order, which I don’t think is a real concern. But of course we could choose > to be defensive and add code that guards against it. > > Hannes > > >> Am 01.12.2018 um 13:05 schrieb Attila Szegedi : >> >> The relevant Dynalink algorithm processes the class before moving on to >> superclass, so Hannes fix is correct in that we won’t stomp over a subclass’ >> inner class (inserted into the map earlier) with the superclass’ inner class >> (encountered later by the algorithm). So yeah, getClasses() doesn’t specify >> an order, but the Dynalink code has a subclass-towards-superclass traversal >> order. >> >> Attila. >> >>> On 2018. Dec 1., at 7:13, Sundararajan Athijegannathan >>> wrote: >>> >>> Class.getClasses() javadoc does not mention anything about order of classes >>> returned. >>> >>> https://docs.oracle.com/javase/10/docs/api/java/lang/Class.html#getClasses() >>> >>> Do we need a check using Class.getDeclaringClass() or do I something here? >>> >>> Thanks, >>> -Sundar >>> >>> On 30/11/18, 4:44 PM, Attila Szegedi wrote: +1. Thanks for fixing this. > On 2018. Nov 29., at 18:01, Hannes > Wallnöfer wrote: > > Please review: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8210943 > Webrev: http://cr.openjdk.java.net/~hannesw/8210943/webrev.00/ > > AccessibleMembersLookup#lookupAccessibleMembers adds all nested classes > returned by Class.getClasses(), but these may contain inherited classes > that are shadowed and thus not visible from the current class. The > solution is to make sure we use the first inner class with any given name. > > Thanks, > Hannes >> >
Re: RFR: 8210943: Hiding of inner classes not resolved properly
Attila, the subclass-to-superclass traversal is actually not done in dynalink but directly in java.lang.Class.getClasses(). So Sundar has a point in that my code relies on implementation rather than specified behaviour of Class.getClasses(). Now I do think it is highly unlikely that the order of classes returned by Classes.getClasses() would change in a future release. That would certainly break a lot of other code. Furthermore, if the order was reversed (superclasses to subclasses) that change would be caught by the test. The only change that could go unnoticed would be classes returned in random order, which I don’t think is a real concern. But of course we could choose to be defensive and add code that guards against it. Hannes > Am 01.12.2018 um 13:05 schrieb Attila Szegedi : > > The relevant Dynalink algorithm processes the class before moving on to > superclass, so Hannes fix is correct in that we won’t stomp over a subclass’ > inner class (inserted into the map earlier) with the superclass’ inner class > (encountered later by the algorithm). So yeah, getClasses() doesn’t specify > an order, but the Dynalink code has a subclass-towards-superclass traversal > order. > > Attila. > >> On 2018. Dec 1., at 7:13, Sundararajan Athijegannathan >> wrote: >> >> Class.getClasses() javadoc does not mention anything about order of classes >> returned. >> >> https://docs.oracle.com/javase/10/docs/api/java/lang/Class.html#getClasses() >> >> Do we need a check using Class.getDeclaringClass() or do I something here? >> >> Thanks, >> -Sundar >> >> On 30/11/18, 4:44 PM, Attila Szegedi wrote: >>> +1. Thanks for fixing this. >>> On 2018. Nov 29., at 18:01, Hannes Wallnöfer wrote: Please review: Bug: https://bugs.openjdk.java.net/browse/JDK-8210943 Webrev: http://cr.openjdk.java.net/~hannesw/8210943/webrev.00/ AccessibleMembersLookup#lookupAccessibleMembers adds all nested classes returned by Class.getClasses(), but these may contain inherited classes that are shadowed and thus not visible from the current class. The solution is to make sure we use the first inner class with any given name. Thanks, Hannes >
Re: RFR: 8210943: Hiding of inner classes not resolved properly
The relevant Dynalink algorithm processes the class before moving on to superclass, so Hannes fix is correct in that we won’t stomp over a subclass’ inner class (inserted into the map earlier) with the superclass’ inner class (encountered later by the algorithm). So yeah, getClasses() doesn’t specify an order, but the Dynalink code has a subclass-towards-superclass traversal order. Attila. > On 2018. Dec 1., at 7:13, Sundararajan Athijegannathan > wrote: > > Class.getClasses() javadoc does not mention anything about order of classes > returned. > > https://docs.oracle.com/javase/10/docs/api/java/lang/Class.html#getClasses() > > Do we need a check using Class.getDeclaringClass() or do I something here? > > Thanks, > -Sundar > > On 30/11/18, 4:44 PM, Attila Szegedi wrote: >> +1. Thanks for fixing this. >> >>> On 2018. Nov 29., at 18:01, Hannes Wallnöfer >>> wrote: >>> >>> Please review: >>> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210943 >>> Webrev: http://cr.openjdk.java.net/~hannesw/8210943/webrev.00/ >>> >>> AccessibleMembersLookup#lookupAccessibleMembers adds all nested classes >>> returned by Class.getClasses(), but these may contain inherited classes >>> that are shadowed and thus not visible from the current class. The solution >>> is to make sure we use the first inner class with any given name. >>> >>> Thanks, >>> Hannes
