Remove Painless Type From Locals, Variables, Params, and ScriptInfo (#28471)

Author: jdconrad

modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java

@@ -93,7 +93,7 @@
  *         private $$Lambda0(List arg$0) {
  *             this.arg$0 = arg$0;
  *         }
- *         
+ *
  *         public static Consumer create$lambda(List arg$0) {
  *             return new $$Lambda0(arg$0);
  *         }
@@ -212,7 +212,7 @@ public static CallSite lambdaBootstrap(
         ClassWriter cw = beginLambdaClass(lambdaClassName, factoryMethodType.returnType());
         Capture[] captures = generateCaptureFields(cw, factoryMethodType);
         generateLambdaConstructor(cw, lambdaClassType, factoryMethodType, captures);
-        
+
         // Handles the special case where a method reference refers to a ctor (we need a static wrapper method):
         if (delegateInvokeType == H_NEWINVOKESPECIAL) {
             assert CTOR_METHOD_NAME.equals(delegateMethodName);
@@ -226,7 +226,7 @@ public static CallSite lambdaBootstrap(
         generateInterfaceMethod(cw, factoryMethodType, lambdaClassType, interfaceMethodName,
             interfaceMethodType, delegateClassType, delegateInvokeType,
             delegateMethodName, delegateMethodType, captures);
-        
+
         endLambdaClass(cw);
 
         Class<?> lambdaClass = createLambdaClass(loader, cw, lambdaClassType);
@@ -321,7 +321,7 @@ private static void generateLambdaConstructor(
 
         constructor.returnValue();
         constructor.endMethod();
-        
+
         // Add a factory method, if lambda takes captures.
         // @uschindler says: I talked with Rémi Forax about this. Technically, a plain ctor
         // and a MethodHandle to the ctor would be enough - BUT: Hotspot is unable to
@@ -337,10 +337,10 @@ private static void generateLambdaConstructor(
     /**
      * Generates a factory method to delegate to constructors.
      */
-    private static void generateStaticCtorDelegator(ClassWriter cw, int access, String delegatorMethodName, 
+    private static void generateStaticCtorDelegator(ClassWriter cw, int access, String delegatorMethodName,
             Type delegateClassType, MethodType delegateMethodType) {
         Method wrapperMethod = new Method(delegatorMethodName, delegateMethodType.toMethodDescriptorString());
-        Method constructorMethod = 
+        Method constructorMethod =
             new Method(CTOR_METHOD_NAME, delegateMethodType.changeReturnType(void.class).toMethodDescriptorString());
         int modifiers = access | ACC_STATIC;
 
@@ -379,7 +379,7 @@ private static void generateInterfaceMethod(
         GeneratorAdapter iface = new GeneratorAdapter(modifiers, lamMeth,
             cw.visitMethod(modifiers, interfaceMethodName, lamDesc, null, null));
         iface.visitCode();
-        
+
         // Loads any captured variables onto the stack.
         for (int captureCount = 0; captureCount < captures.length; ++captureCount) {
             iface.loadThis();
@@ -473,7 +473,7 @@ private static Class<?> createLambdaClass(
     private static CallSite createNoCaptureCallSite(
             MethodType factoryMethodType,
             Class<?> lambdaClass) {
-        
+
         try {
             return new ConstantCallSite(MethodHandles.constant(
                 factoryMethodType.returnType(), lambdaClass.getConstructor().newInstance()));
@@ -503,7 +503,7 @@ private static CallSite createCaptureCallSite(
      * delegate method will use converted types from the interface method.  Using
      * invokedynamic to make the delegate method call allows
      * {@link MethodHandle#asType} to be used to do the type conversion instead
-     * of either a lot more code or requiring many {@link Definition.Type}s to be looked
+     * of either a lot more code or requiring many {@link Class}es to be looked
      * up at link-time.
      */
     public static CallSite delegateBootstrap(Lookup lookup,

modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java

@@ -21,7 +21,6 @@
 
 import org.elasticsearch.painless.Definition.Method;
 import org.elasticsearch.painless.Definition.MethodKey;
-import org.elasticsearch.painless.Definition.Type;
 import org.elasticsearch.painless.ScriptClassInfo.MethodArgument;
 
 import java.util.Arrays;
@@ -58,7 +57,7 @@ public static Locals newLocalScope(Locals currentScope) {
      * <p>
      * This is just like {@link #newFunctionScope}, except the captured parameters are made read-only.
      */
-    public static Locals newLambdaScope(Locals programScope, Type returnType, List<Parameter> parameters,
+    public static Locals newLambdaScope(Locals programScope, Class<?> returnType, List<Parameter> parameters,
                                         int captureCount, int maxLoopCounter) {
         Locals locals = new Locals(programScope, programScope.definition, returnType, KEYWORDS);
         for (int i = 0; i < parameters.size(); i++) {
@@ -68,43 +67,43 @@ public static Locals newLambdaScope(Locals programScope, Type returnType, List<P
             // currently, this cannot be allowed, as we swap in real types,
             // but that can prevent a store of a different type...
             boolean isCapture = true;
-            locals.addVariable(parameter.location, parameter.type, parameter.name, isCapture);
+            locals.addVariable(parameter.location, parameter.clazz, parameter.name, isCapture);
         }
         // Loop counter to catch infinite loops.  Internal use only.
         if (maxLoopCounter > 0) {
-            locals.defineVariable(null, locals.getDefinition().intType, LOOP, true);
+            locals.defineVariable(null, int.class, LOOP, true);
         }
         return locals;
     }
 
     /** Creates a new function scope inside the current scope */
-    public static Locals newFunctionScope(Locals programScope, Type returnType, List<Parameter> parameters, int maxLoopCounter) {
+    public static Locals newFunctionScope(Locals programScope, Class<?> returnType, List<Parameter> parameters, int maxLoopCounter) {
         Locals locals = new Locals(programScope, programScope.definition, returnType, KEYWORDS);
         for (Parameter parameter : parameters) {
-            locals.addVariable(parameter.location, parameter.type, parameter.name, false);
+            locals.addVariable(parameter.location, parameter.clazz, parameter.name, false);
         }
         // Loop counter to catch infinite loops.  Internal use only.
         if (maxLoopCounter > 0) {
-            locals.defineVariable(null, locals.getDefinition().intType, LOOP, true);
+            locals.defineVariable(null, int.class, LOOP, true);
         }
         return locals;
     }
 
     /** Creates a new main method scope */
     public static Locals newMainMethodScope(ScriptClassInfo scriptClassInfo, Locals programScope, int maxLoopCounter) {
-        Locals locals = new Locals(programScope, programScope.definition,
-                scriptClassInfo.getExecuteMethodReturnType(), KEYWORDS);
+        Locals locals = new Locals(
+            programScope, programScope.definition, scriptClassInfo.getExecuteMethodReturnType(), KEYWORDS);
         // This reference. Internal use only.
-        locals.defineVariable(null, programScope.definition.getType("Object"), THIS, true);
+        locals.defineVariable(null, Object.class, THIS, true);
 
         // Method arguments
         for (MethodArgument arg : scriptClassInfo.getExecuteArguments()) {
-            locals.defineVariable(null, arg.getType(), arg.getName(), true);
+            locals.defineVariable(null, arg.getClazz(), arg.getName(), true);
         }
 
         // Loop counter to catch infinite loops.  Internal use only.
         if (maxLoopCounter > 0) {
-            locals.defineVariable(null, locals.getDefinition().intType, LOOP, true);
+            locals.defineVariable(null, int.class, LOOP, true);
         }
         return locals;
     }
@@ -155,18 +154,18 @@ public Method getMethod(MethodKey key) {
     }
 
     /** Creates a new variable. Throws IAE if the variable has already been defined (even in a parent) or reserved. */
-    public Variable addVariable(Location location, Type type, String name, boolean readonly) {
+    public Variable addVariable(Location location, Class<?> clazz, String name, boolean readonly) {
         if (hasVariable(name)) {
             throw location.createError(new IllegalArgumentException("Variable [" + name + "] is already defined."));
         }
         if (keywords.contains(name)) {
             throw location.createError(new IllegalArgumentException("Variable [" + name + "] is reserved."));
         }
-        return defineVariable(location, type, name, readonly);
+        return defineVariable(location, clazz, name, readonly);
     }
 
     /** Return type of this scope (e.g. int, if inside a function that returns int) */
-    public Type getReturnType() {
+    public Class<?> getReturnType() {
         return returnType;
     }
 
@@ -191,7 +190,7 @@ public Definition getDefinition() {
     // parent scope
     private final Locals parent;
     // return type of this scope
-    private final Type returnType;
+    private final Class<?> returnType;
     // keywords for this scope
     private final Set<String> keywords;
     // next slot number to assign
@@ -211,7 +210,7 @@ private Locals(Locals parent) {
     /**
      * Create a new Locals with specified return type
      */
-    private Locals(Locals parent, Definition definition, Type returnType, Set<String> keywords) {
+    private Locals(Locals parent, Definition definition, Class<?> returnType, Set<String> keywords) {
         this.parent = parent;
         this.definition = definition;
         this.returnType = returnType;
@@ -246,13 +245,13 @@ private Method lookupMethod(MethodKey key) {
 
 
     /** Defines a variable at this scope internally. */
-    private Variable defineVariable(Location location, Type type, String name, boolean readonly) {
+    private Variable defineVariable(Location location, Class<?> type, String name, boolean readonly) {
         if (variables == null) {
             variables = new HashMap<>();
         }
         Variable variable = new Variable(location, name, type, getNextSlot(), readonly);
         variables.put(name, variable); // TODO: check result
-        nextSlotNumber += type.type.getSize();
+        nextSlotNumber += MethodWriter.getType(type).getSize();
         return variable;
     }
 
@@ -272,15 +271,15 @@ private int getNextSlot() {
     public static final class Variable {
         public final Location location;
         public final String name;
-        public final Type type;
+        public final Class<?> clazz;
         public final boolean readonly;
         private final int slot;
         private boolean used;
 
-        public Variable(Location location, String name, Type type, int slot, boolean readonly) {
+        public Variable(Location location, String name, Class<?> clazz, int slot, boolean readonly) {
             this.location = location;
             this.name = name;
-            this.type = type;
+            this.clazz = clazz;
             this.slot = slot;
             this.readonly = readonly;
         }
@@ -292,7 +291,7 @@ public int getSlot() {
         @Override
         public String toString() {
             StringBuilder b = new StringBuilder();
-            b.append("Variable[type=").append(type);
+            b.append("Variable[type=").append(Definition.ClassToName(clazz));
             b.append(",name=").append(name);
             b.append(",slot=").append(slot);
             if (readonly) {
@@ -306,12 +305,12 @@ public String toString() {
     public static final class Parameter {
         public final Location location;
         public final String name;
-        public final Type type;
+        public final Class<?> clazz;
 
-        public Parameter(Location location, String name, Type type) {
+        public Parameter(Location location, String name, Class<?> clazz) {
             this.location = location;
             this.name = name;
-            this.type = type;
+            this.clazz = clazz;
         }
     }
 }

modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptClassInfo.java

@@ -38,11 +38,11 @@ public class ScriptClassInfo {
 
     private final Class<?> baseClass;
     private final org.objectweb.asm.commons.Method executeMethod;
-    private final Definition.Type executeMethodReturnType;
+    private final Class<?> executeMethodReturnType;
     private final List<MethodArgument> executeArguments;
     private final List<org.objectweb.asm.commons.Method> needsMethods;
     private final List<org.objectweb.asm.commons.Method> getMethods;
-    private final List<Definition.Type> getReturns;
+    private final List<Class<?>> getReturns;
 
     public ScriptClassInfo(Definition definition, Class<?> baseClass) {
         this.baseClass = baseClass;
@@ -51,7 +51,7 @@ public ScriptClassInfo(Definition definition, Class<?> baseClass) {
         java.lang.reflect.Method executeMethod = null;
         List<org.objectweb.asm.commons.Method> needsMethods = new ArrayList<>();
         List<org.objectweb.asm.commons.Method> getMethods = new ArrayList<>();
-        List<Definition.Type> getReturns = new ArrayList<>();
+        List<Class<?>> getReturns = new ArrayList<>();
         for (java.lang.reflect.Method m : baseClass.getMethods()) {
             if (m.isDefault()) {
                 continue;
@@ -65,7 +65,7 @@ public ScriptClassInfo(Definition definition, Class<?> baseClass) {
                                     + "] has more than one.");
                 }
             }
-            if (m.getName().startsWith("needs") && m.getReturnType().equals(boolean.class) && m.getParameterTypes().length == 0) {
+            if (m.getName().startsWith("needs") && m.getReturnType() == boolean.class && m.getParameterTypes().length == 0) {
                 needsMethods.add(new org.objectweb.asm.commons.Method(m.getName(), NEEDS_PARAMETER_METHOD_TYPE.toMethodDescriptorString()));
             }
             if (m.getName().startsWith("get") && m.getName().equals("getClass") == false && Modifier.isStatic(m.getModifiers()) == false) {
@@ -118,15 +118,15 @@ public org.objectweb.asm.commons.Method getExecuteMethod() {
     }
 
     /**
-     * The Painless {@link Definition.Type} or the return type of the {@code execute} method. This is used to generate the appropriate
+     * The Painless {@link Class} or the return type of the {@code execute} method. This is used to generate the appropriate
      * return bytecode.
      */
-    public Definition.Type getExecuteMethodReturnType() {
+    public Class<?> getExecuteMethodReturnType() {
         return executeMethodReturnType;
     }
 
     /**
-     * Painless {@link Definition.Type}s and names of the arguments to the {@code execute} method. The names are exposed to the Painless
+     * Painless {@link Class}s and names of the arguments to the {@code execute} method. The names are exposed to the Painless
      * script.
      */
     public List<MethodArgument> getExecuteArguments() {
@@ -150,38 +150,38 @@ public List<org.objectweb.asm.commons.Method> getGetMethods() {
     /**
      * The {@code getVarName} methods return types.
      */
-    public List<Definition.Type> getGetReturns() {
+    public List<Class<?>> getGetReturns() {
         return getReturns;
     }
 
     /**
-     * Painless {@link Definition.Type}s and name of the argument to the {@code execute} method.
+     * Painless {@link Class}es and name of the argument to the {@code execute} method.
      */
     public static class MethodArgument {
-        private final Definition.Type type;
+        private final Class<?> clazz;
         private final String name;
 
-        public MethodArgument(Definition.Type type, String name) {
-            this.type = type;
+        public MethodArgument(Class<?> clazz, String name) {
+            this.clazz = clazz;
             this.name = name;
         }
 
-        public Definition.Type getType() {
-            return type;
+        public Class<?> getClazz() {
+            return clazz;
         }
 
         public String getName() {
             return name;
         }
     }
 
-    private MethodArgument methodArgument(Definition definition, Class<?> type, String argName) {
-        Definition.Type defType = definitionTypeForClass(definition, type, componentType -> "[" + argName + "] is of unknown type ["
+    private MethodArgument methodArgument(Definition definition, Class<?> clazz, String argName) {
+        Class<?> defClass = definitionTypeForClass(definition, clazz, componentType -> "[" + argName + "] is of unknown type ["
                 + componentType.getName() + ". Painless interfaces can only accept arguments that are of whitelisted types.");
-        return new MethodArgument(defType, argName);
+        return new MethodArgument(defClass, argName);
     }
 
-    private static Definition.Type definitionTypeForClass(Definition definition, Class<?> type,
+    private static Class<?> definitionTypeForClass(Definition definition, Class<?> type,
             Function<Class<?>, String> unknownErrorMessageSource) {
         int dimensions = 0;
         Class<?> componentType = type;
@@ -190,16 +190,16 @@ private static Definition.Type definitionTypeForClass(Definition definition, Cla
             componentType = componentType.getComponentType();
         }
         Definition.Struct struct;
-        if (componentType.equals(Object.class)) {
-            struct = definition.DefType.struct;
+        if (componentType == Object.class) {
+            struct = definition.getType("def").struct;
         } else {
             Definition.RuntimeClass runtimeClass = definition.getRuntimeClass(componentType);
             if (runtimeClass == null) {
                 throw new IllegalArgumentException(unknownErrorMessageSource.apply(componentType));
             }
             struct = runtimeClass.getStruct();
         }
-        return definition.getType(struct, dimensions);
+        return Definition.TypeToClass(definition.getType(struct, dimensions));
     }
 
     private static String[] readArgumentNamesConstant(Class<?> iface) {