On Mon, 16 Nov 2020 10:29:48 GMT, Joel Borggrén-Franck <[email protected]>
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