Fix PyUnicode_InternInPlace

Author: msimacek

graalpython/com.oracle.graal.python.cext/src/unicodeobject.c

@@ -819,10 +819,9 @@ PyUnicode_InternInPlace(PyObject **p)
     }
 
     if (t != s) {
-        Py_INCREF(t);
-        Py_SETREF(*p, t);
-        return;
+        *p = t;
     }
+    Py_DECREF(s);
 }
 
 // 15940

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_unicode.py

@@ -1030,18 +1030,21 @@ def compile_module(self, name):
         cmpfunc=unhandled_error_compare
     )
 
+
 class TestUnicodeObject(object):
     def test_intern(self):
         TestIntern = CPyExtType(
             "TestIntern",
             '''
             static PyObject* set_intern_str(PyObject* self, PyObject* str) {
+                Py_INCREF(str);
                 PyUnicode_InternInPlace(&str);
                 ((TestInternObject*)self)->str = str;
                 return str;
             }
 
             static PyObject* check_is_same_str_ptr(PyObject* self, PyObject* str) {
+                Py_INCREF(str);
                 PyUnicode_InternInPlace(&str);
                 if (str == ((TestInternObject*)self)->str) {
                     Py_RETURN_TRUE;
@@ -1057,6 +1060,7 @@ def test_intern(self):
             ''',
         )
         tester = TestIntern()
-        s = 'some text'
-        assert tester.set_intern_str(s) == s
-        assert tester.check_is_same_str_ptr(s)
+        s1 = b'some text'.decode('ascii')
+        s2 = b'some text'.decode('ascii')
+        assert tester.set_intern_str(s1) == s2
+        assert tester.check_is_same_str_ptr(s2)

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextUnicodeBuiltins.java

@@ -132,7 +132,6 @@
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.StringLiterals;
-import com.oracle.graal.python.nodes.attributes.ReadAttributeFromDynamicObjectNode;
 import com.oracle.graal.python.nodes.call.CallNode;
 import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
 import com.oracle.graal.python.nodes.object.GetClassNode;
@@ -298,45 +297,42 @@ static Object doGeneric(Object obj, Object encodingObj, Object errorsObj,
         }
     }
 
-    @CApiBuiltin(ret = PyObject, args = {PyObject}, call = Ignored)
+    @CApiBuiltin(ret = PyObjectTransfer, args = {PyObject}, call = Ignored)
     abstract static class PyTruffleUnicode_LookupAndIntern extends CApiUnaryBuiltinNode {
-        @Specialization
-        static Object withTS(TruffleString str,
-                        @Bind("this") Node inliningTarget,
-                        @Exclusive @Cached StringNodes.InternStringNode internNode,
-                        @Exclusive @Cached HashingStorageGetItem getItem,
-                        @Exclusive @Cached HashingStorageSetItem setItem,
-                        @Exclusive @Cached PythonObjectFactory.Lazy factory) {
-            PDict dict = getCApiContext(inliningTarget).getInternedUnicode();
-            if (dict == null) {
-                dict = factory.get(inliningTarget).createDict();
-                getCApiContext(inliningTarget).setInternedUnicode(dict);
-            }
-            Object interned = getItem.execute(inliningTarget, dict.getDictStorage(), str);
-            if (interned == null) {
-                interned = internNode.execute(inliningTarget, str);
-                dict.setDictStorage(setItem.execute(inliningTarget, dict.getDictStorage(), str, interned));
-            }
-            return interned;
-        }
 
         @Specialization
         static Object withPString(PString str,
                         @Bind("this") Node inliningTarget,
                         @Cached PyUnicodeCheckExactNode unicodeCheckExactNode,
-                        @Cached ReadAttributeFromDynamicObjectNode readNode,
-                        @Exclusive @Cached StringNodes.InternStringNode internNode,
-                        @Exclusive @Cached HashingStorageGetItem getItem,
-                        @Exclusive @Cached HashingStorageSetItem setItem,
-                        @Exclusive @Cached PythonObjectFactory.Lazy factory) {
+                        @Cached CastToTruffleStringNode cast,
+                        @Cached StringNodes.IsInternedStringNode isInternedStringNode,
+                        @Cached StringNodes.InternStringNode internNode,
+                        @Cached HashingStorageGetItem getItem,
+                        @Cached HashingStorageSetItem setItem,
+                        @Cached PythonObjectFactory.Lazy factory) {
             if (!unicodeCheckExactNode.execute(inliningTarget, str)) {
                 return getNativeNull(inliningTarget);
             }
-            boolean isInterned = readNode.execute(str, PString.INTERNED) != PNone.NO_VALUE;
+            /*
+             * TODO this is not integrated with str.intern, pointer comparisons of two str.intern'ed
+             * string may still yield failse
+             */
+            boolean isInterned = isInternedStringNode.execute(inliningTarget, str);
             if (isInterned) {
                 return str;
             }
-            return withTS(str.getValueUncached(), inliningTarget, internNode, getItem, setItem, factory);
+            TruffleString ts = cast.execute(inliningTarget, str);
+            PDict dict = getCApiContext(inliningTarget).getInternedUnicode();
+            if (dict == null) {
+                dict = factory.get(inliningTarget).createDict();
+                getCApiContext(inliningTarget).setInternedUnicode(dict);
+            }
+            Object interned = getItem.execute(inliningTarget, dict.getDictStorage(), ts);
+            if (interned == null) {
+                interned = internNode.execute(inliningTarget, str);
+                dict.setDictStorage(setItem.execute(inliningTarget, dict.getDictStorage(), ts, interned));
+            }
+            return interned;
         }
 
         @Fallback