[GR-25788] GraalPython allows to put multiple values for keyword argument.

ppisl · ppisl · commit 5268374b2dbf · 2020-10-07T11:44:37.000+02:00
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_call.py b/graalpython/com.oracle.graal.python.test/src/tests/test_call.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 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
@@ -167,6 +167,8 @@ def f24(*args, a, b=5):
 def f25(*args, a, b=5, **kwds):
     pass
 
+def f26(**kw):
+    return kw
 
 def assert_parses(call_expr):
     raised = False
@@ -234,8 +236,7 @@ def test_parse_args():
 
     assert_parses("f11(a=1, b=2)")
     assert_parses("f11(a=1, b=2, c=3)")
-    # TODO
-    # assert_call_raises(SyntaxError, "f11(a=1, b=2, a=3)")  # SyntaxError: keyword argument repeated
+    assert_call_raises(SyntaxError, "f11(a=1, b=2, a=3)")  # SyntaxError: keyword argument repeated
     assert_call_raises(TypeError, "f11(1, b=2, a=3)")  # TypeError: f11() got multiple values for argument 'a'
 
     assert_parses("f12(1,2)")
@@ -286,8 +287,7 @@ def test_parse_args():
     assert_parses("f20(a=1)")
     assert_parses("f20(a=1, b=2)")
     assert_parses("f20(a=1, b=2, c=3)")
-    # TODO
-    # assert_call_raises(SyntaxError, "f20(a=1, b=2, a=3)")  # SyntaxError: keyword argument repeated
+    assert_call_raises(SyntaxError, "f20(a=1, b=2, a=3)")  # SyntaxError: keyword argument repeated
     assert_call_raises(TypeError, "f20(1, b=2)")  # TypeError: f20() takes 0 positional arguments but 1 positional argument (and 1 keyword-only argument) were given
     assert_call_raises(TypeError, "f20(1)")  # TypeError: f20() takes 0 positional arguments but 1 was given
 
@@ -324,3 +324,36 @@ def test_parse_args():
     assert_call_raises(TypeError, "f25(1,2,3,c=6)")  # TypeError: f25() missing 1 required keyword-only argument: 'a'
     assert_parses("f25(a=4,c=6)")
     assert_parses("f25(a=4)")
+
+def test_multiple_values_for_keyword_argument():
+    assert_call_raises(TypeError, "f26(a=1, **{'a' : 2})") # TypeError: f26() got multiple values for keyword argument 'a'
+    assert_call_raises(TypeError, "f26(**{'a' : 4}, **{'a': 3})")  # TypeError: f26() got multiple values for keyword argument 'a'
+    
+b = 1
+def test_argument_must_be_mapping():    
+    assert_call_raises(TypeError, "f26(a=1, **b)") # TypeError: f26() argument after ** must be a mapping, not int
+    assert_call_raises(TypeError, "f26(**b)") # TypeError: f26() argument after ** must be a mapping, not int
+    class MyDict(dict):
+        pass
+    d = MyDict()
+    assert f26(**d) == {}
+
+fooo = f26
+def test_multiple_values_with_callable_name():
+    def assert_get_message(err, fn, *args, **kwargs):
+        raised = False
+        try:
+            fn(*args, **kwargs)
+        except err as ex:
+            return str(ex)
+        assert raised
+        
+    def assert_call_raises_get_message(exception, call_expr):
+        return assert_get_message(exception, eval, call_expr)
+
+    msg = assert_call_raises_get_message(TypeError, "fooo(a=1, **b)") # TypeError: f26() argument after ** must be a mapping, not int
+    assert msg == "f26() argument after ** must be a mapping, not int"
+    
+    msg = assert_call_raises_get_message(TypeError, "fooo(**{'a' : 4}, **{'a': 3})")  # TypeError: f26() got multiple values for keyword argument 'a'
+    assert msg == "f26() got multiple values for keyword argument 'a'"
+    
diff --git a/graalpython/com.oracle.graal.python.test/testData/goldenFiles/BasicTests/call12.tast b/graalpython/com.oracle.graal.python.test/testData/goldenFiles/BasicTests/call12.tast
@@ -10,7 +10,7 @@ ModuleRootNode Name: <module 'call12'> SourceSection: [0,25]`foo(**mydict1, **my
       KeywordArgumentsNodeGen SourceSection: None
         CompactKeywordsNodeGen SourceSection: None
         ExecuteKeywordStarargsNodeGen SourceSection: None
-          DictConcatNodeGen SourceSection: None
+          ConcatKeywordsNodeGen SourceSection: None
             ReadNameNodeGen SourceSection: [6,13]`mydict1`
               Identifier: mydict1
               IsBuiltinClassProfile SourceSection: None
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java
@@ -43,6 +43,7 @@
 public abstract class ErrorMessages {
     public static final String ABSOLUTE_VALUE_TOO_LARGE = "absolute value too large";
     public static final String ARG_AFTER_MUST_BE_ITERABLE = "argument after * must be an iterable, not %p";
+    public static final String ARG_AFTER_MUST_BE_MAPPING = "%s() argument after ** must be a mapping, not %p";
     public static final String ARG_CONVERTED_NOT_EXECUTABLE = "argument converted is not executable";
     public static final String ARG_D_MUST_BE_S = "%s arg %d must be a %s";
     public static final String ARG_D_MUST_BE_S_NOT_P = "%s argument %d must be %s, not %p";
@@ -252,7 +253,7 @@ public abstract class ErrorMessages {
     public static final String GETATTR_ATTRIBUTE_NAME_MUST_BE_STRING = "getattr(): attribute name must be string";
     public static final String GETTING_THER_SOURCE_NOT_SUPPORTED_FOR_P = "getting the source is not supported for '%p'";
     public static final String GLOBALS_MUST_BE_DICT = "%s() globals must be a dict, not %p";
-    public static final String GOT_MULTIPLE_VALUES_FOR_ARG = "%s() got multiple values for argument '%s'";
+    public static final String GOT_MULTIPLE_VALUES_FOR_ARG = "%s() got multiple values for keyword argument '%s'";
     public static final String GOT_SOME_POS_ONLY_ARGS_PASSED_AS_KEYWORD = "%s() got some positional-only arguments passed as keyword arguments: '%s'";
     public static final String GOT_UNEXPECTED_KEYWORD_ARG = "%s() got an unexpected keyword argument '%s'";
     public static final String HAS_NO_ATTR = "%s has no attribute %s";
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/CreateArgumentsNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/CreateArgumentsNode.java
@@ -500,7 +500,7 @@ Object[] applyCached(Object callee, @SuppressWarnings("unused") Signature callee
                 if (kwIdx != -1) {
                     if (positionalOnlyArgIndex > -1 && kwIdx < positionalOnlyArgIndex) {
                         if (unusedKeywords != null) {
-                            unusedKeywords[k++] = kwArg;
+                            addKeyword(unusedKeywords, k++, kwArg, callee, raise);
                         } else {
                             posArgOnlyPassedAsKeywordProfile.enter();
                             posArgOnlyPassedAsKeywordNames = addPosArgOnlyPassedAsKeyword(posArgOnlyPassedAsKeywordNames, name);
@@ -512,7 +512,7 @@ Object[] applyCached(Object callee, @SuppressWarnings("unused") Signature callee
                         PArguments.setArgument(arguments, kwIdx, kwArg.getValue());
                     }
                 } else if (unusedKeywords != null) {
-                    unusedKeywords[k++] = kwArg;
+                    addKeyword(unusedKeywords, k++, kwArg, callee, raise);
                 } else {
                     additionalKwds++;
                     lastWrongKeyword = name;
@@ -553,7 +553,7 @@ Object[] applyUncached(Object callee, Signature calleeSignature, Object[] argume
                 if (kwIdx != -1) {
                     if (positionalOnlyArgIndex > -1 && kwIdx < positionalOnlyArgIndex) {
                         if (unusedKeywords != null) {
-                            unusedKeywords[k++] = kwArg;
+                            addKeyword(unusedKeywords, k++, kwArg, callee, raise);
                         } else {
                             posArgOnlyPassedAsKeywordProfile.enter();
                             posArgOnlyPassedAsKeywordNames = addPosArgOnlyPassedAsKeyword(posArgOnlyPassedAsKeywordNames, name);
@@ -565,7 +565,7 @@ Object[] applyUncached(Object callee, Signature calleeSignature, Object[] argume
                         PArguments.setArgument(arguments, kwIdx, kwArg.getValue());
                     }
                 } else if (unusedKeywords != null) {
-                    unusedKeywords[k++] = kwArg;
+                    addKeyword(unusedKeywords, k++, kwArg, callee, raise);
                 } else {
                     additionalKwds++;
                     lastWrongKeyword = name;
@@ -575,6 +575,16 @@ Object[] applyUncached(Object callee, Signature calleeSignature, Object[] argume
             return arguments;
         }
 
+        private static void addKeyword(PKeyword[] keywords, int where, PKeyword keyword, Object callee, PRaiseNode raise) {
+            String name = keyword.getName();
+            for (int i = 0; i < where; i++) {
+                if (keywords[i].getName().equals(name)) {
+                    throw raise.raise(PythonBuiltinClassType.TypeError, ErrorMessages.GOT_MULTIPLE_VALUES_FOR_ARG, CreateArgumentsNode.getName(callee), name);
+                }
+            }
+            keywords[where] = keyword;
+        }
+
         @TruffleBoundary
         private static List<String> addPosArgOnlyPassedAsKeyword(List<String> names, String name) {
             if (names == null) {
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/ConcatKeywordsNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/ConcatKeywordsNode.java
@@ -0,0 +1,115 @@
+/*
+ * 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.nodes.argument.keywords;
+
+import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.GetDictStorageNode;
+import com.oracle.graal.python.builtins.objects.common.HashingStorage;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
+import com.oracle.graal.python.builtins.objects.dict.PDict;
+import com.oracle.graal.python.nodes.expression.ExpressionNode;
+import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.GenerateNodeFactory;
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.library.CachedLibrary;
+import com.oracle.truffle.api.nodes.ExplodeLoop;
+import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.profiles.ConditionProfile;
+
+@GenerateNodeFactory
+public abstract class ConcatKeywordsNode extends ExpressionNode {
+
+    @Node.Children final ExpressionNode[] mappables;
+
+    protected ConcatKeywordsNode(ExpressionNode... mappablesNodes) {
+        this.mappables = mappablesNodes;
+    }
+
+    @ExplodeLoop
+    @Specialization
+    public Object concat(VirtualFrame frame,
+                    @Cached("createBinaryProfile()") ConditionProfile hasFrame,
+                    @Cached GetDictStorageNode getStore,
+                    @CachedLibrary(limit = "2") HashingStorageLibrary firstlib,
+                    @CachedLibrary(limit = "1") HashingStorageLibrary otherlib,
+                    @Cached("createBinaryProfile()") ConditionProfile sameKeyProfile) {
+        PDict first = null;
+        PDict other;
+        for (ExpressionNode n : mappables) {
+            if (first == null) {
+                first = expectDict(n.execute(frame));
+            } else {
+                other = expectDict(n.execute(frame));
+                addAllToDict(frame, first, other, hasFrame, firstlib, otherlib, getStore, sameKeyProfile);
+            }
+        }
+        return first;
+    }
+
+    private static void addAllToDict(VirtualFrame frame, PDict dict, PDict other, ConditionProfile hasFrame,
+                    HashingStorageLibrary firstlib, HashingStorageLibrary otherlib, GetDictStorageNode getStore, ConditionProfile sameKeyProfile) {
+        HashingStorage dictStorage = getStore.execute(dict);
+        HashingStorage otherStorage = getStore.execute(other);
+        for (Object key : otherlib.keys(otherStorage)) {
+            Object value = otherlib.getItemWithFrame(otherStorage, key, hasFrame, frame);
+            if (sameKeyProfile.profile(firstlib.hasKey(dictStorage, key))) {
+                throw createSameDictKeyException(key);
+            }
+            dictStorage = firstlib.setItemWithFrame(dictStorage, key, value, hasFrame, frame);
+        }
+        dict.setDictStorage(dictStorage);
+    }
+
+    @CompilerDirectives.TruffleBoundary
+    private static SameDictKeyException createSameDictKeyException(Object key) {
+        return new SameDictKeyException(key.toString());
+    }
+
+    private static PDict expectDict(Object first) {
+        if (!(first instanceof PDict)) {
+            CompilerDirectives.transferToInterpreter();
+            throw new NonMappingException(first);
+        }
+        return (PDict) first;
+    }
+
+}
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/ExecuteKeywordStarargsNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/ExecuteKeywordStarargsNode.java
@@ -40,6 +40,7 @@
  */
 package com.oracle.graal.python.nodes.argument.keywords;
 
+import com.oracle.graal.python.builtins.objects.PNone;
 import com.oracle.graal.python.builtins.objects.common.EmptyStorage;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
 import com.oracle.graal.python.builtins.objects.common.KeywordsStorage;
@@ -110,7 +111,10 @@ static PKeyword[] doDict(PDict starargs,
 
         @Specialization(guards = "!isDict(object)")
         static PKeyword[] doNonMapping(@SuppressWarnings("unused") Object object) {
-            return PKeyword.EMPTY_KEYWORDS;
+            if (object instanceof PNone) {
+                return PKeyword.EMPTY_KEYWORDS;
+            }
+            throw new NonMappingException(object);
         }
 
         @Specialization(replaces = {"doKeywordsArray", "doKeywordsStorage", "doEmptyStorage", "doDictCached", "doDict", "doNonMapping"})
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/NonMappingException.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/NonMappingException.java
@@ -0,0 +1,59 @@
+/*
+ * 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.nodes.argument.keywords;
+
+import com.oracle.truffle.api.nodes.ControlFlowException;
+
+public class NonMappingException extends ControlFlowException {
+    private static final long serialVersionUID = 1L;
+
+    private final Object object;
+
+    public NonMappingException(Object object) {
+        super();
+        this.object = object;
+    }
+
+    public Object getObject() {
+        return object;
+    }
+
+}
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/SameDictKeyException.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/SameDictKeyException.java
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/PythonCallNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/PythonCallNode.java
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/ArgListBuilder.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/ArgListBuilder.java
