[GR-26607] Simplify custom conversions in argument clinic

PullRequest: graalpython/1320

Author: otethal

graalpython/com.oracle.graal.python.annotations/src/com/oracle/graal/python/annotations/ArgumentClinic.java

@@ -58,30 +58,12 @@
      * configuration for the conversion routine. Note that not all routines support all the
      * configuration options.
      * 
-     * Conversion routines are implemented in {@code ArgumentClinicModel#BuiltinConvertor}. It
-     * creates Java code snippets that instantiate the actual cast nodes, which should implement
+     * Conversion routines are implemented in {@code ConverterFactory}. It creates Java code
+     * snippets that instantiate the actual cast nodes, which should implement
      * {@code ArgumentCastNode}.
      */
     ClinicConversion conversion() default ClinicConversion.None;
 
-    /**
-     * Overrides the {@link #conversion()} value. Name of a method of the builtin class that should
-     * be used as a factory to create the conversion node. When this value is set, then other
-     * conversion options are ignored.
-     */
-    String customConversion() default "";
-
-    /**
-     * The boxing optimized execute method variants will not attempt to cast the listed primitive
-     * types and will just pass them directly to the specializations. This does not apply to
-     * primitive values that are already boxed: those are always passed to the convertor.
-     * 
-     * It is not necessary to set this when using a builtin conversion. Built-in convertors provide
-     * their own list of short circuit types, which is applied if this field is set to its default
-     * value.
-     */
-    PrimitiveType[] shortCircuitPrimitive() default {};
-
     /**
      * The string should contain valid Java constant value expression, for example, {@code true}, or
      * {@code \"some string\"}. You may have to update the annotation processor to include import of
@@ -95,6 +77,18 @@
      */
     boolean useDefaultForNone() default false;
 
+    /**
+     * Specifies the name of the conversion node class, which must include a static factory method
+     * annotated with {@link ClinicConverterFactory}.
+     */
+    Class<?> conversionClass() default void.class;
+
+    /**
+     * Specifies arguments to the factory method. String literals must be explicitly quoted:
+     * {@code args = "\"abc\""}
+     */
+    String[] args() default {};
+
     enum PrimitiveType {
         Boolean,
         Int,
@@ -104,36 +98,41 @@ enum PrimitiveType {
 
     enum ClinicConversion {
         /**
-         * No builtin convertor will be used.
+         * No builtin converter will be used.
          */
         None,
         /**
-         * Corresponds to CPython's {@code bool} convertor. Supports {@link #defaultValue()}.
+         * Corresponds to CPython's {@code bool} converter. Supports {@link #defaultValue()}.
          * {@code PNone.NONE} is, for now, always converted to {@code false}.
          */
         Boolean,
         /**
-         * GraalPython specific convertor that narrows any String representation to Java String.
+         * GraalPython specific converter that narrows any String representation to Java String.
          * Supports {@link #defaultValue()}, and {@link #useDefaultForNone()}.
          */
         String,
         /**
-         * Corresponds to CPython's {@code int} convertor. Supports {@link #defaultValue()}, and
+         * Corresponds to CPython's {@code int} converter. Supports {@link #defaultValue()}, and
          * {@link #useDefaultForNone()}.
          */
         Int,
         /**
-         * Corresponds to CPython's {@code Py_ssize_t} convertor. Supports {@link #defaultValue()},
+         * Corresponds to CPython's {@code Py_ssize_t} converter. Supports {@link #defaultValue()},
          * and {@link #useDefaultForNone()}.
          */
         Index,
         /**
-         * Corresponds to CPython's {@code int(accept={str})} convertor. Supports
+         * Corresponds to CPython's {@code slice_index} converter. Supports {@link #defaultValue()},
+         * and {@link #useDefaultForNone()}.
+         */
+        SliceIndex,
+        /**
+         * Corresponds to CPython's {@code int(accept={str})} converter. Supports
          * {@link #defaultValue()}, and {@link #useDefaultForNone()}.
          */
         CodePoint,
         /**
-         * Corresponds to CPython's {@code Py_buffer} convertor.
+         * Corresponds to CPython's {@code Py_buffer} converter.
          */
         Buffer,
     }

graalpython/com.oracle.graal.python.annotations/src/com/oracle/graal/python/annotations/ClinicConverterFactory.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or
+ * data (collectively the "Software"), free of charge and under any and all
+ * copyright rights in the Software, and any and all patent rights owned or
+ * freely licensable by each licensor hereunder covering either (i) the
+ * unmodified Software as contributed to or provided by such licensor, or (ii)
+ * the Larger Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ *
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ *
+ * The above copyright notice and either this complete permission notice or at a
+ * minimum a reference to the UPL must be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package com.oracle.graal.python.annotations;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+/**
+ * Annotates the factory method (which must be static) in the class specified by
+ * {@link ArgumentClinic#conversionClass()}.
+ */
+@Target(ElementType.METHOD)
+public @interface ClinicConverterFactory {
+
+    /**
+     * The boxing optimized execute method variants will not attempt to cast the listed primitive
+     * types and will just pass them directly to the specializations. This does not apply to
+     * primitive values that are already boxed: those are always passed to the converter.
+     */
+    ArgumentClinic.PrimitiveType[] shortCircuitPrimitive() default {};
+
+    /**
+     * Annotates parameter of the factory method which will receive the default value
+     * {@link ArgumentClinic#defaultValue()}.
+     */
+    @Target(ElementType.PARAMETER)
+    @interface DefaultValue {
+    }
+
+    /**
+     * Annotates parameter of the factory method which will receive the value of
+     * {@link ArgumentClinic#useDefaultForNone()}.
+     */
+    @Target(ElementType.PARAMETER)
+    @interface UseDefaultForNone {
+    }
+
+    /**
+     * Annotates parameter of the factory method which will receive the name of the builtin
+     * function.
+     */
+    @Target(ElementType.PARAMETER)
+    @interface BuiltinName {
+    }
+
+    /**
+     * Annotates parameter of the factory method which will receive the index of the argument of the
+     * builtin functions.
+     */
+    @Target(ElementType.PARAMETER)
+    @interface ArgumentIndex {
+    }
+
+    /**
+     * Annotates parameter of the factory method which will receive the name of the argument of the
+     * builtin functions.
+     */
+    @Target(ElementType.PARAMETER)
+    @interface ArgumentName {
+    }
+}

graalpython/com.oracle.graal.python.processor/src/com/oracle/graal/python/processor/ArgumentClinicModel.java

@@ -40,9 +40,8 @@
  */
 package com.oracle.graal.python.processor;
 
-import static java.lang.String.format;
-
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -55,93 +54,12 @@
 import com.oracle.graal.python.annotations.ArgumentClinic.PrimitiveType;
 
 public class ArgumentClinicModel {
-    static final class BuiltinConvertor {
-        public static final String CLINIC_PACKAGE = "com.oracle.graal.python.nodes.function.builtins.clinic";
-
-        public static String getCodeSnippet(ArgumentClinic annotation, BuiltinAnnotation builtin) {
-            switch (annotation.conversion()) {
-                case Boolean:
-                    return format("JavaBooleanConvertorNodeGen.create(%s)", annotation.defaultValue());
-                case String:
-                    String defaultVal = annotation.defaultValue();
-                    if (defaultVal.isEmpty()) {
-                        return format("JavaStringConvertorNodeGen.create(\"%s\")", builtin.name);
-                    } else {
-                        return format("JavaStringConvertorWithDefaultValueNodeGen.create(\"%s\", %s, %s)", builtin.name, defaultVal, annotation.useDefaultForNone());
-                    }
-                case Int:
-                    return format("JavaIntConversionNodeGen.create(%s, %s)", annotation.defaultValue(), annotation.useDefaultForNone());
-                case CodePoint:
-                    return format("CodePointConversionNodeGen.create(\"%s\", %s, %s)", builtin.name, annotation.defaultValue(), annotation.useDefaultForNone());
-                case Buffer:
-                    return "BufferConversionNodeGen.create()";
-                case Index:
-                    return format("IndexConversionNodeGen.create(%s, %s)", annotation.defaultValue(), annotation.useDefaultForNone());
-                case None:
-                    return format("DefaultValueNode.create(%s, %s)", annotation.defaultValue(), annotation.useDefaultForNone());
-                default:
-                    throw new IllegalArgumentException(annotation.conversion().toString());
-            }
-        }
-
-        public static void addImports(ArgumentClinic annotation, Set<String> imports) {
-            // We may add imports for some other prominent constants
-            // Another possibility is to introduce something like ArgumentClinicImportStatic
-            // annotation, or piggy-back on Truffle DSL's ImportStatic annotation, but then we can
-            // also use fully qualified names in such rare cases
-            if (annotation.defaultValue().startsWith("PNone.")) {
-                imports.add("com.oracle.graal.python.builtins.objects.PNone");
-            }
-            if (annotation.conversion() != ClinicConversion.None || (annotation.customConversion().isEmpty() && !annotation.defaultValue().isEmpty())) {
-                imports.add(CLINIC_PACKAGE + '.' + getConvertorImport(annotation));
-            }
-        }
-
-        private static String getConvertorImport(ArgumentClinic annotation) {
-            switch (annotation.conversion()) {
-                case Boolean:
-                    return "JavaBooleanConvertorNodeGen";
-                case String:
-                    return annotation.defaultValue().isEmpty() ? "JavaStringConvertorNodeGen" : "JavaStringConvertorWithDefaultValueNodeGen";
-                case Int:
-                    return "JavaIntConversionNodeGen";
-                case Index:
-                    return "IndexConversionNodeGen";
-                case CodePoint:
-                    return "CodePointConversionNodeGen";
-                case Buffer:
-                    return "BufferConversionNodeGen";
-                case None:
-                    return "DefaultValueNode";
-                default:
-                    throw new IllegalArgumentException(annotation.conversion().toString());
-            }
-        }
-
-        public static PrimitiveType[] getAcceptedPrimitiveTypes(ArgumentClinic annotation) {
-            switch (annotation.conversion()) {
-                case Boolean:
-                    return new PrimitiveType[]{PrimitiveType.Boolean};
-                case String:
-                case CodePoint:
-                case Buffer:
-                    return new PrimitiveType[0];
-                case Int:
-                case Index:
-                    return new PrimitiveType[]{PrimitiveType.Int};
-                case None:
-                    return new PrimitiveType[]{PrimitiveType.Boolean, PrimitiveType.Int, PrimitiveType.Long, PrimitiveType.Double};
-                default:
-                    throw new IllegalArgumentException(annotation.conversion().toString());
-            }
-        }
-    }
 
     /**
      * Mirrors the data of the {@code Builtin} annotation, which cannot be in the "annotations"
      * project because of its dependence on other GraalPython runtime classes.
      */
-    static final class BuiltinAnnotation {
+    public static final class BuiltinAnnotation {
         public final String name;
         public final String[] argumentNames;
 
@@ -188,38 +106,73 @@ static final class ArgumentClinicData {
         public final int index;
         public final Set<PrimitiveType> acceptedPrimitiveTypes;
         public final String castNodeFactory;
+        public final Set<String> imports;
 
-        public ArgumentClinicData(ArgumentClinic annotation, int index, Set<PrimitiveType> acceptedPrimitiveTypes, String castNodeFactory) {
+        private ArgumentClinicData(ArgumentClinic annotation, int index, Set<PrimitiveType> acceptedPrimitiveTypes, String castNodeFactory, Set<String> imports) {
             this.annotation = annotation;
             this.index = index;
             this.acceptedPrimitiveTypes = acceptedPrimitiveTypes;
             this.castNodeFactory = castNodeFactory;
+            this.imports = imports;
         }
 
-        public boolean isClinicArgument() {
-            return annotation != null;
+        private static ConverterFactory getFactory(ArgumentClinic annotation, TypeElement type, ConverterFactory factory) throws ProcessingError {
+            if (factory == null && annotation.args().length != 0) {
+                throw new ProcessingError(type, "No conversionClass specified but arguments were provided");
+            }
+            if (factory != null) {
+                return factory;
+            }
+            if (annotation.conversion() == ClinicConversion.None && annotation.defaultValue().isEmpty()) {
+                throw new ProcessingError(type, "ArgumentClinic annotation must declare either builtin conversion or custom conversion.");
+            }
+            return ConverterFactory.getBuiltin(annotation);
         }
 
-        public static ArgumentClinicData create(ArgumentClinic annotation, TypeElement type, BuiltinAnnotation builtinAnnotation, int index) throws ProcessingError {
+        public static ArgumentClinicData create(ArgumentClinic annotation, TypeElement type, BuiltinAnnotation builtinAnnotation, int index, ConverterFactory annotationFactory)
+                        throws ProcessingError {
             if (annotation == null) {
-                return new ArgumentClinicData(null, index, new HashSet<>(Arrays.asList(PrimitiveType.values())), null);
+                return new ArgumentClinicData(null, index, new HashSet<>(Arrays.asList(PrimitiveType.values())), null, Collections.emptySet());
+            }
+            ConverterFactory factory = getFactory(annotation, type, annotationFactory);
+            if (annotation.args().length != factory.extraParamCount) {
+                throw new ProcessingError(type, "Conversion %s.%s expects %d arguments", factory.fullClassName, factory.methodName, factory.extraParamCount);
             }
 
-            PrimitiveType[] acceptedPrimitives = new PrimitiveType[0];
-            String castNodeFactory;
-            if (annotation.customConversion().isEmpty()) {
-                if (annotation.conversion() == ClinicConversion.None && annotation.defaultValue().isEmpty()) {
-                    throw new ProcessingError(type, "ArgumentClinic annotation must declare either builtin conversion or custom conversion.");
+            String[] args = new String[factory.params.length];
+            int extraParamIndex = 0;
+            for (int i = 0; i < args.length; ++i) {
+                switch (factory.params[i]) {
+                    case BuiltinName:
+                        args[i] = String.format("\"%s\"", builtinAnnotation.name);
+                        break;
+                    case ArgumentIndex:
+                        args[i] = String.valueOf(index);
+                        break;
+                    case ArgumentName:
+                        args[i] = String.format("\"%s\"", builtinAnnotation.argumentNames[index]);
+                        break;
+                    case DefaultValue:
+                        args[i] = annotation.defaultValue();
+                        break;
+                    case UseDefaultForNone:
+                        args[i] = String.valueOf(annotation.useDefaultForNone());
+                        break;
+                    case Extra:
+                        args[i] = annotation.args()[extraParamIndex++];
+                        break;
+                    default:
+                        throw new IllegalStateException("Unsupported ClinicArgument: " + factory.params[i]);
                 }
-                castNodeFactory = BuiltinConvertor.getCodeSnippet(annotation, builtinAnnotation);
-                acceptedPrimitives = BuiltinConvertor.getAcceptedPrimitiveTypes(annotation);
-            } else {
-                castNodeFactory = type.getQualifiedName().toString() + '.' + annotation.customConversion() + "()";
             }
-            if (annotation.shortCircuitPrimitive().length > 0) {
-                acceptedPrimitives = annotation.shortCircuitPrimitive();
+            String castNodeFactory = String.format("%s.%s(%s)", factory.className, factory.methodName, String.join(", ", args));
+            Set<String> imports = new HashSet<>();
+            imports.add(factory.fullClassName);
+            if (annotation.defaultValue().startsWith("PNone.")) {
+                imports.add("com.oracle.graal.python.builtins.objects.PNone");
             }
-            return new ArgumentClinicData(annotation, index, new HashSet<>(Arrays.asList(acceptedPrimitives)), castNodeFactory);
+
+            return new ArgumentClinicData(annotation, index, new HashSet<>(Arrays.asList(factory.acceptedPrimitiveTypes)), castNodeFactory, imports);
         }
     }
 }