On Fri, 17 Mar 2023 15:38:49 GMT, Doug Simon <dnsi...@openjdk.org> wrote:

>> This PR extends JVMCI with new API (`jdk.vm.ci.meta.Annotated`) for 
>> accessing annotations. The main differences from 
>> `java.lang.reflect.AnnotatedElement` are:
>> * All methods in the `Annotated` interface explicitly specify requested 
>> annotation type(s). That is, there is no equivalent of 
>> `AnnotatedElement.getAnnotations()`.
>> * Annotation data is returned in a map-like object (of type 
>> `jdk.vm.ci.meta.AnnotationData`) instead of in an `Annotation` object. This 
>> works better for libgraal as it avoids the need for annotation types to be 
>> loaded and included in libgraal.
>>  
>> To demonstrate the new API, here's an example in terms 
>> `java.lang.reflect.AnnotatedElement` (which `ResolvedJavaType` implements):
>> 
>>     ResolvedJavaMethod method = ...;
>>     ExplodeLoop a = method.getAnnotation(ExplodeLoop.class);
>>     return switch (a.kind()) {
>>         case FULL_UNROLL -> LoopExplosionKind.FULL_UNROLL;
>>         case FULL_UNROLL_UNTIL_RETURN -> 
>> LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN;
>>         ...
>>     }
>> 
>> 
>> The same code using the new API:
>> 
>> 
>>     ResolvedJavaMethod method = ...;
>>     ResolvedJavaType explodeLoopType = ...;
>>     AnnotationData a = method.getAnnotationDataFor(explodeLoopType);
>>     return switch (a.getEnum("kind").getName()) {
>>         case "FULL_UNROLL" -> LoopExplosionKind.FULL_UNROLL;
>>         case "FULL_UNROLL_UNTIL_RETURN" -> 
>> LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN;
>>         ...
>>     }
>> 
>> 
>> The implementation relies on new methods in `jdk.internal.vm.VMSupport` for 
>> parsing annotations and serializing/deserializing to/from a byte array. This 
>> allows the annotation data to be passed from the HotSpot heap to the 
>> libgraal heap.
>
> Doug Simon has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   [skip ci] formatting fixes

A few higher-level comments:

>From the long-term perspective, it is likely that the set of kinds of elements 
>that can occur in an annotation will be expanded, for example, method 
>references are a repeated request. Easing future maintenance to gives more 
>inter-source linkage in this situation and error handling for this case in the 
>libgraal code would be prudent IMO.

The java.lang.reflect.AnnotatedElement API 
(https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/reflect/AnnotatedElement.html)
 defines different ways an annotation can be affiliated with an element:

"The terms directly present, indirectly present, present, and associated are 
used throughout this interface to describe precisely which annotations are 
returned by methods: "

tl;dr these terms relate to which of inheriting annotations and looking through 
repeated annotations the methods do on behalf of the caller. I think the 
methods should phrase their operations in terms of these concepts, such as 
"Construct the annotations present..." since inheritance is taken into account.

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

PR Comment: https://git.openjdk.org/jdk/pull/12810#issuecomment-1511735022

Reply via email to