Quantcast

JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

joe darcy
Hello,

Please review the webrev to fix

     JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value
     http://cr.openjdk.java.net/~darcy/8074977.3/

To condense a complicated situation, there are cases where the number of
parameters present for a constructor in source code and the number of
parameters present for that constructor in the class file differ. One of
those cases occurs for the constructor of a non-static member class [1]
where there is a leading parameter to accept the outer this argument.

Bug JDK-8074977 reports on a situation where the type annotations on
constructor parameters are incorrectly reported. Essentially, an
off-by-one error is the cause since the annotation information is stored
with respect to the number of parameters present in the source and an
additional parameter is present at runtime.

An analogous situation exists for declaration annotations and
constructor parameters, declaration annotations being the traditional
flavor of annotations.

Type annotations and declaration annotations are read using different
APIs so require separate fixes to detect the additional parameter and
make the necessary adjustments in the returned information.

The regression tests cover both the type annotation reading API and the
two ways of reading declaration annotations on parameters, calling
getParameterAnnotations on the constructor or call getParameters on the
constructor and then calling getAnnotations on each parameter. The
getParameters API was added in JDK 8.

Static member and non-static member classes are used as test cases, as
are constructors with and without generic type information.

Thanks,

-Joe

[1]
https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

Peter Levart
Hi Joe,

So enum classes, anonymous classes and local classes don't have a
standard way of passing synthetic/implicit constructor parameters? Do
various compilers do it differently than javac?

It's unfortunate but if # of parameter annotations arrays is less than
the # of parameters, Parameter.getAnnotations() may return wrong
annotations or throw IndexOutOfBoundException for enums, anonymous and
local classes. Can't we do anything for them? At least for code compiled
by javac?

For example, javac compiles enum classes so that they always prefix
constructor source parameters with a pair of (String name, int ordinal,
...) and so do anonymous enum subclasses provided by enum constants
(i.e. clazz.isAnonymousClass() && clazz.getSuperclass().isEnum())

Non-static local classes are compiled so that constructor source
parameters are prefixed with a single parameter (OuterClass
outerInstance, ...), while any captured variables follow source parameters

Static local classes are compiled so that constructor source parameters
come 1st, followed by any captured variables.

etc...

Regards, Peter


On 05/19/2017 11:31 PM, joe darcy wrote:

> Hello,
>
> Please review the webrev to fix
>
>     JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong
> value
>     http://cr.openjdk.java.net/~darcy/8074977.3/
>
> To condense a complicated situation, there are cases where the number
> of parameters present for a constructor in source code and the number
> of parameters present for that constructor in the class file differ.
> One of those cases occurs for the constructor of a non-static member
> class [1] where there is a leading parameter to accept the outer this
> argument.
>
> Bug JDK-8074977 reports on a situation where the type annotations on
> constructor parameters are incorrectly reported. Essentially, an
> off-by-one error is the cause since the annotation information is
> stored with respect to the number of parameters present in the source
> and an additional parameter is present at runtime.
>
> An analogous situation exists for declaration annotations and
> constructor parameters, declaration annotations being the traditional
> flavor of annotations.
>
> Type annotations and declaration annotations are read using different
> APIs so require separate fixes to detect the additional parameter and
> make the necessary adjustments in the returned information.
>
> The regression tests cover both the type annotation reading API and
> the two ways of reading declaration annotations on parameters, calling
> getParameterAnnotations on the constructor or call getParameters on
> the constructor and then calling getAnnotations on each parameter. The
> getParameters API was added in JDK 8.
>
> Static member and non-static member classes are used as test cases, as
> are constructors with and without generic type information.
>
> Thanks,
>
> -Joe
>
> [1]
> https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

Peter Levart
I see that local classes in static context don't have STATIC modifier.
The same for anonymous classes. So there's no reliable information in
the class file whether some local class is from the static context or not?

Regards, Peter

On 05/20/2017 08:00 PM, Peter Levart wrote:

> Hi Joe,
>
> So enum classes, anonymous classes and local classes don't have a
> standard way of passing synthetic/implicit constructor parameters? Do
> various compilers do it differently than javac?
>
> It's unfortunate but if # of parameter annotations arrays is less than
> the # of parameters, Parameter.getAnnotations() may return wrong
> annotations or throw IndexOutOfBoundException for enums, anonymous and
> local classes. Can't we do anything for them? At least for code
> compiled by javac?
>
> For example, javac compiles enum classes so that they always prefix
> constructor source parameters with a pair of (String name, int
> ordinal, ...) and so do anonymous enum subclasses provided by enum
> constants (i.e. clazz.isAnonymousClass() &&
> clazz.getSuperclass().isEnum())
>
> Non-static local classes are compiled so that constructor source
> parameters are prefixed with a single parameter (OuterClass
> outerInstance, ...), while any captured variables follow source parameters
>
> Static local classes are compiled so that constructor source
> parameters come 1st, followed by any captured variables.
>
> etc...
>
> Regards, Peter
>
>
> On 05/19/2017 11:31 PM, joe darcy wrote:
>> Hello,
>>
>> Please review the webrev to fix
>>
>>     JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong
>> value
>> http://cr.openjdk.java.net/~darcy/8074977.3/
>>
>> To condense a complicated situation, there are cases where the number
>> of parameters present for a constructor in source code and the number
>> of parameters present for that constructor in the class file differ.
>> One of those cases occurs for the constructor of a non-static member
>> class [1] where there is a leading parameter to accept the outer this
>> argument.
>>
>> Bug JDK-8074977 reports on a situation where the type annotations on
>> constructor parameters are incorrectly reported. Essentially, an
>> off-by-one error is the cause since the annotation information is
>> stored with respect to the number of parameters present in the source
>> and an additional parameter is present at runtime.
>>
>> An analogous situation exists for declaration annotations and
>> constructor parameters, declaration annotations being the traditional
>> flavor of annotations.
>>
>> Type annotations and declaration annotations are read using different
>> APIs so require separate fixes to detect the additional parameter and
>> make the necessary adjustments in the returned information.
>>
>> The regression tests cover both the type annotation reading API and
>> the two ways of reading declaration annotations on parameters,
>> calling getParameterAnnotations on the constructor or call
>> getParameters on the constructor and then calling getAnnotations on
>> each parameter. The getParameters API was added in JDK 8.
>>
>> Static member and non-static member classes are used as test cases,
>> as are constructors with and without generic type information.
>>
>> Thanks,
>>
>> -Joe
>>
>> [1]
>> https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

Remi Forax
In reply to this post by Peter Levart
Hi Joe,
we had the same issue with ASM, we solve it without trying to detect every corner cases but by just comparing the number of parameter from the descriptor and the number of parameter (type?) annotations,
if there are less annotations, we add fake annotations at the starts of the array [1].

I don't remember the exact details but i'm pretty sure that at some point in the past ecj (the eclipse compiler) was generating more than one reference if you had an anonymous class inside an anonymous class.

ASM reports a special fake annotation "Ljava/lang/Synthetic;" so users can see that this is a synthetic parameter but i don't think it's a good idea to do that in the reflection API, ASM is more low level.

cheers,
Rémi

[1] http://websvn.ow2.org/filedetails.php?repname=asm&path=%2Ftrunk%2Fasm%2Fsrc%2Forg%2Fobjectweb%2Fasm%2FClassReader.java, in method readParameterAnnotations().

----- Mail original -----
> De: "Peter Levart" <[hidden email]>
> À: "joe darcy" <[hidden email]>, "core-libs-dev" <[hidden email]>
> Envoyé: Samedi 20 Mai 2017 20:00:25
> Objet: Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

> Hi Joe,
>
> So enum classes, anonymous classes and local classes don't have a
> standard way of passing synthetic/implicit constructor parameters? Do
> various compilers do it differently than javac?
>
> It's unfortunate but if # of parameter annotations arrays is less than
> the # of parameters, Parameter.getAnnotations() may return wrong
> annotations or throw IndexOutOfBoundException for enums, anonymous and
> local classes. Can't we do anything for them? At least for code compiled
> by javac?
>
> For example, javac compiles enum classes so that they always prefix
> constructor source parameters with a pair of (String name, int ordinal,
> ...) and so do anonymous enum subclasses provided by enum constants
> (i.e. clazz.isAnonymousClass() && clazz.getSuperclass().isEnum())
>
> Non-static local classes are compiled so that constructor source
> parameters are prefixed with a single parameter (OuterClass
> outerInstance, ...), while any captured variables follow source parameters
>
> Static local classes are compiled so that constructor source parameters
> come 1st, followed by any captured variables.
>
> etc...
>
> Regards, Peter
>
>
> On 05/19/2017 11:31 PM, joe darcy wrote:
>> Hello,
>>
>> Please review the webrev to fix
>>
>>     JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong
>> value
>>     http://cr.openjdk.java.net/~darcy/8074977.3/
>>
>> To condense a complicated situation, there are cases where the number
>> of parameters present for a constructor in source code and the number
>> of parameters present for that constructor in the class file differ.
>> One of those cases occurs for the constructor of a non-static member
>> class [1] where there is a leading parameter to accept the outer this
>> argument.
>>
>> Bug JDK-8074977 reports on a situation where the type annotations on
>> constructor parameters are incorrectly reported. Essentially, an
>> off-by-one error is the cause since the annotation information is
>> stored with respect to the number of parameters present in the source
>> and an additional parameter is present at runtime.
>>
>> An analogous situation exists for declaration annotations and
>> constructor parameters, declaration annotations being the traditional
>> flavor of annotations.
>>
>> Type annotations and declaration annotations are read using different
>> APIs so require separate fixes to detect the additional parameter and
>> make the necessary adjustments in the returned information.
>>
>> The regression tests cover both the type annotation reading API and
>> the two ways of reading declaration annotations on parameters, calling
>> getParameterAnnotations on the constructor or call getParameters on
>> the constructor and then calling getAnnotations on each parameter. The
>> getParameters API was added in JDK 8.
>>
>> Static member and non-static member classes are used as test cases, as
>> are constructors with and without generic type information.
>>
>> Thanks,
>>
>> -Joe
>>
>> [1]
>> https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

Peter Levart
Hi Remi

On 05/20/2017 11:09 PM, Remi Forax wrote:

> Hi Joe,
> we had the same issue with ASM, we solve it without trying to detect every corner cases but by just comparing the number of parameter from the descriptor and the number of parameter (type?) annotations,
> if there are less annotations, we add fake annotations at the starts of the array [1].
>
> I don't remember the exact details but i'm pretty sure that at some point in the past ecj (the eclipse compiler) was generating more than one reference if you had an anonymous class inside an anonymous class.
>
> ASM reports a special fake annotation "Ljava/lang/Synthetic;" so users can see that this is a synthetic parameter but i don't think it's a good idea to do that in the reflection API, ASM is more low level.
>
> cheers,
> Rémi
>
> [1] http://websvn.ow2.org/filedetails.php?repname=asm&path=%2Ftrunk%2Fasm%2Fsrc%2Forg%2Fobjectweb%2Fasm%2FClassReader.java, in method readParameterAnnotations().
         // ... This work-around supposes that the synthetic parameters
are the first ones.
         int synthetics = Type.getArgumentTypes(context.desc).length - n;

Not always for local and anonymous classes. Captured values are appended
to the parameter list. Source-level parameters are therefore between the
outer instance and the captured values.

And if local or anonymous class is in the static context, there's no
outer instance parameter, just captured values which are appended.

Here's what I was trying to do but then I found that local and anonymous
classes never have STATIC modifier even if they are from static context:

http://cr.openjdk.java.net/~plevart/jdk9-dev/8074977_ConstructorParameterAnnotations/webrev.01/

If there was a reliable way to determine whether a local or anonymous
class is from static context or not, then this could work for javac
generated code at least.

Regards, Peter

> ----- Mail original -----
>> De: "Peter Levart" <[hidden email]>
>> À: "joe darcy" <[hidden email]>, "core-libs-dev" <[hidden email]>
>> Envoyé: Samedi 20 Mai 2017 20:00:25
>> Objet: Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value
>> Hi Joe,
>>
>> So enum classes, anonymous classes and local classes don't have a
>> standard way of passing synthetic/implicit constructor parameters? Do
>> various compilers do it differently than javac?
>>
>> It's unfortunate but if # of parameter annotations arrays is less than
>> the # of parameters, Parameter.getAnnotations() may return wrong
>> annotations or throw IndexOutOfBoundException for enums, anonymous and
>> local classes. Can't we do anything for them? At least for code compiled
>> by javac?
>>
>> For example, javac compiles enum classes so that they always prefix
>> constructor source parameters with a pair of (String name, int ordinal,
>> ...) and so do anonymous enum subclasses provided by enum constants
>> (i.e. clazz.isAnonymousClass() && clazz.getSuperclass().isEnum())
>>
>> Non-static local classes are compiled so that constructor source
>> parameters are prefixed with a single parameter (OuterClass
>> outerInstance, ...), while any captured variables follow source parameters
>>
>> Static local classes are compiled so that constructor source parameters
>> come 1st, followed by any captured variables.
>>
>> etc...
>>
>> Regards, Peter
>>
>>
>> On 05/19/2017 11:31 PM, joe darcy wrote:
>>> Hello,
>>>
>>> Please review the webrev to fix
>>>
>>>      JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong
>>> value
>>>      http://cr.openjdk.java.net/~darcy/8074977.3/
>>>
>>> To condense a complicated situation, there are cases where the number
>>> of parameters present for a constructor in source code and the number
>>> of parameters present for that constructor in the class file differ.
>>> One of those cases occurs for the constructor of a non-static member
>>> class [1] where there is a leading parameter to accept the outer this
>>> argument.
>>>
>>> Bug JDK-8074977 reports on a situation where the type annotations on
>>> constructor parameters are incorrectly reported. Essentially, an
>>> off-by-one error is the cause since the annotation information is
>>> stored with respect to the number of parameters present in the source
>>> and an additional parameter is present at runtime.
>>>
>>> An analogous situation exists for declaration annotations and
>>> constructor parameters, declaration annotations being the traditional
>>> flavor of annotations.
>>>
>>> Type annotations and declaration annotations are read using different
>>> APIs so require separate fixes to detect the additional parameter and
>>> make the necessary adjustments in the returned information.
>>>
>>> The regression tests cover both the type annotation reading API and
>>> the two ways of reading declaration annotations on parameters, calling
>>> getParameterAnnotations on the constructor or call getParameters on
>>> the constructor and then calling getAnnotations on each parameter. The
>>> getParameters API was added in JDK 8.
>>>
>>> Static member and non-static member classes are used as test cases, as
>>> are constructors with and without generic type information.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>> [1]
>>> https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

joe darcy
In reply to this post by Peter Levart
Hi Peter,


On 5/20/2017 11:00 AM, Peter Levart wrote:
> Hi Joe,
>
> So enum classes, anonymous classes and local classes don't have a
> standard way of passing synthetic/implicit constructor parameters?

Right; there is no standardized or mandated way for compilers to do
that. Such details are intentionally outside of what the JLS specifies.

Since non-static member classes can be referred to and instances
instantiated from outside of the enclosing class, there has to be a
standardized linkage model in that case for separate compilation, etc.
and that is covered in the JLS. This isn't the case for local and
anonymous classes since there isn't a way to refer to such classes from
outside of their immediate declaration context. The instances of such
types can be returned of course and then it is possible for people to
getClass().getConstructors()... their way into the issue.


> Do various compilers do it differently than javac?

IIRC, ecj does compile enum classes differently than javac.

>
> It's unfortunate but if # of parameter annotations arrays is less than
> the # of parameters, Parameter.getAnnotations() may return wrong
> annotations or throw IndexOutOfBoundException for enums, anonymous and
> local classes. Can't we do anything for them? At least for code
> compiled by javac?
>
> For example, javac compiles enum classes so that they always prefix
> constructor source parameters with a pair of (String name, int
> ordinal, ...) and so do anonymous enum subclasses provided by enum
> constants (i.e. clazz.isAnonymousClass() &&
> clazz.getSuperclass().isEnum())
>
> Non-static local classes are compiled so that constructor source
> parameters are prefixed with a single parameter (OuterClass
> outerInstance, ...), while any captured variables follow source parameters

A wrinkle with local classes is that if they occur in a static context,
such as a static initializer block, there is no leading outer instance
parameter.

This is admittedly not a 100% solution to parameter mismatches and
annotations; however, it is an improvement over the current state of
affairs.

Ideally, the code for something like Parameters.getAnnotations could do
something like look at the synthetic bit and pair up natural parameters
with annotations if there was a mismatch in the number of parameters and
annotations. Unfortunately, the Parameters.isSynthetic API wasn't
introduced until JDK 8 and that information doesn't have to be available.

Cheers,

-Joe

>
> Static local classes are compiled so that constructor source
> parameters come 1st, followed by any captured variables.
>
> etc...
>
> Regards, Peter
>
>
> On 05/19/2017 11:31 PM, joe darcy wrote:
>> Hello,
>>
>> Please review the webrev to fix
>>
>>     JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong
>> value
>> http://cr.openjdk.java.net/~darcy/8074977.3/
>>
>> To condense a complicated situation, there are cases where the number
>> of parameters present for a constructor in source code and the number
>> of parameters present for that constructor in the class file differ.
>> One of those cases occurs for the constructor of a non-static member
>> class [1] where there is a leading parameter to accept the outer this
>> argument.
>>
>> Bug JDK-8074977 reports on a situation where the type annotations on
>> constructor parameters are incorrectly reported. Essentially, an
>> off-by-one error is the cause since the annotation information is
>> stored with respect to the number of parameters present in the source
>> and an additional parameter is present at runtime.
>>
>> An analogous situation exists for declaration annotations and
>> constructor parameters, declaration annotations being the traditional
>> flavor of annotations.
>>
>> Type annotations and declaration annotations are read using different
>> APIs so require separate fixes to detect the additional parameter and
>> make the necessary adjustments in the returned information.
>>
>> The regression tests cover both the type annotation reading API and
>> the two ways of reading declaration annotations on parameters,
>> calling getParameterAnnotations on the constructor or call
>> getParameters on the constructor and then calling getAnnotations on
>> each parameter. The getParameters API was added in JDK 8.
>>
>> Static member and non-static member classes are used as test cases,
>> as are constructors with and without generic type information.
>>
>> Thanks,
>>
>> -Joe
>>
>> [1]
>> https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

Peter Levart
In reply to this post by Peter Levart


On 05/20/2017 11:30 PM, Peter Levart wrote:
> http://cr.openjdk.java.net/~plevart/jdk9-dev/8074977_ConstructorParameterAnnotations/webrev.01/
>
> If there was a reliable way to determine whether a local or anonymous
> class is from static context or not, then this could work for javac
> generated code at least.
>

... it works for member classes and for enum classes and enum constant
anonymous subclasses. Only local and anonymous classes are problematic...

Peter


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

Peter Levart
In reply to this post by joe darcy
Hi Joe,

Thanks for explanation.

Regarding your patch. Take a look here:

http://cr.openjdk.java.net/~plevart/jdk9-dev/8074977_ConstructorParameterAnnotations/webrev.01/

I just want to point out that the logic could be centralized for
classical and type annotations. What do you think?


Regards, Peter


On 05/20/2017 11:38 PM, joe darcy wrote:

> Hi Peter,
>
>
> On 5/20/2017 11:00 AM, Peter Levart wrote:
>> Hi Joe,
>>
>> So enum classes, anonymous classes and local classes don't have a
>> standard way of passing synthetic/implicit constructor parameters?
>
> Right; there is no standardized or mandated way for compilers to do
> that. Such details are intentionally outside of what the JLS specifies.
>
> Since non-static member classes can be referred to and instances
> instantiated from outside of the enclosing class, there has to be a
> standardized linkage model in that case for separate compilation, etc.
> and that is covered in the JLS. This isn't the case for local and
> anonymous classes since there isn't a way to refer to such classes
> from outside of their immediate declaration context. The instances of
> such types can be returned of course and then it is possible for
> people to getClass().getConstructors()... their way into the issue.
>
>
>> Do various compilers do it differently than javac?
>
> IIRC, ecj does compile enum classes differently than javac.
>
>>
>> It's unfortunate but if # of parameter annotations arrays is less
>> than the # of parameters, Parameter.getAnnotations() may return wrong
>> annotations or throw IndexOutOfBoundException for enums, anonymous
>> and local classes. Can't we do anything for them? At least for code
>> compiled by javac?
>>
>> For example, javac compiles enum classes so that they always prefix
>> constructor source parameters with a pair of (String name, int
>> ordinal, ...) and so do anonymous enum subclasses provided by enum
>> constants (i.e. clazz.isAnonymousClass() &&
>> clazz.getSuperclass().isEnum())
>>
>> Non-static local classes are compiled so that constructor source
>> parameters are prefixed with a single parameter (OuterClass
>> outerInstance, ...), while any captured variables follow source
>> parameters
>
> A wrinkle with local classes is that if they occur in a static
> context, such as a static initializer block, there is no leading outer
> instance parameter.
>
> This is admittedly not a 100% solution to parameter mismatches and
> annotations; however, it is an improvement over the current state of
> affairs.
>
> Ideally, the code for something like Parameters.getAnnotations could
> do something like look at the synthetic bit and pair up natural
> parameters with annotations if there was a mismatch in the number of
> parameters and annotations. Unfortunately, the Parameters.isSynthetic
> API wasn't introduced until JDK 8 and that information doesn't have to
> be available.
>
> Cheers,
>
> -Joe
>
>>
>> Static local classes are compiled so that constructor source
>> parameters come 1st, followed by any captured variables.
>>
>> etc...
>>
>> Regards, Peter
>>
>>
>> On 05/19/2017 11:31 PM, joe darcy wrote:
>>> Hello,
>>>
>>> Please review the webrev to fix
>>>
>>>     JDK-8074977: Constructor.getAnnotatedParameterTypes returns
>>> wrong value
>>> http://cr.openjdk.java.net/~darcy/8074977.3/
>>>
>>> To condense a complicated situation, there are cases where the
>>> number of parameters present for a constructor in source code and
>>> the number of parameters present for that constructor in the class
>>> file differ. One of those cases occurs for the constructor of a
>>> non-static member class [1] where there is a leading parameter to
>>> accept the outer this argument.
>>>
>>> Bug JDK-8074977 reports on a situation where the type annotations on
>>> constructor parameters are incorrectly reported. Essentially, an
>>> off-by-one error is the cause since the annotation information is
>>> stored with respect to the number of parameters present in the
>>> source and an additional parameter is present at runtime.
>>>
>>> An analogous situation exists for declaration annotations and
>>> constructor parameters, declaration annotations being the
>>> traditional flavor of annotations.
>>>
>>> Type annotations and declaration annotations are read using
>>> different APIs so require separate fixes to detect the additional
>>> parameter and make the necessary adjustments in the returned
>>> information.
>>>
>>> The regression tests cover both the type annotation reading API and
>>> the two ways of reading declaration annotations on parameters,
>>> calling getParameterAnnotations on the constructor or call
>>> getParameters on the constructor and then calling getAnnotations on
>>> each parameter. The getParameters API was added in JDK 8.
>>>
>>> Static member and non-static member classes are used as test cases,
>>> as are constructors with and without generic type information.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>> [1]
>>> https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

joe darcy
Hi Peter,

Yours is a more ambitious attempt to more fully resolve this issue. It
would likely nearly always work in practice, at least for
javac-generated class files.

However, at this stage of JDK 9, I'd be more comfortable with something
like the point fixes in my current patch, deferring a more complete
solution to JDK 10. At that point, I think a two-stage fall back would
be more complete: if the number of parameters differs, first try to see
if java.lang.reflect.Parameter.{isSynthetic, isImplicit} information is
available and if it is, use it accordingly. Otherwise, use the sort of
case analysis present in the patch.

FWIW, on the javax.lang.model side of reflective APIs, in JDK 9 we added
notions in the API to describe these sorts of differences in the
javax.lang.model.util.Elements.Origin enum:

http://download.java.net/java/jdk9/docs/api/javax/lang/model/util/Elements.Origin.html

and the Elements.getOrigin methods which return that type.

Thanks,

-Joe


On 5/20/2017 2:48 PM, Peter Levart wrote:

> Hi Joe,
>
> Thanks for explanation.
>
> Regarding your patch. Take a look here:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/8074977_ConstructorParameterAnnotations/webrev.01/
>
> I just want to point out that the logic could be centralized for
> classical and type annotations. What do you think?
>
>
> Regards, Peter
>
>
> On 05/20/2017 11:38 PM, joe darcy wrote:
>> Hi Peter,
>>
>>
>> On 5/20/2017 11:00 AM, Peter Levart wrote:
>>> Hi Joe,
>>>
>>> So enum classes, anonymous classes and local classes don't have a
>>> standard way of passing synthetic/implicit constructor parameters?
>>
>> Right; there is no standardized or mandated way for compilers to do
>> that. Such details are intentionally outside of what the JLS specifies.
>>
>> Since non-static member classes can be referred to and instances
>> instantiated from outside of the enclosing class, there has to be a
>> standardized linkage model in that case for separate compilation,
>> etc. and that is covered in the JLS. This isn't the case for local
>> and anonymous classes since there isn't a way to refer to such
>> classes from outside of their immediate declaration context. The
>> instances of such types can be returned of course and then it is
>> possible for people to getClass().getConstructors()... their way into
>> the issue.
>>
>>
>>> Do various compilers do it differently than javac?
>>
>> IIRC, ecj does compile enum classes differently than javac.
>>
>>>
>>> It's unfortunate but if # of parameter annotations arrays is less
>>> than the # of parameters, Parameter.getAnnotations() may return
>>> wrong annotations or throw IndexOutOfBoundException for enums,
>>> anonymous and local classes. Can't we do anything for them? At least
>>> for code compiled by javac?
>>>
>>> For example, javac compiles enum classes so that they always prefix
>>> constructor source parameters with a pair of (String name, int
>>> ordinal, ...) and so do anonymous enum subclasses provided by enum
>>> constants (i.e. clazz.isAnonymousClass() &&
>>> clazz.getSuperclass().isEnum())
>>>
>>> Non-static local classes are compiled so that constructor source
>>> parameters are prefixed with a single parameter (OuterClass
>>> outerInstance, ...), while any captured variables follow source
>>> parameters
>>
>> A wrinkle with local classes is that if they occur in a static
>> context, such as a static initializer block, there is no leading
>> outer instance parameter.
>>
>> This is admittedly not a 100% solution to parameter mismatches and
>> annotations; however, it is an improvement over the current state of
>> affairs.
>>
>> Ideally, the code for something like Parameters.getAnnotations could
>> do something like look at the synthetic bit and pair up natural
>> parameters with annotations if there was a mismatch in the number of
>> parameters and annotations. Unfortunately, the Parameters.isSynthetic
>> API wasn't introduced until JDK 8 and that information doesn't have
>> to be available.
>>
>> Cheers,
>>
>> -Joe
>>
>>>
>>> Static local classes are compiled so that constructor source
>>> parameters come 1st, followed by any captured variables.
>>>
>>> etc...
>>>
>>> Regards, Peter
>>>
>>>
>>> On 05/19/2017 11:31 PM, joe darcy wrote:
>>>> Hello,
>>>>
>>>> Please review the webrev to fix
>>>>
>>>>     JDK-8074977: Constructor.getAnnotatedParameterTypes returns
>>>> wrong value
>>>> http://cr.openjdk.java.net/~darcy/8074977.3/
>>>>
>>>> To condense a complicated situation, there are cases where the
>>>> number of parameters present for a constructor in source code and
>>>> the number of parameters present for that constructor in the class
>>>> file differ. One of those cases occurs for the constructor of a
>>>> non-static member class [1] where there is a leading parameter to
>>>> accept the outer this argument.
>>>>
>>>> Bug JDK-8074977 reports on a situation where the type annotations
>>>> on constructor parameters are incorrectly reported. Essentially, an
>>>> off-by-one error is the cause since the annotation information is
>>>> stored with respect to the number of parameters present in the
>>>> source and an additional parameter is present at runtime.
>>>>
>>>> An analogous situation exists for declaration annotations and
>>>> constructor parameters, declaration annotations being the
>>>> traditional flavor of annotations.
>>>>
>>>> Type annotations and declaration annotations are read using
>>>> different APIs so require separate fixes to detect the additional
>>>> parameter and make the necessary adjustments in the returned
>>>> information.
>>>>
>>>> The regression tests cover both the type annotation reading API and
>>>> the two ways of reading declaration annotations on parameters,
>>>> calling getParameterAnnotations on the constructor or call
>>>> getParameters on the constructor and then calling getAnnotations on
>>>> each parameter. The getParameters API was added in JDK 8.
>>>>
>>>> Static member and non-static member classes are used as test cases,
>>>> as are constructors with and without generic type information.
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>> [1]
>>>> https://blogs.oracle.com/darcy/nested,-inner,-member,-and-top-level-classes
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 RFR of JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value

Mandy Chung
In reply to this post by joe darcy

> On May 19, 2017, at 2:31 PM, joe darcy <[hidden email]> wrote:
>
> Hello,
>
> Please review the webrev to fix
>
>    JDK-8074977: Constructor.getAnnotatedParameterTypes returns wrong value
>    http://cr.openjdk.java.net/~darcy/8074977.3/

This fix looks okay to me.  Deferring a more complete solution to JDK 10 is a reasonable choice at this point of the release.

Mandy

Loading...