[GR-10825] fix dict.setdefault

PullRequest: graalpython/116

Author: cosminbasca

graalpython/com.oracle.graal.python.test/src/tests/test_dict.py

@@ -343,3 +343,33 @@ def test_dictview_mixed_set_operations():
     assert {(1, 1)} == {1: 1}.items()
     assert {1: 1}.items() | {2} == {(1, 1), 2}
     assert {2} | {1: 1}.items() == {(1, 1), 2}
+
+
+def test_setdefault():
+    # dict.setdefault()
+    d = {}
+    assert d.setdefault('key0') is None
+    d.setdefault('key0', [])
+    assert d.setdefault('key0') is None
+    d.setdefault('key', []).append(3)
+    assert d['key'][0] == 3
+    d.setdefault('key', []).append(4)
+    assert len(d['key']) == 2
+    assert_raises(TypeError, d.setdefault)
+
+    class Exc(Exception):
+        pass
+
+    class BadHash(object):
+        fail = False
+
+        def __hash__(self):
+            if self.fail:
+                raise Exc()
+            else:
+                return 42
+
+    x = BadHash()
+    d[x] = 42
+    x.fail = True
+    assert_raises(Exc, d.setdefault, x, [])

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/HashingStorageNodes.java

@@ -78,6 +78,7 @@
 import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode;
 import com.oracle.graal.python.nodes.control.GetIteratorNode;
 import com.oracle.graal.python.nodes.control.GetNextNode;
+import com.oracle.graal.python.nodes.datamodel.IsHashableNode;
 import com.oracle.graal.python.nodes.expression.BinaryComparisonNode;
 import com.oracle.graal.python.nodes.object.GetClassNode;
 import com.oracle.graal.python.runtime.exception.PException;
@@ -170,7 +171,7 @@ public static PythonEquivalence create() {
     @ImportStatic(PGuards.class)
     abstract static class DictStorageBaseNode extends PBaseNode {
         @Child private GetClassNode getClassNode;
-        @Child private LookupAndCallUnaryNode lookupHashAttributeNode;
+        @Child private IsHashableNode isHashableNode;
         @Child private Equivalence equivalenceNode;
 
         protected Equivalence getEquivalence() {
@@ -189,8 +190,6 @@ protected PythonClass getClass(Object object) {
             return getClassNode.execute(object);
         }
 
-        private final ValueProfile keyTypeProfile = ValueProfile.createClassProfile();
-
         protected boolean isJavaString(Object o) {
             return o instanceof String || o instanceof PString && wrappedString((PString) o);
         }
@@ -205,27 +204,11 @@ protected static Shape lookupShape(DynamicObject receiver) {
         }
 
         protected boolean isHashable(Object key) {
-            Object profiledKey = keyTypeProfile.profile(key);
-            if (PGuards.isString(profiledKey)) {
-                return true;
-            } else if (PGuards.isInteger(profiledKey)) {
-                return true;
-            } else if (profiledKey instanceof Double || PGuards.isPFloat(profiledKey)) {
-                return true;
-            }
-
-            if (lookupHashAttributeNode == null) {
+            if (isHashableNode == null) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
-                lookupHashAttributeNode = insert(LookupAndCallUnaryNode.create(__HASH__));
+                isHashableNode = insert(IsHashableNode.create());
             }
-
-            try {
-                lookupHashAttributeNode.executeObject(key);
-                return true;
-            } catch (PException e) {
-                // ignore
-            }
-            return false;
+            return isHashableNode.execute(key);
         }
 
         protected PException unhashable(Object key) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictBuiltins.java

@@ -68,6 +68,7 @@
 import com.oracle.truffle.api.dsl.Fallback;
 import com.oracle.truffle.api.dsl.GenerateNodeFactory;
 import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.profiles.ConditionProfile;
 
 @CoreFunctions(extendClasses = PDict.class)
 public final class DictBuiltins extends PythonBuiltins {
@@ -110,7 +111,7 @@ public Object doGeneric(@SuppressWarnings("unused") PDict self, Object[] args, @
     }
 
     // setdefault(key[, default])
-    @Builtin(name = "setdefault", fixedNumOfArguments = 3)
+    @Builtin(name = "setdefault", minNumOfArguments = 2, keywordArguments = {"default"})
     @GenerateNodeFactory
     public abstract static class SetDefaultNode extends PythonBuiltinNode {
         @Child private HashingStorageNodes.ContainsKeyNode containsKeyNode;
@@ -131,10 +132,14 @@ public Object setDefault(PDict dict, Object key, @SuppressWarnings("unused") Obj
 
         @Specialization(guards = "!containsKey(dict.getDictStorage(), key)")
         public Object setDefault(PDict dict, Object key, Object defaultValue,
-                        @Cached("create()") HashingStorageNodes.SetItemNode setItemNode) {
-
-            setItemNode.execute(dict, dict.getDictStorage(), key, defaultValue);
-            return defaultValue;
+                        @Cached("create()") HashingStorageNodes.SetItemNode setItemNode,
+                        @Cached("createBinaryProfile()") ConditionProfile defaultValProfile) {
+            Object value = defaultValue;
+            if (defaultValProfile.profile(defaultValue == PNone.NO_VALUE)) {
+                value = PNone.NONE;
+            }
+            setItemNode.execute(dict, dict.getDictStorage(), key, value);
+            return value;
         }
     }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/datamodel/IsHashableNode.java

@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates.
+ *
+ * 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.datamodel;
+
+import com.oracle.graal.python.builtins.PythonBuiltinClassType;
+import com.oracle.graal.python.builtins.modules.BuiltinFunctions;
+import com.oracle.graal.python.builtins.objects.type.PythonClass;
+import com.oracle.graal.python.nodes.PGuards;
+import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode;
+import com.oracle.graal.python.runtime.exception.PythonErrorType;
+import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.Specialization;
+
+public abstract class IsHashableNode extends PDataModelEmulationNode {
+    protected PythonClass getBuiltinIntType() {
+        return getCore().lookupType(PythonBuiltinClassType.PInt);
+    }
+
+    protected boolean isDouble(Object object) {
+        return object instanceof Double || PGuards.isPFloat(object);
+    }
+
+    @Specialization(guards = "isString(object)")
+    protected boolean isHashableString(@SuppressWarnings("unused") Object object) {
+        return true;
+    }
+
+    @Specialization(guards = "isInteger(object)")
+    protected boolean isHashableInt(@SuppressWarnings("unused") Object object) {
+        return true;
+    }
+
+    @Specialization(guards = "isDouble(object)")
+    protected boolean isHashableGeneric(@SuppressWarnings("unused") Object object) {
+        return true;
+    }
+
+    @Specialization
+    protected boolean isHashableGeneric(Object object,
+                    @Cached("create(__HASH__)") LookupAndCallUnaryNode lookupHashAttributeNode,
+                    @Cached("create()") BuiltinFunctions.IsInstanceNode isInstanceNode,
+                    @Cached("getBuiltinIntType()") PythonClass IntType) {
+        Object hashValue = lookupHashAttributeNode.executeObject(object);
+        if (isInstanceNode.executeWith(hashValue, IntType)) {
+            return true;
+        }
+        throw raise(PythonErrorType.TypeError, "__hash__ method should return an integer");
+    }
+
+    public static IsHashableNode create() {
+        return IsHashableNodeGen.create();
+    }
+}