[Truffle] Arity: replace List<String> with String[], fix bugs and con…

…fusion around hasKeywords and acceptsKeywords
jruby · Jul 31, 2015 · 7ec4860 · 7ec4860
1 parent e3f9f81
commit 7ec4860
Showing 5 changed files with 47 additions and 46 deletions.
diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/arguments/CheckArityNode.java b/truffle/src/main/java/org/jruby/truffle/nodes/arguments/CheckArityNode.java
@@ -51,12 +51,12 @@ public void executeVoid(VirtualFrame frame) {
         //TODO (MS): Check merge
         if (RubyArguments.isKwOptimized(frame.getArguments())) {
             given = RubyArguments.getUserArgumentsCount(frame.getArguments())
-                    - arity.getCountKeywords() - 2;
+                    - arity.getKeywordsCount() - 2;
         } else {
             given = RubyArguments.getUserArgumentsCount(frame.getArguments());
         }
 
-        if (arity.hasKeywords()) {
+        if (arity.acceptsKeywords()) {
             keywordArguments = RubyArguments.getUserKeywordsHash(frameArguments, arity.getRequired());
         } else {
             keywordArguments = null;

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/dispatch/RubyCallNode.java b/truffle/src/main/java/org/jruby/truffle/nodes/dispatch/RubyCallNode.java
@@ -238,8 +238,7 @@ public RubyNode[] expandedArgumentNodes(InternalMethod method, RubyNode[] argume
         final RubyNode[] result;
 
         boolean shouldExpand = true;
-        if (method == null
-                || method.getSharedMethodInfo().getArity().getKeywordArguments() == null) {
+        if (method == null || !method.getSharedMethodInfo().getArity().acceptsKeywords()) {
             // no keyword arguments in method definition
             shouldExpand = false;
         } else if (argumentNodes.length != 0
@@ -258,9 +257,9 @@ public RubyNode[] expandedArgumentNodes(InternalMethod method, RubyNode[] argume
         }
 
         if (shouldExpand) {
-            List<String> kwargs = method.getSharedMethodInfo().getArity().getKeywordArguments();
+            String[] kwargs = method.getSharedMethodInfo().getArity().getKeywordArguments();
 
-            int countArgNodes = argumentNodes.length + kwargs.size() + 1;
+            int countArgNodes = argumentNodes.length + kwargs.length + 1;
             if (argumentNodes.length == 0) {
                 countArgNodes++;
             }
@@ -315,7 +314,7 @@ public RubyNode[] expandedArgumentNodes(InternalMethod method, RubyNode[] argume
             result[i++] = new MarkerNode(getContext(), null);
 
             if (restKeywordLabels.size() > 0
-                    && !method.getSharedMethodInfo().getArity().hasKeyRest()) {
+                    && !method.getSharedMethodInfo().getArity().hasKeywordsRest()) {
                 result[firstMarker] = new UnknownArgumentErrorNode(getContext(), null, restKeywordLabels.get(0));
             } else if (restKeywordLabels.size() > 0) {
                 i = 0;

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/RubyArguments.java b/truffle/src/main/java/org/jruby/truffle/runtime/RubyArguments.java
@@ -42,7 +42,7 @@ public static Object[] pack(InternalMethod method, MaterializedFrame declaration
 
         return packed;
     }
-    
+
     public static Object getOptimizedKeywordArgument(Object[] arguments,
             int index) {
         return arguments[arguments.length - 1 + index];
@@ -86,7 +86,7 @@ public static int getNamedUserArgumentsCount(Object[] internalArguments) {
         if (isKwOptimized(internalArguments)) {
             return getUserArgumentsCount(internalArguments)
                     - getMethod(internalArguments).getSharedMethodInfo().getArity()
-                            .getKeywordArguments().size() - 1;
+                            .getKeywordsCount() - 1;
         } else {
             return getUserArgumentsCount(internalArguments);
         }

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/methods/Arity.java b/truffle/src/main/java/org/jruby/truffle/runtime/methods/Arity.java
@@ -9,12 +9,11 @@
  */
 package org.jruby.truffle.runtime.methods;
 
-import java.util.Collections;
-import java.util.List;
+import java.util.Arrays;
 
 public class Arity {
 
-    public static final List<String> NO_KEYWORDS = Collections.emptyList();
+    public static final String[] NO_KEYWORDS = new String[]{};
     public static final Arity NO_ARGUMENTS = new Arity(0, 0, false);
     public static final Arity ONE_REQUIRED = new Arity(1, 0, false);
     public static final Arity AT_LEAST_ONE = new Arity(1, 0, true);
@@ -23,21 +22,20 @@ public class Arity {
     private final int optional;
     private final boolean hasRest;
     private final int postRequired;
-    private final boolean hasKeyRest;
-
-    private final List<String> keywordArguments;
+    private final boolean hasKeywordsRest;
+    private final String[] keywordArguments;
 
     public Arity(int preRequired, int optional, boolean hasRest) {
         this(preRequired, optional, hasRest, 0, NO_KEYWORDS, false);
     }
 
-    public Arity(int preRequired, int optional, boolean hasRest, int postRequired, List<String> keywordArguments, boolean hasKeyRest) {
+    public Arity(int preRequired, int optional, boolean hasRest, int postRequired, String[] keywordArguments, boolean hasKeywordsRest) {
         this.preRequired = preRequired;
         this.optional = optional;
         this.hasRest = hasRest;
         this.postRequired = postRequired;
         this.keywordArguments = keywordArguments;
-        this.hasKeyRest = hasKeyRest;
+        this.hasKeywordsRest = hasKeywordsRest;
 
         assert keywordArguments != null && preRequired >= 0 && optional >= 0 && postRequired >= 0 : toString();
     }
@@ -66,22 +64,26 @@ public int getRestPosition() {
         return preRequired + optional;
     }
 
+    public boolean acceptsKeywords() {
+        return hasKeywords() || hasKeywordsRest();
+    }
+
     public boolean hasKeywords() {
-        return keywordArguments != NO_KEYWORDS;
+        return keywordArguments.length != 0;
     }
 
-    public int getCountKeywords() {
-        return keywordArguments.size();
+    public int getKeywordsCount() {
+        return keywordArguments.length;
     }
 
-    public boolean hasKeyRest() {
-        return hasKeyRest;
+    public boolean hasKeywordsRest() {
+        return hasKeywordsRest;
     }
 
     public int getArityNumber() {
-        int count = preRequired + postRequired;
+        int count = getRequired();
 
-        if (hasKeywords()) {
+        if (acceptsKeywords()) {
             count++;
         }
 
@@ -92,7 +94,7 @@ public int getArityNumber() {
         return count;
     }
 
-    public List<String> getKeywordArguments() {
+    public String[] getKeywordArguments() {
         return keywordArguments;
     }
 
@@ -103,8 +105,8 @@ public String toString() {
                 ", optional=" + optional +
                 ", hasRest=" + hasRest +
                 ", postRequired=" + postRequired +
-                ", keywordArguments=" + keywordArguments +
-                ", hasKeyRest=" + hasKeyRest +
+                ", keywordArguments=" + Arrays.toString(keywordArguments) +
+                ", hasKeywordsRest=" + hasKeywordsRest +
                 '}';
     }
 }
diff --git a/truffle/src/main/java/org/jruby/truffle/translator/MethodTranslator.java b/truffle/src/main/java/org/jruby/truffle/translator/MethodTranslator.java
@@ -46,6 +46,7 @@
 import org.jruby.truffle.runtime.methods.SharedMethodInfo;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 class MethodTranslator extends BodyTranslator {
@@ -265,25 +266,24 @@ public RubyNode compileFunctionNode(SourceSection sourceSection, String methodNa
     }
 
     public static Arity getArity(org.jruby.ast.ArgsNode argsNode) {
-        final List<String> keywordArguments;
-
-        if (argsNode.hasKwargs()) {
-            keywordArguments = new ArrayList<>();
-            if (argsNode.getKeywords() != null) {
-                for (org.jruby.ast.Node node : argsNode.getKeywords().children()) {
-                    final KeywordArgNode kwarg = (KeywordArgNode) node;
-                    final AssignableNode assignableNode = kwarg.getAssignable();
-
-                    if (assignableNode instanceof LocalAsgnNode) {
-                        keywordArguments.add(((LocalAsgnNode) assignableNode)
-                                .getName());
-                    } else if (assignableNode instanceof DAsgnNode) {
-                        keywordArguments.add(((DAsgnNode) assignableNode)
-                                .getName());
-                    } else {
-                        throw new UnsupportedOperationException(
-                                "unsupported keyword arg " + node);
-                    }
+        final String[] keywordArguments;
+
+        if (argsNode.hasKwargs() && argsNode.getKeywords() != null) {
+            final org.jruby.ast.Node[] keywordNodes = argsNode.getKeywords().children();
+            final int keywordsCount = keywordNodes.length;
+
+            keywordArguments = new String[keywordsCount];
+            for (int i = 0; i < keywordsCount; i++) {
+                final KeywordArgNode kwarg = (KeywordArgNode) keywordNodes[i];
+                final AssignableNode assignableNode = kwarg.getAssignable();
+
+                if (assignableNode instanceof LocalAsgnNode) {
+                    keywordArguments[i] = ((LocalAsgnNode) assignableNode).getName();
+                } else if (assignableNode instanceof DAsgnNode) {
+                    keywordArguments[i] = ((DAsgnNode) assignableNode).getName();
+                } else {
+                    throw new UnsupportedOperationException(
+                            "unsupported keyword arg " + kwarg);
                 }
             }
         } else {