Translate nodes (method call) - fix keyword arguments handling

Author: andrykonchin

spec/tags/truffle/parsing/parsing_tags.txt

@@ -76,7 +76,6 @@ fails:Parsing a Method call (super / outside a method body without explicit argu
 fails:Parsing a Method call (Arguments/with a &-deconstruction to a block argument (&bar)) case is parsed correctly
 fails:Parsing a Method call (Arguments/with block literal argument) case is parsed correctly
 fails:Parsing a Method call (Arguments/with double splat operator (**kw)) case is parsed correctly
-fails:Parsing a Method call (Arguments/with keyword arguments) case is parsed correctly
 fails:Parsing a Method call (Arguments/with positional argument and splat operator (a, *args)) case is parsed correctly
 fails:Parsing a Method call (Arguments/with splat operator (*args)) case is parsed correctly
 fails:Parsing a Method call (Arguments/with splat operator and positional arguments (*args, a)) case is parsed correctly

spec/truffle/parsing/fixtures/method_calls/arguments/with_keyword_arguments.yaml

@@ -2,9 +2,8 @@ subject: "Method call"
 description: "Arguments/with keyword arguments"
 notes: |
   Keyword arguments are represented with PackedHashStoreLibrary$SmallHashLiteralNode node.
-  The only difference between keyword arguments and a Hash literal argument is a descriptor attribute:
-    `descriptor = org.truffleruby.language.arguments.KeywordArgumentsDescriptor@...`
-
+  The only difference between keyword arguments and a Hash literal argument is a `descriptor` attribute:
+    descriptor = KeywordArgumentsDescriptor(keywords = [bar])
 focused_on_node: "org.truffleruby.language.dispatch.RubyCallNode"
 ruby: |
   foo(bar: :baz)

src/main/java/org/truffleruby/language/arguments/ReadUserKeywordsHashNode.java

@@ -26,7 +26,7 @@ public RubyHash execute(VirtualFrame frame) {
         if (keywordArgumentsProfile.profile(descriptor instanceof KeywordArgumentsDescriptor)) {
             final RubyHash keywords = (RubyHash) RubyArguments.getLastArgument(frame);
             assert !keywords.empty();
-            assert assertHashMatchesDescriptor(keywords, (KeywordArgumentsDescriptor) descriptor);
+            assert assertHashMatchesDescriptor(keywords, (KeywordArgumentsDescriptor) descriptor); // TODO: seems incorrect as far as descriptor contains keyword names from the arguments, not declared in a called method
             return keywords;
         } else {
             return null;

src/main/java/org/truffleruby/parser/YARPTranslator.java

@@ -49,6 +49,7 @@
 import org.truffleruby.language.RubyRootNode;
 import org.truffleruby.language.RubyTopLevelRootNode;
 import org.truffleruby.language.SourceIndexLength;
+import org.truffleruby.language.arguments.ArgumentsDescriptor;
 import org.truffleruby.language.arguments.EmptyArgumentsDescriptor;
 import org.truffleruby.language.arguments.ProfileArgumentNodeGen;
 import org.truffleruby.language.arguments.ReadSelfNode;
@@ -249,10 +250,12 @@ public RubyNode visitArrayPatternNode(Nodes.ArrayPatternNode node) {
         return defaultVisit(node);
     }
 
+    // handled in #visitHashNode
     public RubyNode visitAssocNode(Nodes.AssocNode node) {
         return defaultVisit(node);
     }
 
+    // handled in #visitHashNode
     public RubyNode visitAssocSplatNode(Nodes.AssocSplatNode node) {
         return defaultVisit(node);
     }
@@ -446,6 +449,7 @@ public RubyNode visitCallNode(Nodes.CallNode node) {
         }
 
         var translatedArguments = translate(arguments);
+        var argumentsDescriptor = getKeywordArgumentsDescriptor(arguments);
 
         // If the receiver is explicit or implicit 'self' then we can call private methods
         final boolean ignoreVisibility = node.receiver == null || node.receiver instanceof Nodes.SelfNode;
@@ -493,7 +497,7 @@ public RubyNode visitCallNode(Nodes.CallNode node) {
                 receiver,
                 methodName,
                 null,
-                EmptyArgumentsDescriptor.INSTANCE,
+                argumentsDescriptor,
                 translatedArguments,
                 false,
                 ignoreVisibility,
@@ -506,6 +510,35 @@ public RubyNode visitCallNode(Nodes.CallNode node) {
         return rubyNode;
     }
 
+    private ArgumentsDescriptor getKeywordArgumentsDescriptor(Nodes.Node[] arguments) {
+        if (arguments.length == 0) {
+            return EmptyArgumentsDescriptor.INSTANCE;
+        }
+
+        Nodes.Node last = arguments[arguments.length - 1];
+
+        if (!(last instanceof Nodes.KeywordHashNode)) {
+            return EmptyArgumentsDescriptor.INSTANCE;
+        }
+
+        var keywords = (Nodes.KeywordHashNode) last;
+
+        final List<String> names = new ArrayList<>();
+
+        for (var n : keywords.elements) {
+            // TODO: we ignore InterpolatedSymbolNode and other arbitrary possible key types here
+            //      not sure whether we need keys at all
+            if (n instanceof Nodes.AssocNode assoc && assoc.key instanceof Nodes.SymbolNode symbol) {
+                names.add(toString(symbol.unescaped));
+            }
+        }
+
+        var array = names.toArray(StringUtils.EMPTY_STRING_ARRAY);
+        var manager = language.keywordArgumentsDescriptorManager;
+        var descriptor = manager.getArgumentsDescriptor(array);
+        return descriptor;
+    }
+
     public RubyNode visitCallOperatorWriteNode(Nodes.CallOperatorWriteNode node) {
         return defaultVisit(node);
     }
@@ -1287,7 +1320,9 @@ public RubyNode visitInterpolatedXStringNode(Nodes.InterpolatedXStringNode node)
     }
 
     public RubyNode visitKeywordHashNode(Nodes.KeywordHashNode node) {
-        return defaultVisit(node);
+        // store keyword arguments as a literal Hash
+        final var hash = new Nodes.HashNode(node.elements, node.startOffset, node.length);
+        return hash.accept(this);
     }
 
     public RubyNode visitLambdaNode(Nodes.LambdaNode node) {
@@ -2154,6 +2189,11 @@ protected String toString(Nodes.Node node) {
                 node.length, sourceEncoding.tencoding, false), sourceEncoding);
     }
 
+    protected String toString(byte[] bytes) {
+        return TStringUtils.toJavaStringOrThrow(TruffleString.fromByteArrayUncached(bytes, 0,
+                bytes.length, sourceEncoding.tencoding, false), sourceEncoding);
+    }
+
     protected SourceSection getSourceSection(Nodes.Node yarpNode) {
         return source.createSection(yarpNode.startOffset, yarpNode.length);
     }