[GR-35631] Make __str__ call toDisplayString for foreign objects

PullRequest: graalpython/2068

Author: msimacek

CHANGELOG.md

@@ -8,6 +8,7 @@ language runtime. The main focus is on user-observable behavior of the engine.
 * Added partial support for `PYTHONHASHSEED` environment variable (also available via `HashSeed` context option), currently only affecting hashing in `pyexpat` module.
 * Implement `_csv` module.
 * Improved compatibility with PyPI packages `wheel` and `click`
+* String conversion (`__str__`) now calls `toString` for Java objects and `toDisplayString` interop message for foreign objects.
 
 ## Version 21.3.0
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/foreign/ForeignObjectBuiltins.java

@@ -1030,7 +1030,6 @@ protected static long doIt(Object object,
     abstract static class StrNode extends PythonUnaryBuiltinNode {
         @Child private LookupAndCallUnaryNode callStrNode;
         @Child private CastToListNode castToListNode;
-        @Child private ObjectNodes.DefaultObjectReprNode defaultReprNode;
 
         @Specialization
         Object str(VirtualFrame frame, Object object,
@@ -1042,7 +1041,7 @@ Object str(VirtualFrame frame, Object object,
                         @Cached BranchProfile isLong,
                         @Cached BranchProfile isDouble,
                         @Cached BranchProfile isArray,
-                        @Cached BranchProfile isHostObject) {
+                        @Cached BranchProfile defaultCase) {
             try {
                 if (lib.isNull(object)) {
                     isNull.enter();
@@ -1100,31 +1099,12 @@ Object str(VirtualFrame frame, Object object,
                         PForeignArrayIterator iterable = factory().createForeignArrayIterator(object);
                         return getCallStrNode().executeObject(frame, getCastToListNode().execute(frame, iterable));
                     }
-                } else if (getContext().getEnv().isHostObject(object)) {
-                    isHostObject.enter();
-                    boolean isMetaObject = lib.isMetaObject(object);
-                    Object metaObject = isMetaObject
-                                    ? object
-                                    : lib.hasMetaObject(object) ? lib.getMetaObject(object) : null;
-                    if (metaObject != null) {
-                        Object displayName = lib.toDisplayString(metaObject);
-                        String text = createDisplayName(isMetaObject, displayName);
-                        return PythonUtils.format("<%s at 0x%x>", text, PythonAbstractObject.systemHashCode(object));
-                    }
                 }
             } catch (UnsupportedMessageException e) {
                 // Fall back to the generic impl
             }
-            return defaultRepr(frame, object);
-        }
-
-        @TruffleBoundary
-        private static String createDisplayName(boolean isMetaObject, Object object) {
-            StringBuilder sb = new StringBuilder();
-            sb.append(isMetaObject ? "JavaClass[" : "JavaObject[");
-            sb.append(object.toString());
-            sb.append("]");
-            return sb.toString();
+            defaultCase.enter();
+            return defaultConversion(frame, lib, object);
         }
 
         private LookupAndCallUnaryNode getCallStrNode() {
@@ -1143,19 +1123,50 @@ private CastToListNode getCastToListNode() {
             return castToListNode;
         }
 
-        protected String defaultRepr(VirtualFrame frame, Object object) {
-            if (defaultReprNode == null) {
-                CompilerDirectives.transferToInterpreterAndInvalidate();
-                defaultReprNode = insert(ObjectNodes.DefaultObjectReprNode.create());
+        protected String defaultConversion(@SuppressWarnings("unused") VirtualFrame frame, InteropLibrary lib, Object object) {
+            try {
+                return lib.asString(lib.toDisplayString(object));
+            } catch (UnsupportedMessageException e) {
+                throw CompilerDirectives.shouldNotReachHere("toDisplayString result not convertible to String");
             }
-            return defaultReprNode.execute(frame, object);
         }
     }
 
     @Builtin(name = __REPR__, minNumOfPositionalArgs = 1)
     @GenerateNodeFactory
     abstract static class ReprNode extends StrNode {
-        protected final String method = __REPR__;
+        @Child private ObjectNodes.DefaultObjectReprNode defaultReprNode;
+
+        protected String defaultConversion(VirtualFrame frame, @SuppressWarnings("unused") InteropLibrary lib, Object object) {
+            try {
+                if (getContext().getEnv().isHostObject(object)) {
+                    boolean isMetaObject = lib.isMetaObject(object);
+                    Object metaObject = null;
+                    if (isMetaObject) {
+                        metaObject = object;
+                    } else if (lib.hasMetaObject(object)) {
+                        metaObject = lib.getMetaObject(object);
+                    }
+                    if (metaObject != null) {
+                        Object displayName = lib.toDisplayString(metaObject);
+                        String text = createDisplayName(isMetaObject, displayName);
+                        return PythonUtils.format("<%s at 0x%x>", text, PythonAbstractObject.systemHashCode(object));
+                    }
+                }
+            } catch (UnsupportedMessageException e) {
+                // fallthrough to default
+            }
+            if (defaultReprNode == null) {
+                CompilerDirectives.transferToInterpreterAndInvalidate();
+                defaultReprNode = insert(ObjectNodes.DefaultObjectReprNode.create());
+            }
+            return defaultReprNode.execute(frame, object);
+        }
+
+        @TruffleBoundary
+        private static String createDisplayName(boolean isMetaObject, Object object) {
+            return (isMetaObject ? "JavaClass[" : "JavaObject[") + object + "]";
+        }
     }
 
     @Builtin(name = __BASES__, minNumOfPositionalArgs = 1, isGetter = true, isSetter = false)

graalpython/lib-python/3/test/test_subprocess.py

@@ -1592,7 +1592,8 @@ def test_run_with_shell_timeout_and_capture_output(self):
             stacks = traceback.format_exc()  # assertRaises doesn't give this.
         else:
             self.fail("TimeoutExpired not raised.")
-        self.assertLess(after_secs - before_secs, 1.5,
+        # GraalVM change: increase the timing tolerance
+        self.assertLess(after_secs - before_secs, 3.0,
                         msg="TimeoutExpired was delayed! Bad traceback:\n```\n"
                         f"{stacks}```")