[GR-70912] Replace AnnotatedElement usages with AnnotatedValueSupport in the compiler.

PullRequest: graal/22421

Author: dougxc

compiler/mx.compiler/suite.py

@@ -140,7 +140,6 @@
       "requiresConcealed" : {
         "java.base" : [
           "jdk.internal.misc",
-          "sun.reflect.generics.parser",
         ],
         "jdk.internal.vm.ci" : [
           "jdk.vm.ci.meta",

compiler/src/jdk.graal.compiler.libgraal/src/jdk/graal/compiler/libgraal/LibGraalFeature.java

@@ -44,7 +44,6 @@
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
-import jdk.graal.compiler.serviceprovider.GraalServices;
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.jniutils.NativeBridgeSupport;
 import org.graalvm.nativeimage.ImageInfo;
@@ -72,6 +71,7 @@
 import jdk.graal.compiler.options.OptionDescriptor;
 import jdk.graal.compiler.options.OptionKey;
 import jdk.graal.compiler.options.OptionsParser;
+import jdk.graal.compiler.serviceprovider.GraalServices;
 import jdk.graal.compiler.truffle.host.TruffleHostEnvironment;
 import jdk.graal.compiler.util.CollectionsUtil;
 import jdk.graal.compiler.util.EconomicHashMap;

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/annotation/test/TestAnnotationsBase.java

@@ -29,6 +29,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
@@ -209,6 +210,13 @@ private static Annotated toAnnotated(AnnotatedElement element) {
         }
     }
 
+    /**
+     * Used to test error handling in {@link AnnotationValue#getEnum(Class, String)}.
+     */
+    enum MyEnum {
+    }
+
+    @SuppressWarnings({"unchecked", "rawtypes"})
     public static void assertAnnotationsEquals(Annotation a, AnnotationValue av) {
         Map<String, Object> values = AnnotationSupport.memberValues(a);
         for (Map.Entry<String, Object> e : values.entrySet()) {
@@ -224,6 +232,46 @@ public static void assertAnnotationsEquals(Annotation a, AnnotationValue av) {
             if (!(aElement instanceof ExceptionProxy)) {
                 Class<?> elementType = toAnnotationValueElementType(aElement.getClass());
                 av.get(name, elementType);
+
+                Object actual;
+                if (elementType == Byte.class) {
+                    actual = av.getByte(name);
+                } else if (elementType == Boolean.class) {
+                    actual = av.getBoolean(name);
+                } else if (elementType == Short.class) {
+                    actual = av.getShort(name);
+                } else if (elementType == Character.class) {
+                    actual = av.getChar(name);
+                } else if (elementType == Integer.class) {
+                    actual = av.getInt(name);
+                } else if (elementType == Float.class) {
+                    actual = av.getFloat(name);
+                } else if (elementType == Long.class) {
+                    actual = av.getLong(name);
+                } else if (elementType == Double.class) {
+                    actual = av.getDouble(name);
+                } else if (elementType == String.class) {
+                    actual = av.getString(name);
+                } else if (elementType == ResolvedJavaType.class) {
+                    actual = av.getType(name);
+                } else if (elementType == EnumElement.class) {
+                    actual = av.getEnum(name);
+                    Class<? extends Enum> enumClass = (Class<? extends Enum>) aElement.getClass();
+                    var avEnumConstant = av.getEnum(enumClass, name);
+                    assertEquals(aElement, avEnumConstant);
+                    try {
+                        av.getEnum(MyEnum.class, name);
+                        fail("expected " + IllegalArgumentException.class.getName());
+                    } catch (IllegalArgumentException iae) {
+                        // expected
+                    }
+                } else if (elementType == AnnotationValue.class) {
+                    actual = av.getAnnotation(name);
+                } else {
+                    assert elementType == List.class : aElement + " // " + elementType;
+                    actual = avElement;
+                }
+                assertAnnotationElementsEqual(aElement, actual);
             }
         }
     }

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/annotation/test/TestAnnotationsOnMethods.java

@@ -198,7 +198,7 @@ public void getAnnotationValuesTest() throws Exception {
         checkAnnotationValues(AnnotationTestInput.class.getDeclaredMethod("missingMember"));
         List<AnnotationValue> avList = checkAnnotationValues(AnnotationTestInput.class.getDeclaredMethod("addedMember"));
         try {
-            avList.getFirst().get("addedElement", Integer.class);
+            avList.getFirst().getInt("addedElement");
             throw new AssertionError("expected " + IllegalArgumentException.class.getName());
         } catch (IllegalArgumentException e) {
             Assert.assertEquals(MemberAdded.class.getName() + " missing element addedElement", e.getMessage());

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/annotation/test/TestAnnotationsOnTypes.java

@@ -102,7 +102,7 @@ public void getAnnotationValuesTest() {
         // Test that inherited annotations are handled properly.
         ResolvedJavaType namedType = metaAccess.lookupJavaType(AnnotationTestInput.Named.class);
         AnnotationValue av = AnnotationValueSupport.getDeclaredAnnotationValue(namedType, metaAccess.lookupJavaType(AnnotationTestInput.OwnName.class));
-        Assert.assertEquals("NonInheritedValue", av.get("value", String.class));
+        Assert.assertEquals("NonInheritedValue", av.getString("value"));
         av = getDeclaredAnnotationValue(namedType, metaAccess.lookupJavaType(AnnotationTestInput.InheritedName1.class));
         Assert.assertNull(av);
         av = getDeclaredAnnotationValue(namedType, metaAccess.lookupJavaType(AnnotationTestInput.InheritedName2.class));

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/api/directives/test/BlackholeDirectiveTest.java

@@ -29,13 +29,16 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+import org.junit.Assert;
+import org.junit.Test;
+
+import jdk.graal.compiler.annotation.AnnotationValue;
+import jdk.graal.compiler.annotation.AnnotationValueSupport;
 import jdk.graal.compiler.api.directives.GraalDirectives;
 import jdk.graal.compiler.core.test.GraalCompilerTest;
 import jdk.graal.compiler.nodes.ParameterNode;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.phases.OptimisticOptimizations;
-import org.junit.Assert;
-import org.junit.Test;
 
 /**
  * Tests for {@link GraalDirectives#blackhole}.
@@ -137,9 +140,9 @@ protected OptimisticOptimizations getOptimisticOptimizations() {
 
     @Override
     protected void checkLowTierGraph(StructuredGraph graph) {
-        BlackholeSnippet snippet = graph.method().getAnnotation(BlackholeSnippet.class);
+        AnnotationValue snippet = AnnotationValueSupport.getAnnotationValue(graph.method(), BlackholeSnippet.class);
         ParameterNode arg = graph.getParameter(0);
-        if (snippet.expectParameterUsage()) {
+        if (snippet != null && snippet.getBoolean("expectParameterUsage")) {
             Assert.assertNotNull("couldn't find ParameterNode(0)", arg);
             Assert.assertFalse("expected usages of " + arg, arg.hasNoUsages());
         } else {

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/CheckGraalInvariants.java

@@ -215,6 +215,13 @@ public boolean shouldVerifyFoldableMethods() {
             return true;
         }
 
+        /**
+         * Determines if {@link VerifyAnnotatedElementUsage} is to be checked.
+         */
+        public boolean shouldVerifyAnnotatedElementUsages() {
+            return true;
+        }
+
         public void verifyCurrentTimeMillis(MetaAccessProvider meta, MethodCallTargetNode t, ResolvedJavaType declaringClass) {
             final ResolvedJavaType services = meta.lookupJavaType(GraalServices.class);
             if (!declaringClass.equals(services)) {
@@ -393,6 +400,9 @@ public static void runTest(InvariantsTool tool) {
         if (tool.shouldVerifyFoldableMethods()) {
             verifiers.add(foldableMethodsVerifier);
         }
+        if (tool.shouldVerifyAnnotatedElementUsages()) {
+            verifiers.add(new VerifyAnnotatedElementUsage());
+        }
 
         verifiers.add(new VerifyCurrentTimeMillisUsage(tool));
 

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/VerifyAnnotatedElementUsage.java

@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jdk.graal.compiler.core.test;
+
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Method;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import jdk.graal.compiler.annotation.AnnotationValueSupport;
+import jdk.graal.compiler.core.common.type.Stamp;
+import jdk.graal.compiler.nodes.NodeView;
+import jdk.graal.compiler.nodes.StructuredGraph;
+import jdk.graal.compiler.nodes.ValueNode;
+import jdk.graal.compiler.nodes.java.MethodCallTargetNode;
+import jdk.graal.compiler.nodes.spi.CoreProviders;
+import jdk.graal.compiler.nodes.spi.UncheckedInterfaceProvider;
+import jdk.graal.compiler.util.EconomicHashMap;
+import jdk.vm.ci.meta.MetaAccessProvider;
+import jdk.vm.ci.meta.ResolvedJavaField;
+import jdk.vm.ci.meta.ResolvedJavaMethod;
+import jdk.vm.ci.meta.ResolvedJavaType;
+
+/**
+ * Verifies that calls to methods declared by {@link AnnotatedElement} never have a receiver of type
+ * {@link ResolvedJavaType}, {@link ResolvedJavaMethod} or {@link ResolvedJavaField}. Once GR-69713,
+ * is resolved ("Remove AnnotatedElement from JVMCI types"), this verification can be deleted.
+ */
+public class VerifyAnnotatedElementUsage extends VerifyStringFormatterUsage {
+
+    private volatile Map<String, String> annotatedElementMethods;
+
+    private static final String JVMCI_META_PACKAGE_PREFIX = "L" + ResolvedJavaType.class.getPackage().getName().replace('.', '/');
+    private static final Set<String> ANNOTATED_ELEMENT_METHOD_NAMES = Stream.of(AnnotatedElement.class.getDeclaredMethods()).map(Method::getName).collect(Collectors.toSet());
+
+    @Override
+    protected void verify(StructuredGraph graph, CoreProviders context) {
+        MetaAccessProvider metaAccess = context.getMetaAccess();
+        ResolvedJavaType annotatedElementType = metaAccess.lookupJavaType(AnnotatedElement.class);
+        ResolvedJavaType resolvedJavaTypeType = metaAccess.lookupJavaType(ResolvedJavaType.class);
+        ResolvedJavaType resolvedJavaMethodType = metaAccess.lookupJavaType(ResolvedJavaMethod.class);
+        ResolvedJavaType resolvedJavaFieldType = metaAccess.lookupJavaType(ResolvedJavaField.class);
+
+        if (annotatedElementMethods == null) {
+            Map<String, String> map = new EconomicHashMap<>();
+            for (Method m : AnnotatedElement.class.getDeclaredMethods()) {
+                map.put(m.getName(), metaAccess.lookupJavaMethod(m).getSignature().toMethodDescriptor());
+            }
+            annotatedElementMethods = map;
+        }
+
+        for (MethodCallTargetNode t : graph.getNodes(MethodCallTargetNode.TYPE)) {
+            ResolvedJavaMethod callee = t.targetMethod();
+            String descriptor = annotatedElementMethods.get(callee.getName());
+            if (descriptor != null && descriptor.equals(callee.getSignature().toMethodDescriptor())) {
+                if (callee.hasReceiver()) {
+                    ValueNode receiver = t.arguments().getFirst();
+                    Stamp receiverStamp = receiver.stamp(NodeView.DEFAULT);
+                    if (receiver instanceof UncheckedInterfaceProvider unchecked) {
+                        Stamp uncheckedStamp = unchecked.uncheckedStamp();
+                        if (uncheckedStamp != null) {
+                            receiverStamp = uncheckedStamp;
+                        }
+                    }
+                    ResolvedJavaType receiverType = receiverStamp.javaType(metaAccess);
+                    if (resolvedJavaTypeType.isAssignableFrom(receiverType) ||
+                                    resolvedJavaMethodType.isAssignableFrom(receiverType) ||
+                                    resolvedJavaFieldType.isAssignableFrom(receiverType)) {
+
+                        throw new VerificationError(
+                                        t, "call to %s with receiver type %s should be replaced by use of %s.%n",
+                                        callee.format("%H.%n(%p)"),
+                                        receiverType.toClassName(),
+                                        AnnotationValueSupport.class.getName());
+                    }
+                }
+            }
+        }
+    }
+}