Skip to content

Commit e9dd540

Browse files
qunaibitqunaibit
committed
Simplify dict.__repr__
1 parent bdf82de commit e9dd540

File tree

5 files changed

+48
-52
lines changed

5 files changed

+48
-52
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/DynamicObjectNativeWrapper.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -956,8 +956,10 @@ private SubclassAddState(HashingStorage storage, HashingStorageLibrary lib, Set<
956956
}
957957
}
958958

959+
@GenerateUncached
959960
static final class EachSubclassAdd extends HashingStorageLibrary.ForEachNode<SubclassAddState> {
960-
private static final EachSubclassAdd SINGLETON = new EachSubclassAdd();
961+
962+
private static final EachSubclassAdd UNCACHED = new EachSubclassAdd();
961963

962964
@Override
963965
public SubclassAddState execute(Object key, SubclassAddState s) {
@@ -971,11 +973,11 @@ protected static void setAdd(Set<PythonAbstractClass> set, PythonClass cls) {
971973
}
972974

973975
static EachSubclassAdd create() {
974-
return SINGLETON;
976+
return new EachSubclassAdd();
975977
}
976978

977979
static EachSubclassAdd getUncached() {
978-
return SINGLETON;
980+
return UNCACHED;
979981
}
980982
}
981983

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

Lines changed: 38 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@
7171
import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
7272
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
7373
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
74-
import com.oracle.graal.python.nodes.util.CastToJavaStringNodeGen;
7574
import com.oracle.graal.python.runtime.PythonCore;
7675
import com.oracle.graal.python.runtime.exception.PythonErrorType;
7776
import com.oracle.truffle.api.CompilerDirectives;
@@ -504,82 +503,77 @@ public PDictView values(PDict self) {
504503
@GenerateNodeFactory
505504
public abstract static class ReprNode extends PythonUnaryBuiltinNode {
506505
@ValueType
507-
private static final class ReprState {
508-
private final PDict self;
506+
protected static final class ReprState {
509507
private final HashingStorage dictStorage;
510508
private final StringBuilder result;
511509

512-
ReprState(PDict self, HashingStorage dictStorage, StringBuilder result) {
513-
this.self = self;
510+
ReprState(HashingStorage dictStorage, StringBuilder result) {
514511
this.dictStorage = dictStorage;
515512
this.result = result;
516513
}
517514
}
518515

519-
static final class EachRepr extends HashingStorageLibrary.ForEachNode<ReprState> {
520-
@Child LookupAndCallUnaryDynamicNode reprKeyNode;
521-
@Child LookupAndCallUnaryDynamicNode reprValueNode;
522-
@Child CastToJavaStringNode castStr;
523-
@Child PRaiseNode raiseNode;
524-
@Child HashingStorageLibrary lib;
516+
abstract static class EachRepr extends HashingStorageLibrary.ForEachNode<ReprState> {
525517
private final int limit;
526518

527519
EachRepr(int limit) {
528520
this.limit = limit;
529521
}
530522

531-
static final EachRepr create(int limit) {
532-
return new EachRepr(limit);
523+
protected final int getLimit() {
524+
return limit;
533525
}
534526

527+
public abstract ReprState executeReprState(Object key, ReprState arg);
528+
535529
@Override
536-
public ReprState execute(Object key, ReprState s) {
537-
if (lib == null) {
538-
lib = insert(HashingStorageLibrary.getFactory().createDispatched(limit));
539-
}
540-
Object value = lib.getItem(s.dictStorage, key);
541-
if (reprKeyNode == null) {
542-
reprKeyNode = insert(LookupAndCallUnaryDynamicNode.create());
543-
}
530+
public final ReprState execute(Object key, ReprState arg) {
531+
return executeReprState(key, arg);
532+
}
533+
534+
@Specialization
535+
public ReprState dict(Object key, ReprState s,
536+
@Cached LookupAndCallUnaryDynamicNode reprKeyNode,
537+
@Cached LookupAndCallUnaryDynamicNode reprValueNode,
538+
@Cached CastToJavaStringNode castStr,
539+
@Cached PRaiseNode raiseNode,
540+
@Cached("createBinaryProfile()") ConditionProfile lengthCheck,
541+
@Cached("createBinaryProfile()") ConditionProfile nullKeyCheck,
542+
@Cached("createBinaryProfile()") ConditionProfile nullValueCheck,
543+
@CachedLibrary(limit = "getLimit()") HashingStorageLibrary lib) {
544544
Object keyRepr = reprKeyNode.executeObject(key, __REPR__);
545-
if (reprValueNode == null) {
546-
reprValueNode = insert(LookupAndCallUnaryDynamicNode.create());
547-
}
548-
Object valueRepr = value != s.self ? reprValueNode.executeObject(value, __REPR__) : "{...}";
549-
if (castStr == null) {
550-
castStr = insert(CastToJavaStringNodeGen.create());
551-
}
552545
String keyReprString = castStr.execute(keyRepr);
553-
checkString(keyReprString);
546+
if (nullKeyCheck.profile(keyReprString == null)) {
547+
throw raiseNode.raise(PythonErrorType.TypeError, "__repr__ returned non-string (type %s)", keyRepr);
548+
}
549+
550+
Object value = lib.getItem(s.dictStorage, key);
551+
Object valueRepr = value != s.dictStorage ? reprValueNode.executeObject(value, __REPR__) : "{...}";
554552
String valueReprString = castStr.execute(valueRepr);
555-
checkString(valueReprString);
553+
if (nullValueCheck.profile(valueReprString == null)) {
554+
throw raiseNode.raise(PythonErrorType.TypeError, "__repr__ returned non-string (type %s)", valueRepr);
555+
}
556556

557-
if (s.result.length() > 0) {
557+
// assuming '{' is inserted already
558+
if (lengthCheck.profile(s.result.length() > 1)) {
558559
sbAppend(s.result, ", ");
559560
}
560-
s.result.append(keyReprString).append(": ").append(valueReprString);
561+
sbAppend(s.result, keyReprString);
562+
sbAppend(s.result, ": ");
563+
sbAppend(s.result, valueReprString);
561564
return s;
562565
}
563566

564-
private void checkString(Object strObj) {
565-
if (!(strObj instanceof String)) {
566-
if (raiseNode == null) {
567-
raiseNode = insert(PRaiseNode.create());
568-
}
569-
throw raiseNode.raise(PythonErrorType.TypeError, "__repr__ returned non-string (type %s)", strObj);
570-
}
571-
}
572567
}
573568

574569
@Specialization(limit = "3") // use same limit as for EachRepr nodes library
575570
public Object repr(PDict self,
576571
@Cached("create(3)") EachRepr consumerNode,
577572
@CachedLibrary("self.getDictStorage()") HashingStorageLibrary lib) {
578-
StringBuilder result = new StringBuilder();
579-
sbAppend(result, "{");
573+
StringBuilder keyValue = new StringBuilder("{");
580574
HashingStorage dictStorage = self.getDictStorage();
581-
lib.forEach(dictStorage, consumerNode, new ReprState(self, dictStorage, result));
582-
return sbAppend(result, "}").toString();
575+
lib.forEach(dictStorage, consumerNode, new ReprState(dictStorage, keyValue));
576+
return sbAppend(keyValue, "}").toString();
583577
}
584578

585579
@TruffleBoundary

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ public Iterator<PythonAbstractClass> iterator() {
422422
@TruffleBoundary
423423
public Object[] toArray() {
424424
Object[] result = new Object[size()];
425-
Iterator<Object> keys = HashingStorageLibrary.getUncached().keys(dict.getDictStorage());
425+
Iterator<Object> keys = HashingStorageLibrary.getUncached().keys(dict.getDictStorage()).iterator();
426426
for (int i = 0; i < result.length; i++) {
427427
result[i] = keys.next();
428428
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
import java.util.function.Supplier;
5252
import java.util.logging.Level;
5353

54+
import org.graalvm.nativeimage.ImageInfo;
55+
import org.graalvm.options.OptionKey;
56+
5457
import com.oracle.graal.python.PythonLanguage;
5558
import com.oracle.graal.python.builtins.objects.PNone;
5659
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
@@ -94,9 +97,6 @@
9497
import com.oracle.truffle.api.nodes.Node;
9598
import com.oracle.truffle.api.utilities.CyclicAssumption;
9699

97-
import org.graalvm.nativeimage.ImageInfo;
98-
import org.graalvm.options.OptionKey;
99-
100100
public final class PythonContext {
101101
private static final TruffleLogger LOGGER = PythonLanguage.getLogger(PythonContext.class);
102102

0 commit comments

Comments
 (0)