On Sun, 15 Nov 2020 19:49:57 GMT, Joel Borggrén-Franck <jfra...@openjdk.org> 
wrote:

>> Rafael Winterhalter has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR. The pull request 
>> contains one new commit since the last revision:
>> 
>>   8202471: A method's or constructor's owner type might carry annotations on 
>> its potential type parameters but is never represented as parameterized type 
>> what makes these parameters inaccessible at runtime, despite the presence of 
>> parameter type annotations.
>
> test/jdk/java/lang/annotation/typeAnnotations/TestReceiverTypeParameterizedConstructor.java
>  line 52:
> 
>> 50:         Inner(TestReceiverTypeParameterizedConstructor<@TypeAnnotation 
>> T> TestReceiverTypeParameterizedConstructor.this) { }
>> 51:     }
>> 52: 
> 
> It might be good to add a case where the annotated type variable isn't on the 
> immediate outer type like:
> 
>     class Inner2 {
>         class  TwiceInner {
>             
> TwiceInner(TestReceiverTypeParameterizedConstructor<@TypeAnnotation T>.Inner2 
> TestReceiverTypeParameterizedConstructor.Inner2.this) { }
>         }
>     }
> 
> Same for the method test.

I added two tests for it.

> src/java.base/share/classes/java/lang/reflect/Executable.java line 699:
> 
>> 697:         }
>> 698:         Class<?> c = getDeclaringClass();
>> 699:         TypeVariable<?>[] v = c.getTypeParameters();
> 
> Same here, can this be pushed down to a common method that either returns a 
> Class or resolves the appropriate Type instance?

I moved it to an annotation factory class and avoided the import.

> src/java.base/share/classes/java/lang/reflect/Constructor.java line 660:
> 
>> 658:         }
>> 659: 
>> 660:         TypeVariable<?>[] v = enclosingClass.getTypeParameters();
> 
> Can you push down most or all of this to resolveOwner? It can then either 
> return a Class instance or an appropriate Type instance.

Pushed it down to the factory.

> src/java.base/share/classes/java/lang/reflect/Constructor.java line 664:
> 
>> 662:         Type t;
>> 663:         if (o != null || v.length > 0) {
>> 664:             t = ParameterizedTypeImpl.make(enclosingClass, v, o);
> 
> I think these should be instantiated by a GenericsFactory. Ideally you 
> shouldn't have to import the reflective object directly.

I added it to the existing generics factory but as a static method since the 
instance values are not really required.

-------------

PR: https://git.openjdk.java.net/jdk/pull/851

Reply via email to