On Mon, 16 Nov 2020 10:29:48 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.
>
> test/jdk/java/lang/annotation/typeAnnotations/TestReceiverTypeParameterizedConstructorNested.java
>  line 36:
> 
>> 34: import java.lang.reflect.Constructor;
>> 35: 
>> 36: public class TestReceiverTypeParameterizedConstructorNested {
> 
> Suggestion, what I would like to test is that we can walk up the chain and 
> find type parameters on the outermost owner. Same for Method.

I adjusted the test to check for an inner and outer variable for both method 
and constructor.

> src/java.base/share/classes/sun/reflect/generics/factory/CoreReflectionFactory.java
>  line 93:
> 
>> 91:     }
>> 92: 
>> 93:     public static Type ownerType(Class<?> c) {
> 
> This feels somewhat odd. How about, moving these back to Executable and 
> rename `ownerType` to `resolveToType` ? I realise we can't get use 
> getFactory() in executable but as you write, it isn't needed, it should be 
> fine in this case to create it directly as in your v1. The reason is the 
> semantics of this is somewhat unintuitive and is only used in the annotated 
> receiver case.

I moved the code back and it's fairly compact now. I added "owner" to the 
method name since it does not make sense otherwise. Let me know what you think!

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

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

Reply via email to