Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using Javac internals in Enso DSL #5817

Closed
JaroslavTulach opened this issue Mar 6, 2023 · 1 comment · Fixed by #8027
Closed

Avoid using Javac internals in Enso DSL #5817

JaroslavTulach opened this issue Mar 6, 2023 · 1 comment · Fixed by #8027
Assignees
Labels
--bug Type: bug -build-script Category: build script -compiler

Comments

@JaroslavTulach
Copy link
Member

When trying to upgrade to new version of Frgaal I noticed compilation problems in lib/scala/interpreter-dsl project. It is using internals of com.sun.tools.javac package which is not official JDK API. It'd be better to avoid accessing internals and use proper API - which probably is AnnotationValueVisitor.

Following diff shows the affected code:

diff --git lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/MethodGenerator.java lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/MethodGenerator.java
index 6c491df75..dc540a81a 100644
--- lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/MethodGenerator.java
+++ lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/MethodGenerator.java
@@ -1,23 +1,22 @@
 package org.enso.interpreter.dsl.builtins;
 
-import com.google.common.base.CaseFormat;
-import com.sun.tools.javac.code.Attribute;
-import com.sun.tools.javac.code.Symbol;
-import com.sun.tools.javac.util.Pair;
-import org.apache.commons.lang3.StringUtils;
-import org.enso.interpreter.dsl.Builtin;
-
-import javax.annotation.processing.ProcessingEnvironment;
-import javax.lang.model.element.*;
-import javax.lang.model.type.TypeMirror;
-import javax.lang.model.util.Types;
 import java.lang.annotation.Annotation;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.VariableElement;
+import javax.lang.model.type.TypeMirror;
+import javax.lang.model.util.Types;
+
+import org.apache.commons.lang3.StringUtils;
+import org.enso.interpreter.dsl.Builtin;
+
+import com.google.common.base.CaseFormat;
+
 public abstract class MethodGenerator {
   protected final boolean isStatic;
   protected final boolean isConstructor;
@@ -180,24 +179,6 @@ public abstract class MethodGenerator {
       TypeMirror builtinType = builtinElement.asType();
 
       List<SafeWrapException> exceptionWrappers = new ArrayList<>();
-      for (AnnotationMirror am : element.getAnnotationMirrors()) {
-        if (am.getAnnotationType().equals(builtinType)) {
-          Attribute.Class valueFrom = null;
-          Attribute.Class valueTo = null;
-          for (Map.Entry<? extends ExecutableElement, ? extends AnnotationValue> entry :
-              am.getElementValues().entrySet()) {
-            Name key = entry.getKey().getSimpleName();
-            if (key.toString().equals(FromElementName)) {
-              valueFrom = (Attribute.Class) (entry.getValue());
-            } else if (key.toString().equals(ToElementName)) {
-              valueTo = (Attribute.Class) (entry.getValue());
-            }
-          }
-          if (valueFrom != null) {
-            exceptionWrappers.add(new SafeWrapException(valueFrom, Optional.ofNullable(valueTo)));
-          }
-        }
-      }
       return exceptionWrappers.toArray(new SafeWrapException[0]);
     }
 
@@ -210,35 +191,6 @@ public abstract class MethodGenerator {
       TypeMirror builtinType = builtinElement.asType();
 
       List<SafeWrapException> wrappedExceptions = new ArrayList<>();
-      for (AnnotationMirror am : element.getAnnotationMirrors()) {
-        if (tpeUtils.isSameType(am.getAnnotationType(), builtinType)) {
-          for (Map.Entry<? extends ExecutableElement, ? extends AnnotationValue> entry :
-              am.getElementValues().entrySet()) {
-            if (ValueElementName.equals(entry.getKey().getSimpleName().toString())) {
-              Attribute.Array wrapExceptions = (Attribute.Array) entry.getValue();
-              for (int i = 0; i < wrapExceptions.values.length; i++) {
-                Attribute.Class valueFrom = null;
-                Attribute.Class valueTo = null;
-                Attribute.Compound attr = (Attribute.Compound) wrapExceptions.values[i];
-                for (Pair<Symbol.MethodSymbol, Attribute> p : attr.values) {
-                  Name key = p.fst.getSimpleName();
-                  if (key.contentEquals(FromElementName)) {
-                    valueFrom = (Attribute.Class) p.snd;
-                  } else if (key.contentEquals(ToElementName)) {
-                    valueTo = (Attribute.Class) p.snd;
-                  }
-                }
-                if (valueFrom != null) {
-                  SafeWrapException converted =
-                      new SafeWrapException(valueFrom, Optional.ofNullable(valueTo));
-                  wrappedExceptions.add(converted);
-                }
-              }
-              break;
-            }
-          }
-        }
-      }
       return wrappedExceptions.toArray(new SafeWrapException[0]);
     }
   }
diff --git lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/SafeWrapException.java lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/SafeWrapException.java
index 9d98a609a..cf521cac5 100644
--- lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/SafeWrapException.java
+++ lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/builtins/SafeWrapException.java
@@ -1,19 +1,15 @@
 package org.enso.interpreter.dsl.builtins;
 
-import com.sun.tools.javac.code.Attribute;
-import org.apache.commons.lang3.StringUtils;
-import org.enso.interpreter.dsl.Builtin;
-
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
-import java.util.stream.Collectors;
+
+import org.enso.interpreter.dsl.Builtin;
 
 /**
  * Wrapper around {@link Builtin.WrapException} annotation with all elements of Class type resolved.
  * extracted
  */
-public record SafeWrapException(Attribute.Class from, Optional<Attribute.Class> to) {
+public record SafeWrapException(Object from, Optional<Object> to) {
 
     private static final String PanicExceptionClassName = "PanicException";
     private static final String UnsupportedMessageExceptionClassName = "UnsupportedMessageException";
@@ -54,8 +50,8 @@ public record SafeWrapException(Attribute.Class from, Optional<Attribute.Class>
         return clazz.map(c -> c.equals(PanicExceptionClassName)).orElse(true);
     }
 
-    private String fromAttributeToClassName(Attribute.Class clazz, Boolean fullName) {
-        String baseType = clazz.classType.baseType().toString();
+    private String fromAttributeToClassName(Object clazz, Boolean fullName) {
+        String baseType = clazz.toString();
         if (fullName) return baseType;
         else {
             String[] clazzElements = baseType.split("\\.");
  • [] (Optionally) Make the build fail when using javac internals
  • [] Rewrite the "safe exception" extraction to use official JDK API
@JaroslavTulach
Copy link
Member Author

This issue seems to be blocking

time to fix it, finally?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -build-script Category: build script -compiler
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants