Skip to content

Commit f235752

Browse files
committed
add support for non string keys to the object's magic __dict__
- added new hybrid storage for the case when non string keys are set - added relevant unittest - HashingStorageNodes: getItem add specialization for the case of the fixed PythonObject storage
1 parent 947f4c8 commit f235752

File tree

3 files changed

+137
-2
lines changed

3 files changed

+137
-2
lines changed

graalpython/com.oracle.graal.python.test/src/tests/test_dict.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,3 +425,13 @@ def helper_keys_contained(fn):
425425
# With the same size, an elementwise compare happens
426426
assert larger != larger3
427427
assert not larger == larger3
428+
429+
430+
def test_object_set_item_single_instance_non_str_key():
431+
class Foo(object):
432+
pass
433+
434+
f = Foo()
435+
f.__dict__[1] = 1
436+
f.a = 'a'
437+
assert f.__dict__ == {1: 1, 'a': 'a'}

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

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,4 +171,118 @@ public HashingStorage copy(Equivalence eq) {
171171
}
172172
}
173173

174+
public static class PythonObjectHybridDictStorage extends DynamicObjectStorage {
175+
private final EconomicMapStorage nonAttributesStorage;
176+
177+
public PythonObjectHybridDictStorage(PythonObjectDictStorage storage) {
178+
this(storage.getStore());
179+
}
180+
181+
public PythonObjectHybridDictStorage(DynamicObject store) {
182+
this(store, EconomicMapStorage.create(false));
183+
}
184+
185+
PythonObjectHybridDictStorage(DynamicObject store, EconomicMapStorage nonAttributesStorage) {
186+
super(store);
187+
this.nonAttributesStorage = nonAttributesStorage;
188+
}
189+
190+
@Override
191+
public int length() {
192+
return super.length() + nonAttributesStorage.length();
193+
}
194+
195+
@Override
196+
public boolean hasKey(Object key, Equivalence eq) {
197+
if (super.hasKey(key, DEFAULT_EQIVALENCE)) {
198+
return true;
199+
}
200+
return nonAttributesStorage.hasKey(key, eq);
201+
}
202+
203+
@Override
204+
public Object getItem(Object key, Equivalence eq) {
205+
Object value = super.getItem(key, DEFAULT_EQIVALENCE);
206+
if (value != null) {
207+
return value;
208+
}
209+
return this.nonAttributesStorage.getItem(key, eq);
210+
}
211+
212+
@Override
213+
public void setItem(Object key, Object value, Equivalence eq) {
214+
if (key instanceof String) {
215+
super.setItem(key, value, DEFAULT_EQIVALENCE);
216+
} else {
217+
this.nonAttributesStorage.setItem(key, value, eq);
218+
}
219+
}
220+
221+
@Override
222+
public boolean remove(Object key, Equivalence eq) {
223+
if (super.remove(key, DEFAULT_EQIVALENCE)) {
224+
return true;
225+
}
226+
return this.nonAttributesStorage.remove(key, eq);
227+
}
228+
229+
@Override
230+
public Iterable<Object> keys() {
231+
if (this.nonAttributesStorage.length() == 0) {
232+
return super.keys();
233+
} else {
234+
ArrayList<Object> entries = new ArrayList<>(this.length());
235+
for (Object entry : super.keys()) {
236+
entries.add(entry);
237+
}
238+
for (Object entry : this.nonAttributesStorage.keys()) {
239+
entries.add(entry);
240+
}
241+
return wrapJavaIterable(entries);
242+
}
243+
}
244+
245+
@Override
246+
public Iterable<Object> values() {
247+
if (this.nonAttributesStorage.length() == 0) {
248+
return super.values();
249+
} else {
250+
ArrayList<Object> entries = new ArrayList<>(this.length());
251+
for (Object entry : super.values()) {
252+
entries.add(entry);
253+
}
254+
for (Object entry : this.nonAttributesStorage.values()) {
255+
entries.add(entry);
256+
}
257+
return wrapJavaIterable(entries);
258+
}
259+
}
260+
261+
@Override
262+
public Iterable<DictEntry> entries() {
263+
if (this.nonAttributesStorage.length() == 0) {
264+
return super.entries();
265+
} else {
266+
ArrayList<DictEntry> entries = new ArrayList<>(this.length());
267+
for (DictEntry entry : super.entries()) {
268+
entries.add(entry);
269+
}
270+
for (DictEntry entry : this.nonAttributesStorage.entries()) {
271+
entries.add(entry);
272+
}
273+
return wrapJavaIterable(entries);
274+
}
275+
}
276+
277+
@Override
278+
public void clear() {
279+
super.clear();
280+
this.nonAttributesStorage.clear();
281+
}
282+
283+
@Override
284+
public HashingStorage copy(Equivalence eq) {
285+
return new PythonObjectHybridDictStorage(getStore().copy(getStore().getShape()), (EconomicMapStorage) nonAttributesStorage.copy(eq));
286+
}
287+
}
174288
}

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242

4343
import static com.oracle.graal.python.builtins.objects.common.HashingStorage.DEFAULT_EQIVALENCE;
4444
import static com.oracle.graal.python.nodes.SpecialMethodNames.__HASH__;
45-
import static com.oracle.graal.python.runtime.exception.PythonErrorType.KeyError;
4645
import static com.oracle.graal.python.runtime.exception.PythonErrorType.TypeError;
4746
import static com.oracle.graal.python.runtime.exception.PythonErrorType.ValueError;
4847

@@ -52,6 +51,7 @@
5251
import com.oracle.graal.python.builtins.objects.PNone;
5352
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage.FastDictStorage;
5453
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage.PythonObjectDictStorage;
54+
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage.PythonObjectHybridDictStorage;
5555
import com.oracle.graal.python.builtins.objects.common.HashingStorage.Equivalence;
5656
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.ContainsKeyNodeGen;
5757
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.ContainsValueNodeGen;
@@ -237,6 +237,12 @@ protected static DynamicObjectStorage switchToFastDictStorage(PHashingCollection
237237
return newStorage;
238238
}
239239

240+
protected static PythonObjectHybridDictStorage switchToHybridDictStorage(PHashingCollection container, PythonObjectDictStorage dictStorage) {
241+
PythonObjectHybridDictStorage newStorage = new PythonObjectHybridDictStorage(dictStorage);
242+
container.setDictStorage(newStorage);
243+
return newStorage;
244+
}
245+
240246
protected static Location lookupLocation(Shape shape, Object name) {
241247
/* Initialization of cached values always happens in a slow path. */
242248
CompilerAsserts.neverPartOfCompilation();
@@ -828,7 +834,7 @@ protected void doKeywordsPStringGeneralize(PHashingCollection container, Keyword
828834
@Specialization(guards = {"!isJavaString(key)", "isHashable(key)"})
829835
@SuppressWarnings("unused")
830836
protected void doDynamicObjectGeneralize(PHashingCollection container, PythonObjectDictStorage storage, Object key, Object value) {
831-
throw raise(KeyError, "unsupported key: %p", key);
837+
switchToHybridDictStorage(container, storage).setItem(key, value, getEquivalence());
832838
}
833839

834840
@Specialization(guards = {"!isJavaString(key)", "isHashable(key)"})
@@ -965,6 +971,11 @@ protected Object doDynamicObjectUpdateShapePString(DynamicObjectStorage storage,
965971
return doDynamicObjectUncachedPString(storage, name);
966972
}
967973

974+
@Specialization(guards = {"!isJavaString(key)", "isHashable(key)"})
975+
Object doDynamicObject(PythonObjectHybridDictStorage storage, Object key) {
976+
return storage.getItem(key, getEquivalence());
977+
}
978+
968979
@Specialization(guards = {"!isJavaString(key)", "isHashable(key)"})
969980
@SuppressWarnings("unused")
970981
Object doDynamicObject(DynamicObjectStorage storage, Object key) {

0 commit comments

Comments
 (0)