Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code [v4]
On Mon, 31 May 2021 12:02:13 GMT, Peter Levart wrote: > A test could be constructed so that it would mimic the migration of an > annotation from CLASS to RUNTIME retention The test is ready for review in #4280 - I am closing this PR without integration as the change of core-libs proposed here hasn't gained enough support. - PR: https://git.openjdk.java.net/jdk/pull/4245
Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code [v4]
On Mon, 31 May 2021 06:06:37 GMT, Jaroslav Tulach wrote: >> This PR exposes runtime invisible annotations via `Class.getAnnotation` when >> `-XX:+PreserveAllAnnotations` option is passed to the JVM. >> >> Existing `-XX:+PreserveAllAnnotations` option can be very useful for code >> that needs to access annotations with `RetentionPolicy.CLASS` without the >> need to parse the .class files manually. While the >> RuntimeInvisibleAnnotations are kept in the runtime, they are not visible >> via java.lang.reflect API. I assume that's just an omission. >> >> This PR provides a new test and a fix to make `Class.getAnnotation(...)` >> useful when `-XX:+PreserveAllAnnotations` option is on. > > Jaroslav Tulach 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. A suggestion for this RFR. Maybe the title of the issue could be rephrased as: "Add a test for -XX+PreserveAllAnnotations option" A test could be constructed so that it would mimic the migration of an annotation from CLASS to RUNTIME retention by using separate compilation of: 1. an annotation with CLASS retention together with a class that uses it to annotate itself 2. the same type of annotation but with RUNTIME retention The test would use a class from compilation (1) combined with annotation from compilation (2) together with `-XX+PreserveAllAnnotations` JVM option to verify that the annotation is visible via reflection. No ClassLoader magic is necessary. WDYT? - PR: https://git.openjdk.java.net/jdk/pull/4245
Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code [v4]
On Mon, 31 May 2021 06:06:37 GMT, Jaroslav Tulach wrote: >> This PR exposes runtime invisible annotations via `Class.getAnnotation` when >> `-XX:+PreserveAllAnnotations` option is passed to the JVM. >> >> Existing `-XX:+PreserveAllAnnotations` option can be very useful for code >> that needs to access annotations with `RetentionPolicy.CLASS` without the >> need to parse the .class files manually. While the >> RuntimeInvisibleAnnotations are kept in the runtime, they are not visible >> via java.lang.reflect API. I assume that's just an omission. >> >> This PR provides a new test and a fix to make `Class.getAnnotation(...)` >> useful when `-XX:+PreserveAllAnnotations` option is on. > > Jaroslav Tulach 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. I think it would be wrong to extend the meaning of `-XX:+PreserveAllAnnotations` to the AnnotationParser so it would return CLASS retention-marked annotations. `-XX:+PreserveAllAnnotations` is just disabling the JVM optimization and it is not meant to be "visible" in the Java code. - PR: https://git.openjdk.java.net/jdk/pull/4245
Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code [v4]
On Mon, 31 May 2021 06:06:37 GMT, Jaroslav Tulach wrote: >> This PR exposes runtime invisible annotations via `Class.getAnnotation` when >> `-XX:+PreserveAllAnnotations` option is passed to the JVM. >> >> Existing `-XX:+PreserveAllAnnotations` option can be very useful for code >> that needs to access annotations with `RetentionPolicy.CLASS` without the >> need to parse the .class files manually. While the >> RuntimeInvisibleAnnotations are kept in the runtime, they are not visible >> via java.lang.reflect API. I assume that's just an omission. >> >> This PR provides a new test and a fix to make `Class.getAnnotation(...)` >> useful when `-XX:+PreserveAllAnnotations` option is on. > > Jaroslav Tulach 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. There is [support for parsing class files in Graal](https://github.com/oracle/graal/blob/89e4cfc7ae/compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ClassfileBytecodeProvider.java#L45-L47) which exists to avoids all bytecode preprocessing and instrumentation that may be performed on the VM internal bytecode representation. This is only done for trusted bytecode that comes from Graal classes (e.g. [Java source code snippets](https://github.com/oracle/graal/blob/89e4cfc7ae/compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/AllocationSnippets.java#L50-L72) used to express compiler (sub)graphs as Java source code). - PR: https://git.openjdk.java.net/jdk/pull/4245
Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code [v4]
On Mon, 31 May 2021 06:06:37 GMT, Jaroslav Tulach
wrote:
>> This PR exposes runtime invisible annotations via `Class.getAnnotation` when
>> `-XX:+PreserveAllAnnotations` option is passed to the JVM.
>>
>> Existing `-XX:+PreserveAllAnnotations` option can be very useful for code
>> that needs to access annotations with `RetentionPolicy.CLASS` without the
>> need to parse the .class files manually. While the
>> RuntimeInvisibleAnnotations are kept in the runtime, they are not visible
>> via java.lang.reflect API. I assume that's just an omission.
>>
>> This PR provides a new test and a fix to make `Class.getAnnotation(...)`
>> useful when `-XX:+PreserveAllAnnotations` option is on.
>
> Jaroslav Tulach 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/AnnotationType/AnnotationTypeChangedToRuntimeTest.java
line 56:
> 54: @Retention(RUNTIME)
> 55: @AnnB
> 56: public @interface AnnA_v2 {
The `AltClassLoader` uses `AnnA_v2` instead of `AnnA_v1` when loading the
classes and that makes (in cooperation with `-XX:+PreserveAllAnnotations` flag)
the `RuntimeInvisibleAnnotation` visible.
-
PR: https://git.openjdk.java.net/jdk/pull/4245
Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code [v4]
> This PR exposes runtime invisible annotations via `Class.getAnnotation` when > `-XX:+PreserveAllAnnotations` option is passed to the JVM. > > Existing `-XX:+PreserveAllAnnotations` option can be very useful for code > that needs to access annotations with `RetentionPolicy.CLASS` without the > need to parse the .class files manually. While the > RuntimeInvisibleAnnotations are kept in the runtime, they are not visible via > java.lang.reflect API. I assume that's just an omission. > > This PR provides a new test and a fix to make `Class.getAnnotation(...)` > useful when `-XX:+PreserveAllAnnotations` option is on. Jaroslav Tulach 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 two new commits since the last revision: - Merging with test for long time existing behavior of -XX:+PreserveAllAnnotations - Test long time existing behavior of -XX:+PreserveAllAnnotations - Changes: - all: https://git.openjdk.java.net/jdk/pull/4245/files - new: https://git.openjdk.java.net/jdk/pull/4245/files/9548a565..0fd2fced Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4245&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4245&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4245.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4245/head:pull/4245 PR: https://git.openjdk.java.net/jdk/pull/4245
