Skip to content

Commit 2e8cad9

Browse files
msimacekmsimacek
committed
Fix repr/str of recursive foreign arrays
Fixes #402
1 parent 829f147 commit 2e8cad9

File tree

2 files changed

+89
-42
lines changed

2 files changed

+89
-42
lines changed

‎graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/interop/JavaInteropTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@
5050
import java.io.IOException;
5151
import java.io.UnsupportedEncodingException;
5252
import java.math.BigInteger;
53+
import java.util.ArrayList;
5354
import java.util.Arrays;
5455
import java.util.HashMap;
56+
import java.util.List;
5557
import java.util.Map;
5658

5759
import org.graalvm.polyglot.Context;
@@ -772,6 +774,21 @@ public void removeUnsupportedArrayElement() throws IOException {
772774
Value foo = context.eval(suitePy);
773775
foo.execute(new UnsupportedProxyArray());
774776
}
777+
778+
@Test
779+
public void recursiveJavaListRepr() throws IOException {
780+
Source source = Source.newBuilder("python", """
781+
def foo(obj):
782+
return repr(obj)
783+
foo
784+
""", "input").build();
785+
Value foo = context.eval(source);
786+
List<Object> recursiveList = new ArrayList<>();
787+
recursiveList.add(1);
788+
recursiveList.add(recursiveList);
789+
Value result = foo.execute(recursiveList);
790+
assertEquals(result.as(String.class), "[1, [...]]");
791+
}
775792
}
776793

777794
@RunWith(Parameterized.class)

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

Lines changed: 72 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@
7272
import static com.oracle.graal.python.nodes.SpecialMethodNames.T___INSTANCECHECK__;
7373
import static com.oracle.graal.python.nodes.SpecialMethodNames.T___LEN__;
7474
import static com.oracle.graal.python.nodes.SpecialMethodNames.T___NEXT__;
75+
import static com.oracle.graal.python.nodes.StringLiterals.T_COMMA_SPACE;
76+
import static com.oracle.graal.python.nodes.StringLiterals.T_ELLIPSIS_IN_BRACKETS;
77+
import static com.oracle.graal.python.nodes.StringLiterals.T_EMPTY_BRACKETS;
78+
import static com.oracle.graal.python.nodes.StringLiterals.T_LBRACKET;
79+
import static com.oracle.graal.python.nodes.StringLiterals.T_NONE;
80+
import static com.oracle.graal.python.nodes.StringLiterals.T_RBRACKET;
7581
import static com.oracle.graal.python.util.PythonUtils.TS_ENCODING;
7682

7783
import java.math.BigInteger;
@@ -90,11 +96,9 @@
9096
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
9197
import com.oracle.graal.python.builtins.objects.function.PKeyword;
9298
import com.oracle.graal.python.builtins.objects.ints.PInt;
93-
import com.oracle.graal.python.builtins.objects.iterator.PForeignArrayIterator;
9499
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltins;
95100
import com.oracle.graal.python.builtins.objects.object.ObjectNodes;
96101
import com.oracle.graal.python.builtins.objects.str.StringBuiltins;
97-
import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
98102
import com.oracle.graal.python.builtins.objects.type.TpSlots;
99103
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotBinaryFunc.MpSubscriptBuiltinNode;
100104
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotGetAttr.GetAttrBuiltinNode;
@@ -103,18 +107,18 @@
103107
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotSetAttr.SetAttrBuiltinNode;
104108
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotSizeArgFun.SqItemBuiltinNode;
105109
import com.oracle.graal.python.lib.PyNumberAsSizeNode;
110+
import com.oracle.graal.python.lib.PyObjectReprAsTruffleStringNode;
106111
import com.oracle.graal.python.lib.PyObjectRichCompareBool;
112+
import com.oracle.graal.python.lib.PyObjectStrAsTruffleStringNode;
107113
import com.oracle.graal.python.nodes.ErrorMessages;
108114
import com.oracle.graal.python.nodes.PGuards;
109115
import com.oracle.graal.python.nodes.PRaiseNode;
110-
import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode;
111116
import com.oracle.graal.python.nodes.expression.BinaryArithmetic;
112117
import com.oracle.graal.python.nodes.expression.BinaryArithmetic.BitAndNode;
113118
import com.oracle.graal.python.nodes.expression.BinaryArithmetic.BitOrNode;
114119
import com.oracle.graal.python.nodes.expression.BinaryArithmetic.BitXorNode;
115120
import com.oracle.graal.python.nodes.expression.BinaryComparisonNode;
116121
import com.oracle.graal.python.nodes.expression.BinaryOpNode;
117-
import com.oracle.graal.python.nodes.expression.CastToListExpressionNode.CastToListNode;
118122
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
119123
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
120124
import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
@@ -162,6 +166,7 @@
162166
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
163167
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
164168
import com.oracle.truffle.api.strings.TruffleString;
169+
import com.oracle.truffle.api.strings.TruffleStringBuilder;
165170

166171
@CoreFunctions(extendClasses = PythonBuiltinClassType.ForeignObject)
167172
public final class ForeignObjectBuiltins extends PythonBuiltins {
@@ -1190,27 +1195,25 @@ protected static Object doIt(Object object,
11901195
@Builtin(name = J___STR__, minNumOfPositionalArgs = 1)
11911196
@GenerateNodeFactory
11921197
abstract static class StrNode extends PythonUnaryBuiltinNode {
1193-
@Child private LookupAndCallUnaryNode callStrNode;
1194-
@Child private CastToListNode castToListNode;
11951198
@Child private TruffleString.SwitchEncodingNode switchEncodingNode;
11961199

11971200
@Specialization
11981201
Object str(VirtualFrame frame, Object object,
11991202
@Bind("this") Node inliningTarget,
12001203
@CachedLibrary(limit = "3") InteropLibrary lib,
12011204
@Cached GilNode gil,
1205+
@Cached PyObjectStrAsTruffleStringNode strNode,
1206+
@Cached StrForeignArrayNode strForeignArrayNode,
12021207
@Cached InlinedBranchProfile isNull,
12031208
@Cached InlinedBranchProfile isBoolean,
12041209
@Cached InlinedBranchProfile isString,
12051210
@Cached InlinedBranchProfile isLong,
12061211
@Cached InlinedBranchProfile isDouble,
1207-
@Cached InlinedBranchProfile isArray,
1208-
@Cached InlinedBranchProfile defaultCase,
1209-
@Cached PythonObjectFactory.Lazy factory) {
1212+
@Cached InlinedBranchProfile defaultCase) {
12101213
try {
12111214
if (lib.isNull(object)) {
12121215
isNull.enter(inliningTarget);
1213-
return getCallStrNode().executeObject(frame, PNone.NONE);
1216+
return T_NONE;
12141217
} else if (lib.isBoolean(object)) {
12151218
isBoolean.enter(inliningTarget);
12161219
boolean value;
@@ -1220,7 +1223,7 @@ Object str(VirtualFrame frame, Object object,
12201223
} finally {
12211224
gil.acquire();
12221225
}
1223-
return getCallStrNode().executeObject(frame, value);
1226+
return strNode.execute(frame, inliningTarget, value);
12241227
} else if (lib.isString(object)) {
12251228
isString.enter(inliningTarget);
12261229
TruffleString value;
@@ -1230,7 +1233,7 @@ Object str(VirtualFrame frame, Object object,
12301233
} finally {
12311234
gil.acquire();
12321235
}
1233-
return getCallStrNode().executeObject(frame, getSwitchEncodingNode().execute(value, TS_ENCODING));
1236+
return strNode.execute(frame, inliningTarget, getSwitchEncodingNode().execute(value, TS_ENCODING));
12341237
} else if (lib.fitsInLong(object)) {
12351238
isLong.enter(inliningTarget);
12361239
long value;
@@ -1240,7 +1243,7 @@ Object str(VirtualFrame frame, Object object,
12401243
} finally {
12411244
gil.acquire();
12421245
}
1243-
return getCallStrNode().executeObject(frame, value);
1246+
return strNode.execute(frame, inliningTarget, value);
12441247
} else if (lib.fitsInDouble(object)) {
12451248
isDouble.enter(inliningTarget);
12461249
double value;
@@ -1250,20 +1253,9 @@ Object str(VirtualFrame frame, Object object,
12501253
} finally {
12511254
gil.acquire();
12521255
}
1253-
return getCallStrNode().executeObject(frame, value);
1256+
return strNode.execute(frame, inliningTarget, value);
12541257
} else if (lib.hasArrayElements(object)) {
1255-
isArray.enter(inliningTarget);
1256-
long size;
1257-
gil.release(true);
1258-
try {
1259-
size = lib.getArraySize(object);
1260-
} finally {
1261-
gil.acquire();
1262-
}
1263-
if (size <= Integer.MAX_VALUE && size >= 0) {
1264-
PForeignArrayIterator iterable = factory.get(inliningTarget).createForeignArrayIterator(object);
1265-
return getCallStrNode().executeObject(frame, getCastToListNode().execute(frame, iterable));
1266-
}
1258+
return strForeignArrayNode.execute(frame, object, lib);
12671259
}
12681260
} catch (UnsupportedMessageException e) {
12691261
// Fall back to the generic impl
@@ -1272,22 +1264,6 @@ Object str(VirtualFrame frame, Object object,
12721264
return defaultConversion(frame, lib, object);
12731265
}
12741266

1275-
private LookupAndCallUnaryNode getCallStrNode() {
1276-
if (callStrNode == null) {
1277-
CompilerDirectives.transferToInterpreterAndInvalidate();
1278-
callStrNode = insert(LookupAndCallUnaryNode.create(SpecialMethodSlot.Str));
1279-
}
1280-
return callStrNode;
1281-
}
1282-
1283-
private CastToListNode getCastToListNode() {
1284-
if (castToListNode == null) {
1285-
CompilerDirectives.transferToInterpreterAndInvalidate();
1286-
castToListNode = insert(CastToListNode.create());
1287-
}
1288-
return castToListNode;
1289-
}
1290-
12911267
protected TruffleString.SwitchEncodingNode getSwitchEncodingNode() {
12921268
if (switchEncodingNode == null) {
12931269
CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -1305,6 +1281,60 @@ protected TruffleString defaultConversion(@SuppressWarnings("unused") VirtualFra
13051281
}
13061282
}
13071283

1284+
@GenerateInline(false) // Uncommon path
1285+
abstract static class StrForeignArrayNode extends Node {
1286+
abstract TruffleString execute(VirtualFrame frame, Object object, InteropLibrary lib) throws UnsupportedMessageException;
1287+
1288+
@Specialization
1289+
static TruffleString str(VirtualFrame frame, Object object, InteropLibrary lib,
1290+
@Bind("this") Node inliningTarget,
1291+
@Cached GilNode gil,
1292+
@Cached PyObjectReprAsTruffleStringNode reprNode,
1293+
@Cached TruffleStringBuilder.AppendStringNode appendStringNode,
1294+
@Cached TruffleStringBuilder.ToStringNode toStringNode) throws UnsupportedMessageException {
1295+
if (!PythonContext.get(inliningTarget).reprEnter(object)) {
1296+
return T_ELLIPSIS_IN_BRACKETS;
1297+
}
1298+
try {
1299+
long length;
1300+
gil.release(true);
1301+
try {
1302+
length = lib.getArraySize(object);
1303+
} finally {
1304+
gil.acquire();
1305+
}
1306+
if (length == 0) {
1307+
return T_EMPTY_BRACKETS;
1308+
}
1309+
TruffleStringBuilder buf = TruffleStringBuilder.create(TS_ENCODING);
1310+
appendStringNode.execute(buf, T_LBRACKET);
1311+
boolean initial = true;
1312+
for (int index = 0; index < length; index++) {
1313+
if (initial) {
1314+
initial = false;
1315+
} else {
1316+
appendStringNode.execute(buf, T_COMMA_SPACE);
1317+
}
1318+
Object value;
1319+
gil.release(true);
1320+
try {
1321+
value = lib.readArrayElement(object, index);
1322+
} catch (InvalidArrayIndexException e) {
1323+
// Concurrent modification?
1324+
break;
1325+
} finally {
1326+
gil.acquire();
1327+
}
1328+
appendStringNode.execute(buf, reprNode.execute(frame, inliningTarget, value));
1329+
}
1330+
appendStringNode.execute(buf, T_RBRACKET);
1331+
return toStringNode.execute(buf);
1332+
} finally {
1333+
PythonContext.get(inliningTarget).reprLeave(object);
1334+
}
1335+
}
1336+
}
1337+
13081338
@Builtin(name = J___REPR__, minNumOfPositionalArgs = 1)
13091339
@GenerateNodeFactory
13101340
abstract static class ReprNode extends StrNode {

0 commit comments

Comments
 (0)