[Truffle] Detect ambiguous optional arguments automatically.

* Needs Java 8 for having Method.getParameters(). * Needs to be compiled with $ javac -parameters.
jruby · Jun 26, 2015 · b4459ef · nirvdrum · Jun 26, 2015 · eregon
1 parent 6bbfc01
commit b4459ef
Showing 3 changed files with 126 additions and 5 deletions.
diff --git a/truffle/pom.rb b/truffle/pom.rb
@@ -21,16 +21,19 @@
           'verbose' => 'false',
           'showWarnings' => 'true',
           'showDeprecation' => 'true',
-          'source' => [ '${base.java.version}', '1.7' ],
-          'target' => [ '${base.javac.version}', '1.7' ],
+          'source' => [ '${base.java.version}', '1.8' ],
+          'target' => [ '${base.javac.version}', '1.8' ],
+          'executable' => '/home/eregon/code/graalvm-jdk1.8.0/bin/javac',
+          'fork' => 'true',
           'useIncrementalCompilation' =>  'false' ) do
     execute_goals( 'compile',
                    :id => 'default-compile',
                    :phase => 'compile',
                    'generatedSourcesDirectory' =>  'target/generated-sources',
                    'compilerArgs' => [ '-XDignore.symbol.file=true',
                                        '-J-Duser.language=en',
-                                       '-J-Dfile.encoding=UTF-8' ] )
+                                       '-J-Dfile.encoding=UTF-8',
+                                       '-parameters' ] )
   end
 
   plugin :shade do

diff --git a/truffle/pom.xml b/truffle/pom.xml
@@ -66,6 +66,7 @@ DO NOT MODIFIY - GENERATED CODE
                 <compilerArg>-XDignore.symbol.file=true</compilerArg>
                 <compilerArg>-J-Duser.language=en</compilerArg>
                 <compilerArg>-J-Dfile.encoding=UTF-8</compilerArg>
+                <compilerArg>-parameters</compilerArg>
               </compilerArgs>
             </configuration>
           </execution>
@@ -77,9 +78,11 @@ DO NOT MODIFIY - GENERATED CODE
           <showWarnings>true</showWarnings>
           <showDeprecation>true</showDeprecation>
           <source>${base.java.version}</source>
-          <source>1.7</source>
+          <source>1.8</source>
           <target>${base.javac.version}</target>
-          <target>1.7</target>
+          <target>1.8</target>
+          <executable>/home/eregon/code/graalvm-jdk1.8.0/bin/javac</executable>
+          <fork>true</fork>
           <useIncrementalCompilation>false</useIncrementalCompilation>
         </configuration>
       </plugin>

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/CoreMethodNodeManager.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/CoreMethodNodeManager.java
@@ -12,7 +12,9 @@
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.dsl.GeneratedBy;
 import com.oracle.truffle.api.dsl.NodeFactory;
+import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.nodes.NodeUtil;
+
 import org.jruby.runtime.Visibility;
 import org.jruby.truffle.nodes.RubyNode;
 import org.jruby.truffle.nodes.RubyRootNode;
@@ -32,6 +34,8 @@
 import org.jruby.truffle.runtime.methods.InternalMethod;
 import org.jruby.truffle.runtime.methods.SharedMethodInfo;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -218,6 +222,8 @@ private static RubyRootNode makeGenericMethod(RubyContext context, MethodDetails
             methodNode = nodeFactory.createNode(args);
         }
 
+        verifyNoAmbiguousOptionalArguments(methodDetails);
+
         final CheckArityNode checkArity = new CheckArityNode(context, sourceSection, arity);
         RubyNode sequence = SequenceNode.sequence(context, sourceSection, checkArity, methodNode);
 
@@ -237,6 +243,115 @@ private static RubyRootNode makeGenericMethod(RubyContext context, MethodDetails
         return new RubyRootNode(context, sourceSection, null, sharedMethodInfo, exceptionTranslatingNode);
     }
 
+    private static final Method GET_PARAMETERS = checkParametersNamesAvailable();
+    private static boolean PARAMETER_NAME_AVAILABLE = true;
+
+    private static Method checkParametersNamesAvailable() {
+        try {
+            return Method.class.getMethod("getParameters");
+        } catch (NoSuchMethodException | SecurityException e) {
+            // Java 7 or could not find how to get names of method parameters
+            System.err.println("Could not find Method.getParameters()");
+            return null;
+        }
+    }
+
+    private static boolean verifyNoAmbiguousOptionalArguments(MethodDetails methodDetails) {
+        if (GET_PARAMETERS == null || !PARAMETER_NAME_AVAILABLE) {
+            return true;
+        }
+
+        try {
+            return verifyNoAmbiguousOptionalArgumentsWithReflection(methodDetails);
+        } catch (Exception e) {
+            e.printStackTrace();
+            return true;
+        }
+    }
+
+    private static boolean verifyNoAmbiguousOptionalArgumentsWithReflection(MethodDetails methodDetails) throws ReflectiveOperationException {
+        boolean success = true;
+
+        if (methodDetails.getMethodAnnotation().optional() > 0) {
+            int opt = methodDetails.getMethodAnnotation().optional();
+            if (methodDetails.getMethodAnnotation().needsBlock()) {
+                opt++;
+            }
+
+            Class<?> node = methodDetails.getNodeFactory().getNodeClass();
+
+            for (int i = 1; i <= opt; i++) {
+                boolean unguardedObjectArgument = false;
+                String errors = "";
+                for (Method method : node.getDeclaredMethods()) {
+                    if (method.isAnnotationPresent(Specialization.class)) {
+                        // count from the end to ignore optional VirtualFrame in front.
+                        Class<?>[] parameterTypes = method.getParameterTypes();
+                        int n = parameterTypes.length - i;
+                        Class<?> parameterType = parameterTypes[n];
+                        Object[] parameters = (Object[]) GET_PARAMETERS.invoke(method);
+
+                        Object parameter = parameters[n];
+                        boolean isNamePresent = (boolean) parameter.getClass().getMethod("isNamePresent").invoke(parameter);
+                        if (!isNamePresent) {
+                            PARAMETER_NAME_AVAILABLE = false;
+                            System.err.println("Method parameters names are not available for " + method);
+                            return true;
+                        }
+                        String name = (String) parameter.getClass().getMethod("getName").invoke(parameter);
+
+                        if (parameterType == Object.class && !name.startsWith("unused")) {
+                            String[] guards = method.getAnnotation(Specialization.class).guards();
+                            if (!isGuarded(name, guards)) {
+                                unguardedObjectArgument = true;
+                                errors += "\"" + name + "\" in " + methodToString(method, parameterTypes, parameters) + "\n";
+                            }
+                        }
+                    }
+                }
+
+                if (unguardedObjectArgument) {
+                    success = false;
+                    System.err.println("Ambiguous optional argument in " + node.getCanonicalName() + ":");
+                    System.err.println(errors);
+                }
+            }
+        }
+
+        if (!success) {
+            System.exit(1);
+        }
+
+        return success;
+    }
+
+    private static boolean isGuarded(String name, String[] guards) {
+        for (String guard : guards) {
+            if (guard.equals("wasProvided(" + name + ")") ||
+                    guard.equals("wasNotProvided(" + name + ")") ||
+                    guard.equals("wasNotProvided(" + name + ") || isRubiniusUndefined(" + name + ")") ||
+                    guard.equals("isNil(" + name + ")")) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static String methodToString(Method method, Class<?>[] parameterTypes, Object[] parameters) throws ReflectiveOperationException {
+        StringBuilder str = new StringBuilder();
+        str.append(method.getName()).append("(");
+        for (int i = 0; i < parameters.length; i++) {
+            Object parameter = parameters[i];
+            String name = (String) parameter.getClass().getMethod("getName").invoke(parameter);
+            str.append(parameterTypes[i].getSimpleName()).append(" ").append(name);
+            if (i < parameters.length - 1) {
+                str.append(", ");
+            }
+        }
+        str.append(")");
+        return str.toString();
+    }
+
     public static class MethodDetails {
 
         private final CoreClass classAnnotation;