[Truffle] More work on keyword arguments.

jruby · Dec 12, 2014 · 0618ada · 0618ada
1 parent ca99b12
commit 0618ada
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 19 deletions.
diff --git a/core/src/main/java/org/jruby/truffle/nodes/methods/arguments/CheckArityNode.java b/core/src/main/java/org/jruby/truffle/nodes/methods/arguments/CheckArityNode.java
@@ -41,7 +41,8 @@ public void executeVoid(VirtualFrame frame) {
 
     private boolean checkArity(int given) {
         if (arity.hasKeywords()) {
-            given -= 1;
+            // TODO(CS): TODO
+            return true;
         }
 
         if (arity.getRequired() != 0 && given < arity.getRequired()) {

diff --git a/core/src/main/java/org/jruby/truffle/nodes/methods/arguments/ReadKeywordArgumentNode.java b/core/src/main/java/org/jruby/truffle/nodes/methods/arguments/ReadKeywordArgumentNode.java
@@ -24,11 +24,13 @@
 
 public class ReadKeywordArgumentNode extends RubyNode {
 
+    private final int minimum;
     private final String name;
     @Child protected RubyNode defaultValue;
 
-    public ReadKeywordArgumentNode(RubyContext context, SourceSection sourceSection, String name, RubyNode defaultValue) {
+    public ReadKeywordArgumentNode(RubyContext context, SourceSection sourceSection, int minimum, String name, RubyNode defaultValue) {
         super(context, sourceSection);
+        this.minimum = minimum;
         this.name = name;
         this.defaultValue = defaultValue;
     }
@@ -37,20 +39,12 @@ public ReadKeywordArgumentNode(RubyContext context, SourceSection sourceSection,
     public Object execute(VirtualFrame frame) {
         notDesignedForCompilation();
 
-        final int last = RubyArguments.getUserArgumentsCount(frame.getArguments()) - 1;
+        final RubyHash hash = getKeywordsHash(frame);
 
-        if (last == -1) {
+        if (hash == null) {
             return defaultValue.execute(frame);
         }
 
-        final Object hashValue = RubyArguments.getUserArgument(frame.getArguments(), last);
-
-        if (!(hashValue instanceof RubyHash)) {
-            return defaultValue.execute(frame);
-        }
-
-        final RubyHash hash = (RubyHash) hashValue;
-
         Object value = null;
 
         for (Map.Entry<Object, Object> entry : hash.slowToMap().entrySet()) {
@@ -67,4 +61,18 @@ public Object execute(VirtualFrame frame) {
         return value;
     }
 
+    private RubyHash getKeywordsHash(VirtualFrame frame) {
+        if (RubyArguments.getUserArgumentsCount(frame.getArguments()) <= minimum) {
+            return null;
+        }
+
+        final Object lastArgument = RubyArguments.getUserArgument(frame.getArguments(), RubyArguments.getUserArgumentsCount(frame.getArguments()) - 1);
+
+        if (lastArgument instanceof RubyHash) {
+            return (RubyHash) lastArgument;
+        }
+
+        return null;
+    }
+
 }
diff --git a/core/src/main/java/org/jruby/truffle/translator/LoadArgumentsTranslator.java b/core/src/main/java/org/jruby/truffle/translator/LoadArgumentsTranslator.java
@@ -40,6 +40,7 @@ private enum State {
         POST
     }
 
+    private int required;
     private int index;
     private State state;
 
@@ -65,6 +66,7 @@ public RubyNode visitArgsNode(org.jruby.ast.ArgsNode node) {
             for (org.jruby.ast.Node arg : node.getPre().childNodes()) {
                 sequence.add(arg.accept(this));
                 ++index;
+                required++;
             }
         }
 
@@ -89,6 +91,7 @@ public RubyNode visitArgsNode(org.jruby.ast.ArgsNode node) {
             for (org.jruby.ast.Node arg : node.getPost().childNodes()) {
                 sequence.add(arg.accept(this));
                 ++index;
+                required++;
             }
         }
 
@@ -136,7 +139,7 @@ public RubyNode visitKeywordArgNode(org.jruby.ast.KeywordArgNode node) {
             throw new UnsupportedOperationException();
         }
 
-        final RubyNode readNode = new ReadKeywordArgumentNode(context, sourceSection, name, defaultValue);
+        final RubyNode readNode = new ReadKeywordArgumentNode(context, sourceSection, required, name, defaultValue);
         final FrameSlot slot = methodBodyTranslator.getEnvironment().getFrameDescriptor().findFrameSlot(name);
 
         return WriteLocalVariableNodeFactory.create(context, sourceSection, slot, readNode);
@@ -224,7 +227,14 @@ private RubyNode translateLocalAssignment(ISourcePosition sourcePosition, String
         } else {
             // Optional argument
             final RubyNode defaultValue = valueNode.accept(this);
-            readNode = new ReadOptionalArgumentNode(context, sourceSection, index, index + 1 + argsNode.getPostCount(), defaultValue);
+
+            int minimum = index + 1 + argsNode.getPostCount();
+
+            if (argsNode.hasKwargs()) {
+                minimum += 1;
+            }
+
+            readNode = new ReadOptionalArgumentNode(context, sourceSection, index, minimum, defaultValue);
         }
 
         final FrameSlot slot = methodBodyTranslator.getEnvironment().getFrameDescriptor().findFrameSlot(name);

diff --git a/spec/truffle/tags/language/method_tags.txt b/spec/truffle/tags/language/method_tags.txt
@@ -32,8 +32,6 @@ fails:"A method assigns local variables from method parameters for definition \n
 fails:"A method assigns local variables from method parameters for definition \n    def m(a, (b, (c, *d), *e))\n      [a, b, c, d, e]\n    end"
 fails:"A method assigns local variables from method parameters for definition \n    def m(a, (b, (c, *d, (e, (*f)), g), (h, (i, j))))\n      [a, b, c, d, e, f, g, h, i, j]\n    end"
 fails:"A method assigns local variables from method parameters for definition \n    def m(a, b:) [a, b] end"
-fails:"A method assigns local variables from method parameters for definition \n    def m(a, b: 1) [a, b] end"
-fails:"A method assigns local variables from method parameters for definition \n    def m(a, **) a end"
 fails:"A method assigns local variables from method parameters for definition \n    def m(a, **k) [a, k] end"
 fails:"A method assigns local variables from method parameters for definition \n    def m(a=1, (b, (c, *d))) [a, b, c, d] end"
 fails:"A method assigns local variables from method parameters for definition \n    def m(a=1, (b, (c, *d), *e)) [a, b, c, d, e] end"

diff --git a/spec/truffle/tags/language/predefined_tags.txt b/spec/truffle/tags/language/predefined_tags.txt
@@ -48,4 +48,3 @@ fails:Predefined global $-0 does not call #to_str to convert the object to a Str
 fails:Predefined global $-0 raises a TypeError if assigned a Fixnum
 fails:Predefined global $-0 raises a TypeError if assigned a boolean
 fails:Global variable $0 raises a TypeError when not given an object that can be coerced to a String
-fails:Predefined global $~ is set at the method-scoped level rather than block-scoped
diff --git a/spec/truffle/tags/language/versions/def_2.0_tags.txt b/spec/truffle/tags/language/versions/def_2.0_tags.txt
@@ -1,5 +1,3 @@
-fails:An instance method with keyword arguments treats a sole hash argument correctly
-fails:An instance method with keyword arguments correctly distinguishes between optional and keyword arguments
 fails:An instance method with keyword arguments correctly distinguishes between rest and keyword arguments
 fails:An instance method with keyword arguments should allow keyword rest arguments
 fails:An instance method with keyword arguments when there is a single keyword argument raises an argument error when a non-keyword argument is provided