Skip to content

Commit 3dd18a5

Browse files
fangerertimfel
authored andcommitted
Fix: 'DictConcatNode' should not use 'addAll'.
1 parent 966899c commit 3dd18a5

File tree

2 files changed

+25
-17
lines changed

2 files changed

+25
-17
lines changed

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,10 @@ public HashingStorage doPDictKwargs(PHashingCollection dictLike, PKeyword[] kwar
324324
}
325325

326326
@Specialization(guards = "!isEmpty(kwargs)")
327-
public HashingStorage doPDictKwargs(PDict iterable, PKeyword[] kwargs) {
327+
public HashingStorage doPDictKwargs(PDict iterable, PKeyword[] kwargs,
328+
@Cached("create()") HashingStorageNodes.UnionNode unionNode) {
328329
HashingStorage dictStorage = iterable.getDictStorage().copy(HashingStorage.DEFAULT_EQIVALENCE);
329-
dictStorage.addAll(new KeywordsStorage(kwargs));
330+
unionNode.execute(dictStorage, new KeywordsStorage(kwargs));
330331
return dictStorage;
331332
}
332333

@@ -1352,8 +1353,8 @@ public HashingStorage doGenericSet(HashingStorage left, HashingStorage right) {
13521353
@Specialization(guards = "!setUnion")
13531354
public HashingStorage doGeneric(HashingStorage left, HashingStorage right) {
13541355
EconomicMapStorage newStorage = EconomicMapStorage.create(setUnion);
1355-
newStorage.addAll(left);
1356-
newStorage.addAll(right);
1356+
newStorage.addAll(left, getEquivalence());
1357+
newStorage.addAll(right, getEquivalence());
13571358
return newStorage;
13581359
}
13591360

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/generator/DictConcatNode.java

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2019, 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
@@ -40,26 +40,20 @@
4040
*/
4141
package com.oracle.graal.python.nodes.generator;
4242

43-
import com.oracle.graal.python.builtins.objects.common.HashingStorage.Equivalence;
44-
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.PythonEquivalence;
43+
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
44+
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes;
4545
import com.oracle.graal.python.builtins.objects.dict.PDict;
4646
import com.oracle.graal.python.nodes.expression.ExpressionNode;
4747
import com.oracle.truffle.api.CompilerDirectives;
48+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4849
import com.oracle.truffle.api.frame.VirtualFrame;
4950
import com.oracle.truffle.api.nodes.ExplodeLoop;
5051

5152
public final class DictConcatNode extends ExpressionNode {
5253

5354
@Children final ExpressionNode[] mappables;
54-
@Child private Equivalence equivalenceNode;
55-
56-
protected Equivalence getEquivalence() {
57-
if (equivalenceNode == null) {
58-
CompilerDirectives.transferToInterpreterAndInvalidate();
59-
equivalenceNode = insert(new PythonEquivalence());
60-
}
61-
return equivalenceNode;
62-
}
55+
@Child private HashingStorageNodes.SetItemNode setItemNode;
56+
@Child private HashingStorageNodes.GetItemNode getItemNode;
6357

6458
private DictConcatNode(ExpressionNode... mappablesNodes) {
6559
this.mappables = mappablesNodes;
@@ -75,12 +69,25 @@ public Object execute(VirtualFrame frame) {
7569
first = expectDict(n.execute(frame));
7670
} else {
7771
other = expectDict(n.execute(frame));
78-
first.getDictStorage().addAll(other.getDictStorage(), getEquivalence());
72+
addAllToDict(first, other);
7973
}
8074
}
8175
return first;
8276
}
8377

78+
@TruffleBoundary
79+
private void addAllToDict(PDict dict, PDict other) {
80+
if (setItemNode == null || getItemNode == null) {
81+
CompilerDirectives.transferToInterpreterAndInvalidate();
82+
setItemNode = insert(HashingStorageNodes.SetItemNode.create());
83+
getItemNode = insert(HashingStorageNodes.GetItemNode.create());
84+
}
85+
HashingStorage dictStorage = dict.getDictStorage();
86+
for (Object key : other.keys()) {
87+
setItemNode.execute(dictStorage, key, getItemNode.execute(other.getDictStorage(), key));
88+
}
89+
}
90+
8491
private static PDict expectDict(Object first) {
8592
if (!(first instanceof PDict)) {
8693
CompilerDirectives.transferToInterpreter();

0 commit comments

Comments
 (0)