Skip to content

Commit 7f330d9

Browse files
committed
[GR-65227] Ensure writing same value for all keys in a HashingStorage handles storage generalization
1 parent b06135f commit 7f330d9

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import com.oracle.graal.python.nodes.PGuards;
6363
import com.oracle.graal.python.nodes.PNodeWithContext;
6464
import com.oracle.graal.python.nodes.util.CastToTruffleStringNode;
65+
import com.oracle.truffle.api.CompilerDirectives;
6566
import com.oracle.truffle.api.HostCompilerDirectives.InliningCutoff;
6667
import com.oracle.truffle.api.dsl.Bind;
6768
import com.oracle.truffle.api.dsl.Cached;
@@ -122,8 +123,8 @@ abstract static class SetValueHashingStorageNode extends PNodeWithContext {
122123

123124
@Specialization
124125
static HashingStorage doEconomicStorage(VirtualFrame frame, Node inliningTarget, EconomicMapStorage map, Object value,
125-
@Cached PutUnsafeNode putNode,
126-
@Cached InlinedLoopConditionProfile loopProfile) {
126+
@Shared("putNode") @Cached PutUnsafeNode putNode,
127+
@Shared("loopProfile") @Cached InlinedLoopConditionProfile loopProfile) {
127128
// We want to avoid calling __hash__() during map.put
128129
map.setValueForAllKeys(frame, inliningTarget, value, putNode, loopProfile);
129130
return map;
@@ -135,14 +136,25 @@ static HashingStorage doGeneric(VirtualFrame frame, Node inliningTarget, Hashing
135136
@Cached HashingStorageSetItem setItem,
136137
@Cached HashingStorageGetIterator getIterator,
137138
@Cached HashingStorageIteratorNext itNext,
138-
@Cached HashingStorageIteratorKey itKey) {
139+
@Cached HashingStorageIteratorKey itKey,
140+
@Shared("putNode") @Cached PutUnsafeNode putNode,
141+
@Shared("loopProfile") @Cached InlinedLoopConditionProfile loopProfile) {
139142
HashingStorageIterator it = getIterator.execute(inliningTarget, map);
140-
HashingStorage storage = map;
141143
while (itNext.execute(inliningTarget, map, it)) {
142-
Object key = itKey.execute(inliningTarget, storage, it);
143-
storage = setItem.execute(frame, inliningTarget, storage, key, value);
144+
Object key = itKey.execute(inliningTarget, map, it);
145+
HashingStorage newStorage = setItem.execute(frame, inliningTarget, map, key, value);
146+
if (newStorage != map) {
147+
// when the storage changes, the iterator state is not a reliable cursor
148+
// anymore and we need to restart.
149+
if (newStorage instanceof EconomicMapStorage mapStorage) {
150+
mapStorage.setValueForAllKeys(frame, inliningTarget, value, putNode, loopProfile);
151+
return mapStorage;
152+
} else {
153+
throw CompilerDirectives.shouldNotReachHere("We only generalize to EconomicMapStorage");
154+
}
155+
}
144156
}
145-
return storage;
157+
return map;
146158
}
147159

148160
protected static boolean isEconomicMapStorage(Object o) {

0 commit comments

Comments
 (0)