Skip to content

Commit 64699f1

Browse files
cosminbascacosminbasca
committed
[GR-12491] PDictView must not override getDict on PythonObject - that method is reserved for access to __dict__, while the view wrapps an actual dict
PullRequest: graalpython/272
2 parents 17ba95e + 28ae091 commit 64699f1

File tree

6 files changed

+101
-41
lines changed

6 files changed

+101
-41
lines changed

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

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@
4242

4343
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodesFactory.LenNodeGen;
4444
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodesFactory.SetItemNodeGen;
45+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodesFactory.GetDictStorageNodeGen;
4546
import com.oracle.graal.python.nodes.PGuards;
4647
import com.oracle.graal.python.nodes.PNodeWithContext;
48+
import com.oracle.truffle.api.CompilerDirectives;
4749
import com.oracle.truffle.api.dsl.Cached;
4850
import com.oracle.truffle.api.dsl.ImportStatic;
4951
import com.oracle.truffle.api.dsl.Specialization;
@@ -52,14 +54,27 @@ public abstract class HashingCollectionNodes {
5254

5355
@ImportStatic(PGuards.class)
5456
public abstract static class LenNode extends PNodeWithContext {
57+
private @Child HashingStorageNodes.LenNode lenNode;
58+
59+
public HashingStorageNodes.LenNode getLenNode() {
60+
if (lenNode == null) {
61+
CompilerDirectives.transferToInterpreterAndInvalidate();
62+
lenNode = insert(HashingStorageNodes.LenNode.create());
63+
}
64+
return lenNode;
65+
}
5566

5667
public abstract int execute(PHashingCollection c);
5768

5869
@Specialization(limit = "4", guards = {"c.getClass() == cachedClass"})
59-
int doWithStorage(PHashingCollection c,
60-
@Cached("c.getClass()") Class<? extends PHashingCollection> cachedClass,
61-
@Cached("create()") HashingStorageNodes.LenNode lenNode) {
62-
return lenNode.execute(cachedClass.cast(c).getDictStorage());
70+
int getLenCached(PHashingCollection c,
71+
@Cached("c.getClass()") Class<? extends PHashingCollection> cachedClass) {
72+
return getLenNode().execute(cachedClass.cast(c).getDictStorage());
73+
}
74+
75+
@Specialization(replaces = "getLenCached")
76+
int getLenGeneric(PHashingCollection c) {
77+
return getLenNode().execute(c.getDictStorage());
6378
}
6479

6580
public static LenNode create() {
@@ -69,18 +84,52 @@ public static LenNode create() {
6984

7085
@ImportStatic(PGuards.class)
7186
public abstract static class SetItemNode extends PNodeWithContext {
87+
private @Child HashingStorageNodes.SetItemNode setItemNode;
88+
89+
public HashingStorageNodes.SetItemNode getSetItemNode() {
90+
if (setItemNode == null) {
91+
CompilerDirectives.transferToInterpreterAndInvalidate();
92+
setItemNode = insert(HashingStorageNodes.SetItemNode.create());
93+
}
94+
return setItemNode;
95+
}
7296

7397
public abstract void execute(PHashingCollection c, Object key, Object value);
7498

7599
@Specialization(limit = "4", guards = {"c.getClass() == cachedClass"})
76-
void doWithStorage(PHashingCollection c, Object key, Object value,
77-
@Cached("c.getClass()") Class<? extends PHashingCollection> cachedClass,
78-
@Cached("create()") HashingStorageNodes.SetItemNode setNode) {
79-
cachedClass.cast(c).setDictStorage(setNode.execute(cachedClass.cast(c).getDictStorage(), key, value));
100+
void doSetItemCached(PHashingCollection c, Object key, Object value,
101+
@Cached("c.getClass()") Class<? extends PHashingCollection> cachedClass) {
102+
cachedClass.cast(c).setDictStorage(getSetItemNode().execute(cachedClass.cast(c).getDictStorage(), key, value));
103+
}
104+
105+
@Specialization(replaces = "doSetItemCached")
106+
void doSetItemGeneric(PHashingCollection c, Object key, Object value) {
107+
c.setDictStorage(getSetItemNode().execute(c.getDictStorage(), key, value));
80108
}
81109

82110
public static SetItemNode create() {
83111
return SetItemNodeGen.create();
84112
}
85113
}
114+
115+
@ImportStatic({PGuards.class})
116+
public abstract static class GetDictStorageNode extends PNodeWithContext {
117+
118+
public abstract HashingStorage execute(PHashingCollection c);
119+
120+
@Specialization(limit = "4", guards = {"c.getClass() == cachedClass"})
121+
HashingStorage getStorageCached(PHashingCollection c,
122+
@Cached("c.getClass()") Class<? extends PHashingCollection> cachedClass) {
123+
return cachedClass.cast(c).getDictStorage();
124+
}
125+
126+
@Specialization(replaces = "getStorageCached")
127+
HashingStorage getStorageGeneric(PHashingCollection c) {
128+
return c.getDictStorage();
129+
}
130+
131+
public static GetDictStorageNode create() {
132+
return GetDictStorageNodeGen.create();
133+
}
134+
}
86135
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
6161
public abstract static class LenNode extends PythonBuiltinNode {
6262
@Specialization
6363
Object run(PDictView self) {
64-
return self.getDict().size();
64+
return self.getWrappedDict().size();
6565
}
6666
}
6767

@@ -70,8 +70,8 @@ Object run(PDictView self) {
7070
public abstract static class IterNode extends PythonUnaryBuiltinNode {
7171
@Specialization
7272
Object doPDictValuesView(PDictValuesView self) {
73-
if (self.getDict() != null) {
74-
return factory().createDictValuesIterator(self.getDict());
73+
if (self.getWrappedDict() != null) {
74+
return factory().createDictValuesIterator(self.getWrappedDict());
7575
}
7676
return PNone.NONE;
7777
}
@@ -84,8 +84,8 @@ public abstract static class EqNode extends PythonBuiltinNode {
8484
boolean doItemsView(PDictValuesView self, PDictValuesView other,
8585
@Cached("create()") HashingStorageNodes.ContainsKeyNode containsKeyNode) {
8686

87-
for (Object selfKey : self.getDict().keys()) {
88-
if (!containsKeyNode.execute(other.getDict().getDictStorage(), selfKey)) {
87+
for (Object selfKey : self.getWrappedDict().keys()) {
88+
if (!containsKeyNode.execute(other.getWrappedDict().getDictStorage(), selfKey)) {
8989
return false;
9090
}
9191
}

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

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
9696
public abstract static class LenNode extends PythonUnaryBuiltinNode {
9797
@Specialization
9898
Object len(PDictView self) {
99-
return self.getDict().size();
99+
return self.getWrappedDict().size();
100100
}
101101
}
102102

@@ -105,16 +105,16 @@ Object len(PDictView self) {
105105
public abstract static class IterNode extends PythonUnaryBuiltinNode {
106106
@Specialization
107107
Object getKeysViewIter(PDictKeysView self) {
108-
if (self.getDict() != null) {
109-
return factory().createDictKeysIterator(self.getDict());
108+
if (self.getWrappedDict() != null) {
109+
return factory().createDictKeysIterator(self.getWrappedDict());
110110
}
111111
return PNone.NONE;
112112
}
113113

114114
@Specialization
115115
Object getItemsViewIter(PDictItemsView self) {
116-
if (self.getDict() != null) {
117-
return factory().createDictItemsIterator(self.getDict());
116+
if (self.getWrappedDict() != null) {
117+
return factory().createDictItemsIterator(self.getWrappedDict());
118118
}
119119
return PNone.NONE;
120120
}
@@ -124,15 +124,15 @@ Object getItemsViewIter(PDictItemsView self) {
124124
@GenerateNodeFactory
125125
public abstract static class ContainsNode extends PythonBinaryBuiltinNode {
126126
@SuppressWarnings("unused")
127-
@Specialization(guards = "self.getDict().size() == 0")
127+
@Specialization(guards = "self.getWrappedDict().size() == 0")
128128
boolean containsEmpty(PDictView self, Object key) {
129129
return false;
130130
}
131131

132132
@Specialization
133133
boolean contains(PDictKeysView self, Object key,
134134
@Cached("create()") HashingStorageNodes.ContainsKeyNode containsKeyNode) {
135-
return containsKeyNode.execute(self.getDict().getDictStorage(), key);
135+
return containsKeyNode.execute(self.getWrappedDict().getDictStorage(), key);
136136
}
137137

138138
@Specialization
@@ -146,7 +146,7 @@ boolean contains(PDictItemsView self, PTuple key,
146146
if (tupleLenProfile.profile(lenNode.execute(tupleStorage) != 2)) {
147147
return false;
148148
}
149-
HashingStorage dictStorage = self.getDict().getDictStorage();
149+
HashingStorage dictStorage = self.getWrappedDict().getDictStorage();
150150
Object value = getDictItemNode.execute(dictStorage, getTupleItemNode.execute(tupleStorage, 0));
151151
return value != null && equivalenceNode.equals(value, getTupleItemNode.execute(tupleStorage, 1));
152152
}
@@ -159,21 +159,21 @@ public abstract static class EqNode extends PythonBinaryBuiltinNode {
159159
@Specialization
160160
boolean doKeysView(PDictKeysView self, PDictKeysView other,
161161
@Cached("create()") HashingStorageNodes.KeysEqualsNode equalsNode) {
162-
return equalsNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage());
162+
return equalsNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage());
163163
}
164164

165165
@Specialization
166166
boolean doKeysView(PDictKeysView self, PBaseSet other,
167167
@Cached("create()") HashingStorageNodes.KeysEqualsNode equalsNode) {
168-
return equalsNode.execute(self.getDict().getDictStorage(), other.getDictStorage());
168+
return equalsNode.execute(self.getWrappedDict().getDictStorage(), other.getDictStorage());
169169
}
170170

171171
@Specialization
172172
boolean doItemsView(PDictItemsView self, PDictItemsView other,
173173
@Cached("create()") HashingStorageNodes.EqualsNode equalsNode) {
174174
// the items view stores the original dict with K:V pairs so full K:V equality needs to
175175
// be tested in this case
176-
return equalsNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage());
176+
return equalsNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage());
177177
}
178178

179179
@Specialization
@@ -227,14 +227,14 @@ abstract static class SubNode extends PythonBinaryBuiltinNode {
227227
@Specialization
228228
PBaseSet doKeysView(PDictKeysView self, PBaseSet other,
229229
@Cached("create()") HashingStorageNodes.DiffNode diffNode) {
230-
HashingStorage storage = diffNode.execute(self.getDict().getDictStorage(), other.getDictStorage());
230+
HashingStorage storage = diffNode.execute(self.getWrappedDict().getDictStorage(), other.getDictStorage());
231231
return factory().createSet(storage);
232232
}
233233

234234
@Specialization
235235
PBaseSet doKeysView(PDictKeysView self, PDictKeysView other,
236236
@Cached("create()") HashingStorageNodes.DiffNode diffNode) {
237-
HashingStorage storage = diffNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage());
237+
HashingStorage storage = diffNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage());
238238
return factory().createSet(storage);
239239
}
240240

@@ -264,14 +264,14 @@ abstract static class AndNode extends PythonBinaryBuiltinNode {
264264
@Specialization
265265
PBaseSet doKeysView(PDictKeysView self, PBaseSet other,
266266
@Cached("create()") HashingStorageNodes.IntersectNode intersectNode) {
267-
HashingStorage intersectedStorage = intersectNode.execute(self.getDict().getDictStorage(), other.getDictStorage());
267+
HashingStorage intersectedStorage = intersectNode.execute(self.getWrappedDict().getDictStorage(), other.getDictStorage());
268268
return factory().createSet(intersectedStorage);
269269
}
270270

271271
@Specialization
272272
PBaseSet doKeysView(PDictKeysView self, PDictKeysView other,
273273
@Cached("create()") HashingStorageNodes.IntersectNode intersectNode) {
274-
HashingStorage intersectedStorage = intersectNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage());
274+
HashingStorage intersectedStorage = intersectNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage());
275275
return factory().createSet(intersectedStorage);
276276
}
277277

@@ -301,13 +301,13 @@ public abstract static class OrNode extends PythonBinaryBuiltinNode {
301301
@Specialization
302302
PBaseSet doKeysView(PDictKeysView self, PBaseSet other,
303303
@Cached("create()") HashingStorageNodes.UnionNode unionNode) {
304-
return factory().createSet(unionNode.execute(self.getDict().getDictStorage(), other.getDictStorage()));
304+
return factory().createSet(unionNode.execute(self.getWrappedDict().getDictStorage(), other.getDictStorage()));
305305
}
306306

307307
@Specialization
308308
PBaseSet doKeysView(PDictKeysView self, PDictKeysView other,
309309
@Cached("create()") HashingStorageNodes.UnionNode unionNode) {
310-
return factory().createSet(unionNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage()));
310+
return factory().createSet(unionNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage()));
311311
}
312312

313313
@Specialization
@@ -334,13 +334,13 @@ public abstract static class XorNode extends PythonBinaryBuiltinNode {
334334
@Specialization
335335
PBaseSet doKeysView(PDictKeysView self, PBaseSet other,
336336
@Cached("create()") HashingStorageNodes.ExclusiveOrNode xorNode) {
337-
return factory().createSet(xorNode.execute(self.getDict().getDictStorage(), other.getDictStorage()));
337+
return factory().createSet(xorNode.execute(self.getWrappedDict().getDictStorage(), other.getDictStorage()));
338338
}
339339

340340
@Specialization
341341
PBaseSet doKeysView(PDictKeysView self, PDictKeysView other,
342342
@Cached("create()") HashingStorageNodes.ExclusiveOrNode xorNode) {
343-
return factory().createSet(xorNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage()));
343+
return factory().createSet(xorNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage()));
344344
}
345345

346346
@Specialization
@@ -367,13 +367,13 @@ abstract static class LessEqualNode extends PythonBinaryBuiltinNode {
367367
@Specialization
368368
boolean lessEqual(PDictKeysView self, PBaseSet other,
369369
@Cached("create()") HashingStorageNodes.KeysIsSubsetNode isSubsetNode) {
370-
return isSubsetNode.execute(self.getDict().getDictStorage(), other.getDictStorage());
370+
return isSubsetNode.execute(self.getWrappedDict().getDictStorage(), other.getDictStorage());
371371
}
372372

373373
@Specialization
374374
boolean lessEqual(PDictKeysView self, PDictKeysView other,
375375
@Cached("create()") HashingStorageNodes.KeysIsSubsetNode isSubsetNode) {
376-
return isSubsetNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage());
376+
return isSubsetNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage());
377377
}
378378

379379
@Specialization
@@ -400,13 +400,13 @@ abstract static class GreaterEqualNode extends PythonBinaryBuiltinNode {
400400
@Specialization
401401
boolean greaterEqual(PDictKeysView self, PBaseSet other,
402402
@Cached("create()") HashingStorageNodes.KeysIsSupersetNode isSupersetNode) {
403-
return isSupersetNode.execute(self.getDict().getDictStorage(), other.getDictStorage());
403+
return isSupersetNode.execute(self.getWrappedDict().getDictStorage(), other.getDictStorage());
404404
}
405405

406406
@Specialization
407407
boolean greaterEqual(PDictKeysView self, PDictKeysView other,
408408
@Cached("create()") HashingStorageNodes.KeysIsSupersetNode isSupersetNode) {
409-
return isSupersetNode.execute(self.getDict().getDictStorage(), other.getDict().getDictStorage());
409+
return isSupersetNode.execute(self.getWrappedDict().getDictStorage(), other.getWrappedDict().getDictStorage());
410410
}
411411

412412
@Specialization

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ public PDictView(LazyPythonClass clazz, String name, PHashingCollection dict) {
5555
this.dict = dict;
5656
}
5757

58-
@Override
59-
public final PHashingCollection getDict() {
58+
public final PHashingCollection getWrappedDict() {
6059
return dict;
6160
}
6261

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/PythonObject.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,11 @@ public String toString() {
182182
/**
183183
* Returns the dictionary backed by {@link #storage} (only available for user objects).
184184
*/
185-
public PHashingCollection getDict() {
185+
public final PHashingCollection getDict() {
186186
return dict;
187187
}
188188

189-
public void setDict(PHashingCollection dict) {
189+
public final void setDict(PHashingCollection dict) {
190190
this.dict = dict;
191191
}
192192
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ObjectAttributeNode.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,31 @@
4141
package com.oracle.graal.python.nodes.attributes;
4242

4343
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage;
44+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
4445
import com.oracle.graal.python.builtins.objects.common.PHashingCollection;
4546
import com.oracle.graal.python.builtins.objects.object.PythonObject;
4647
import com.oracle.graal.python.builtins.objects.str.PString;
4748
import com.oracle.graal.python.nodes.PGuards;
4849
import com.oracle.graal.python.nodes.PNodeWithContext;
4950
import com.oracle.graal.python.runtime.PythonOptions;
51+
import com.oracle.truffle.api.CompilerDirectives;
5052
import com.oracle.truffle.api.dsl.ImportStatic;
5153
import com.oracle.truffle.api.object.HiddenKey;
5254
import com.oracle.truffle.api.object.Location;
5355
import com.oracle.truffle.api.object.Property;
5456

5557
@ImportStatic({PGuards.class, PythonOptions.class})
5658
public abstract class ObjectAttributeNode extends PNodeWithContext {
59+
private @Child HashingCollectionNodes.GetDictStorageNode getDictStorageNode;
60+
61+
public HashingCollectionNodes.GetDictStorageNode getGetDictStorageNode() {
62+
if (getDictStorageNode == null) {
63+
CompilerDirectives.transferToInterpreterAndInvalidate();
64+
getDictStorageNode = insert(HashingCollectionNodes.GetDictStorageNode.create());
65+
}
66+
return getDictStorageNode;
67+
}
68+
5769
protected Object attrKey(Object key) {
5870
if (key instanceof PString) {
5971
return ((PString) key).getValue();
@@ -67,7 +79,7 @@ protected boolean isDictUnsetOrSameAsStorage(PythonObject object) {
6779
if (dict == null) {
6880
return true;
6981
} else {
70-
return dict.getDictStorage() instanceof DynamicObjectStorage.PythonObjectDictStorage;
82+
return getGetDictStorageNode().execute(dict) instanceof DynamicObjectStorage.PythonObjectDictStorage;
7183
}
7284
}
7385

0 commit comments

Comments
 (0)