Skip to content

Commit f605d96

Browse files
fangererfangerer
committed
Fix: do not sync non-user frame values.
1 parent c0ed366 commit f605d96

File tree

3 files changed

+55
-50
lines changed

3 files changed

+55
-50
lines changed

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,17 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.common;
4242

43-
import static com.oracle.graal.python.nodes.frame.FrameSlotIDs.RETURN_SLOT_ID;
44-
import static com.oracle.graal.python.nodes.frame.FrameSlotIDs.TEMP_LOCAL_PREFIX;
45-
4643
import java.util.Iterator;
4744
import java.util.NoSuchElementException;
4845

4946
import com.oracle.graal.python.builtins.objects.cell.PCell;
47+
import com.oracle.graal.python.nodes.frame.FrameSlotIDs;
5048
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5149
import com.oracle.truffle.api.Truffle;
5250
import com.oracle.truffle.api.frame.FrameDescriptor;
5351
import com.oracle.truffle.api.frame.FrameSlot;
5452
import com.oracle.truffle.api.frame.MaterializedFrame;
5553

56-
// XXX: remove special case for RETURN_SLOT_ID
5754
public class LocalsStorage extends HashingStorage {
5855

5956
/* This won't be the real (materialized) frame but a clone of it. */
@@ -92,19 +89,13 @@ public void addAll(HashingStorage other, Equivalence eq) {
9289
@TruffleBoundary
9390
public Object getItem(Object key, Equivalence eq) {
9491
assert eq == DEFAULT_EQIVALENCE;
95-
if (RETURN_SLOT_ID.equals(key)) {
96-
return null;
97-
} else if (isTempLocal(key)) {
92+
if (!FrameSlotIDs.isUserFrameSlot(key)) {
9893
return null;
9994
}
10095
FrameSlot slot = frame.getFrameDescriptor().findFrameSlot(key);
10196
return getValue(slot);
10297
}
10398

104-
private static boolean isTempLocal(Object key) {
105-
return key instanceof String && ((String) key).startsWith(TEMP_LOCAL_PREFIX);
106-
}
107-
10899
@Override
109100
public void setItem(Object key, Object value, Equivalence eq) {
110101
throw UnmodifiableStorageException.INSTANCE;
@@ -124,9 +115,7 @@ public Iterator<Object> iterator() {
124115
@TruffleBoundary
125116
public boolean hasKey(Object key, Equivalence eq) {
126117
assert eq == DEFAULT_EQIVALENCE;
127-
if (RETURN_SLOT_ID.equals(key)) {
128-
return false;
129-
} else if (isTempLocal(key)) {
118+
if (!FrameSlotIDs.isUserFrameSlot(key)) {
130119
return false;
131120
}
132121
return frame.getFrameDescriptor().findFrameSlot(key) != null;
@@ -139,7 +128,7 @@ public int length() {
139128
len = frame.getFrameDescriptor().getSize();
140129
for (FrameSlot slot : frame.getFrameDescriptor().getSlots()) {
141130
Object identifier = slot.getIdentifier();
142-
if (identifier.equals(RETURN_SLOT_ID) || isTempLocal(identifier) || frame.getValue(slot) == null) {
131+
if (!FrameSlotIDs.isUserFrameSlot(identifier) || frame.getValue(slot) == null) {
143132
len--;
144133
}
145134
}
@@ -219,7 +208,7 @@ private boolean loadNext() {
219208
FrameSlot nextCandidate = keysIterator().next();
220209
Object identifier = nextCandidate.getIdentifier();
221210
if (identifier instanceof String) {
222-
if (!RETURN_SLOT_ID.equals(identifier) && !isTempLocal(identifier)) {
211+
if (FrameSlotIDs.isUserFrameSlot(identifier)) {
223212
Object nextValue = frame.getValue(nextCandidate);
224213
if (nextValue != null) {
225214
nextFrameSlot = nextCandidate;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/FrameSlotIDs.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,12 @@
2828
public abstract class FrameSlotIDs {
2929
public static final String RETURN_SLOT_ID = "<return_val>";
3030
public static final String TEMP_LOCAL_PREFIX = "<>temp";
31+
32+
public static boolean isTempLocal(Object key) {
33+
return key instanceof String && ((String) key).startsWith(TEMP_LOCAL_PREFIX);
34+
}
35+
36+
public static boolean isUserFrameSlot(Object key) {
37+
return key instanceof String && !RETURN_SLOT_ID.equals(key) && !isTempLocal(key);
38+
}
3139
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -245,20 +245,22 @@ static void doLocalsStorageCached(PFrame pyFrame, Frame frameToSync,
245245

246246
for (int i = 0; i < cachedSlots.length; i++) {
247247
FrameSlot slot = cachedSlots[i];
248-
if (frameToSync.isBoolean(slot)) {
249-
target.setBoolean(slot, frameToSync.getBoolean(slot));
250-
} else if (frameToSync.isByte(slot)) {
251-
target.setByte(slot, frameToSync.getByte(slot));
252-
} else if (frameToSync.isDouble(slot)) {
253-
target.setDouble(slot, frameToSync.getDouble(slot));
254-
} else if (frameToSync.isFloat(slot)) {
255-
target.setFloat(slot, frameToSync.getFloat(slot));
256-
} else if (frameToSync.isInt(slot)) {
257-
target.setInt(slot, frameToSync.getInt(slot));
258-
} else if (frameToSync.isLong(slot)) {
259-
target.setLong(slot, frameToSync.getLong(slot));
260-
} else if (frameToSync.isObject(slot)) {
261-
target.setObject(slot, frameToSync.getObject(slot));
248+
if (FrameSlotIDs.isUserFrameSlot(slot.getIdentifier())) {
249+
if (frameToSync.isBoolean(slot)) {
250+
target.setBoolean(slot, frameToSync.getBoolean(slot));
251+
} else if (frameToSync.isByte(slot)) {
252+
target.setByte(slot, frameToSync.getByte(slot));
253+
} else if (frameToSync.isDouble(slot)) {
254+
target.setDouble(slot, frameToSync.getDouble(slot));
255+
} else if (frameToSync.isFloat(slot)) {
256+
target.setFloat(slot, frameToSync.getFloat(slot));
257+
} else if (frameToSync.isInt(slot)) {
258+
target.setInt(slot, frameToSync.getInt(slot));
259+
} else if (frameToSync.isLong(slot)) {
260+
target.setLong(slot, frameToSync.getLong(slot));
261+
} else if (frameToSync.isObject(slot)) {
262+
target.setObject(slot, frameToSync.getObject(slot));
263+
}
262264
}
263265
}
264266
} catch (FrameSlotTypeException e) {
@@ -277,20 +279,22 @@ static void doLocalsStorageUncached(PFrame pyFrame, Frame frameToSync) {
277279

278280
for (int i = 0; i < cachedSlots.length; i++) {
279281
FrameSlot slot = cachedSlots[i];
280-
if (frameToSync.isBoolean(slot)) {
281-
target.setBoolean(slot, frameToSync.getBoolean(slot));
282-
} else if (frameToSync.isByte(slot)) {
283-
target.setByte(slot, frameToSync.getByte(slot));
284-
} else if (frameToSync.isDouble(slot)) {
285-
target.setDouble(slot, frameToSync.getDouble(slot));
286-
} else if (frameToSync.isFloat(slot)) {
287-
target.setFloat(slot, frameToSync.getFloat(slot));
288-
} else if (frameToSync.isInt(slot)) {
289-
target.setInt(slot, frameToSync.getInt(slot));
290-
} else if (frameToSync.isLong(slot)) {
291-
target.setLong(slot, frameToSync.getLong(slot));
292-
} else if (frameToSync.isObject(slot)) {
293-
target.setObject(slot, frameToSync.getObject(slot));
282+
if (FrameSlotIDs.isUserFrameSlot(slot.getIdentifier())) {
283+
if (frameToSync.isBoolean(slot)) {
284+
target.setBoolean(slot, frameToSync.getBoolean(slot));
285+
} else if (frameToSync.isByte(slot)) {
286+
target.setByte(slot, frameToSync.getByte(slot));
287+
} else if (frameToSync.isDouble(slot)) {
288+
target.setDouble(slot, frameToSync.getDouble(slot));
289+
} else if (frameToSync.isFloat(slot)) {
290+
target.setFloat(slot, frameToSync.getFloat(slot));
291+
} else if (frameToSync.isInt(slot)) {
292+
target.setInt(slot, frameToSync.getInt(slot));
293+
} else if (frameToSync.isLong(slot)) {
294+
target.setLong(slot, frameToSync.getLong(slot));
295+
} else if (frameToSync.isObject(slot)) {
296+
target.setObject(slot, frameToSync.getObject(slot));
297+
}
294298
}
295299
}
296300
} catch (FrameSlotTypeException e) {
@@ -315,9 +319,11 @@ static void doGenericDictCached(VirtualFrame frame, PFrame pyFrame, Frame frameT
315319

316320
for (int i = 0; i < cachedSlots.length; i++) {
317321
FrameSlot slot = cachedSlots[i];
318-
Object value = frameToSync.getValue(slot);
319-
if (value != null) {
320-
setItemNode.execute(frame, localsDict, slot.getIdentifier(), value);
322+
if (FrameSlotIDs.isUserFrameSlot(slot.getIdentifier())) {
323+
Object value = frameToSync.getValue(slot);
324+
if (value != null) {
325+
setItemNode.execute(frame, localsDict, slot.getIdentifier(), value);
326+
}
321327
}
322328
}
323329
}
@@ -336,9 +342,11 @@ static void doGenericDict(VirtualFrame frame, PFrame pyFrame, Frame frameToSync,
336342

337343
for (int i = 0; i < slots.length; i++) {
338344
FrameSlot slot = slots[i];
339-
Object value = frameToSync.getValue(slot);
340-
if (value != null) {
341-
setItemNode.execute(frame, localsDict, slot.getIdentifier(), value);
345+
if (FrameSlotIDs.isUserFrameSlot(slot.getIdentifier())) {
346+
Object value = frameToSync.getValue(slot);
347+
if (value != null) {
348+
setItemNode.execute(frame, localsDict, slot.getIdentifier(), value);
349+
}
342350
}
343351
}
344352
}

0 commit comments

Comments
 (0)