[
https://issues.apache.org/jira/browse/GROOVY-12389?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113331#comment-18113331
]
ASF GitHub Bot commented on GROOVY-12389:
-----------------------------------------
Copilot commented on code in PR #2911:
URL: https://github.com/apache/groovy/pull/2911#discussion_r3968795274
##########
src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java:
##########
@@ -257,7 +276,7 @@ private GenericsType[] configureTypeParameters(final
TypeVariable<?>[] tp, final
ClassNode[] bounds = configureTypes(tp[i].getBounds());
gt[i] = configureTypeVariableDefinition(t, bounds);
gt[i].setGenericDeclaration(declaration);
- for (Annotation annotation : tp[i].getAnnotations()) {
+ for (Annotation annotation : TYPE_ANNOTATIONS ?
tp[i].getAnnotations() : NO_ANNOTATIONS) {
Review Comment:
This introduces a behavioral change: when `TYPE_ANNOTATIONS` is false, it
will skip processing `TypeVariable#getAnnotations()`. Those annotations are not
obtained via `AnnotatedType` / `getAnnotated*` accessors, so they may still be
available even on runtimes where you need to skip `AnnotatedType` usage. If the
intent is only to avoid missing `getAnnotated*` APIs, consider always iterating
`tp[i].getAnnotations()` here (or add a targeted guard only if there is a known
runtime where `TypeVariable#getAnnotations()` itself is broken).
##########
src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java:
##########
@@ -248,6 +248,25 @@ private GenericsType[] configureTypeArguments(final Type[]
ta) {
return gts;
}
+ /**
+ * Whether the reflection API exposes type-use annotations ({@code
AnnotatedType},
+ * Java 8). Runtimes built on the JDK class library without it (Android's
ART)
+ * lack the {@code getAnnotated*} accessors; type annotations on
precompiled
+ * classes are then not seen, which is the only possible outcome there
+ * (GROOVY-12389).
+ */
+ private static final boolean TYPE_ANNOTATIONS = typeAnnotationsAvailable();
+ private static final Annotation[] NO_ANNOTATIONS = new Annotation[0];
+
+ static boolean typeAnnotationsAvailable() {
+ try {
+ Class.class.getMethod("getAnnotatedSuperclass");
+ return true;
+ } catch (Throwable t) {
Review Comment:
Catching `Throwable` is overly broad and will also swallow serious problems
(e.g., `OutOfMemoryError`, `ThreadDeath`). This check should catch the narrow
set of failures expected from reflective lookup (e.g., `NoSuchMethodException`,
`SecurityException`, and possibly `LinkageError` if you want to cover
incomplete class library implementations) and let other fatal errors propagate.
> Java8 plugin: skip type-use annotations when the reflection API lacks
> AnnotatedType
> -----------------------------------------------------------------------------------
>
> Key: GROOVY-12389
> URL: https://issues.apache.org/jira/browse/GROOVY-12389
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Involves: a TYPE_ANNOTATIONS flag and guards at the seven call sites that
> read annotated types while building ClassNodes from reflection. Same
> deprecation caveat as item 2.
> Impact on normal usage: none on a JVM. Where the API is absent, type-use
> annotations on precompiled classes are simply not seen, which is the only
> possible behaviour there.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
