Skip to content

Commit 7192258

Browse files
tomasstupkatomasstupka
committed
[GR-23218] Make test_descr pass - test_type_lookup_mro_reference.
PullRequest: graalpython/1369
2 parents 0878b8e + 5595c50 commit 7192258

File tree

12 files changed

+193
-131
lines changed

12 files changed

+193
-131
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_mro.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
1+
# Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
22
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
33
#
44
# The Universal Permissive License (UPL), Version 1.0
@@ -206,3 +206,57 @@ class Z():
206206
assert True
207207
else:
208208
assert False
209+
210+
def test_mro_change_on_attr_access():
211+
eq_called = []
212+
class MyKey(object):
213+
def __hash__(self):
214+
return hash('mykey')
215+
def __eq__(self, other):
216+
eq_called.append(1)
217+
X.__bases__ = (Base2,)
218+
219+
class Base(object):
220+
mykey = 'base 42'
221+
222+
class Base2(object):
223+
mykey = 'base2 42'
224+
225+
X = type('X', (Base,), {MyKey(): 5})
226+
assert X.mykey == 'base 42'
227+
assert eq_called == [1]
228+
229+
# ----------------------------------
230+
class MyKey(object):
231+
def __hash__(self):
232+
return hash('mykey')
233+
def __eq__(self, other):
234+
X.__bases__ = (Base,)
235+
236+
class Base(object):
237+
pass
238+
239+
class Base2(object):
240+
mykey = '42'
241+
242+
X = type('X', (Base,Base2,), {MyKey(): 5})
243+
mk = X.mykey
244+
assert mk == '42'
245+
246+
X = type('X', (Base2,), {MyKey(): 5})
247+
assert X.mykey == '42'
248+
249+
# ----------------------------------
250+
class Base(object):
251+
mykey = 'from Base2'
252+
253+
class Base2(object):
254+
pass
255+
256+
X = type('X', (Base2,), {MyKey(): 5})
257+
try:
258+
assert X.mykey == '42'
259+
except AttributeError as e:
260+
assert True
261+
else:
262+
assert False

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_descr.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
*graalpython.lib-python.3.test.test_descr.DictProxyTests.test_iter_keys
102102
*graalpython.lib-python.3.test.test_descr.DictProxyTests.test_iter_values
103103
*graalpython.lib-python.3.test.test_descr.DictProxyTests.test_repr
104+
*graalpython.lib-python.3.test.test_descr.MiscTests.test_type_lookup_mro_reference
104105
*graalpython.lib-python.3.test.test_descr.MroTest.test_incomplete_extend
105106
*graalpython.lib-python.3.test.test_descr.MroTest.test_incomplete_set_bases_on_self
106107
*graalpython.lib-python.3.test.test_descr.MroTest.test_incomplete_super

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2199,8 +2199,8 @@ Object type(Object cls, Object obj, PNone bases, PNone dict, PKeyword[] kwds,
21992199

22002200
@Specialization(guards = "isString(wName)")
22012201
Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict namespaceOrig, PKeyword[] kwds,
2202-
@CachedLibrary(limit = "4") PythonObjectLibrary lib,
2203-
@CachedLibrary(limit = "2") HashingStorageLibrary nslib,
2202+
@CachedLibrary(limit = "5") PythonObjectLibrary lib,
2203+
@CachedLibrary(limit = "3") HashingStorageLibrary hashingStoragelib,
22042204
@Cached BranchProfile updatedStorage,
22052205
@Cached("create(__NEW__)") LookupInheritedAttributeNode getNewFuncNode,
22062206
@Cached("create(__INIT_SUBCLASS__)") GetAttributeNode getInitSubclassNode,
@@ -2230,9 +2230,9 @@ Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict
22302230

22312231
try {
22322232
PDict namespace = (PDict) copyDict.call(frame, namespaceOrig);
2233-
PythonClass newType = typeMetaclass(frame, name, bases, namespace, metaclass, nslib, getDictAttrNode, getWeakRefAttrNode, getBestBaseNode, getItemSize, writeItemSize);
2233+
PythonClass newType = typeMetaclass(frame, name, bases, namespace, metaclass, lib, hashingStoragelib, getDictAttrNode, getWeakRefAttrNode, getBestBaseNode, getItemSize, writeItemSize);
22342234

2235-
for (DictEntry entry : nslib.entries(namespace.getDictStorage())) {
2235+
for (DictEntry entry : hashingStoragelib.entries(namespace.getDictStorage())) {
22362236
Object setName = getSetNameNode.execute(entry.value);
22372237
if (setName != PNone.NO_VALUE) {
22382238
try {
@@ -2254,32 +2254,32 @@ Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict
22542254
PFrame callerFrame = getReadCallerFrameNode().executeWith(frame, 0);
22552255
PythonObject globals = callerFrame.getGlobals();
22562256
if (globals != null) {
2257-
String moduleName = getModuleNameFromGlobals(globals, nslib);
2257+
String moduleName = getModuleNameFromGlobals(globals, hashingStoragelib);
22582258
if (moduleName != null) {
22592259
ensureWriteAttrNode().execute(frame, newType, __MODULE__, moduleName);
22602260
}
22612261
}
22622262
}
22632263

22642264
// delete __qualname__ from namespace
2265-
if (nslib.hasKey(namespace.getDictStorage(), __QUALNAME__)) {
2266-
HashingStorage newStore = nslib.delItem(namespace.getDictStorage(), __QUALNAME__);
2265+
if (hashingStoragelib.hasKey(namespace.getDictStorage(), __QUALNAME__)) {
2266+
HashingStorage newStore = hashingStoragelib.delItem(namespace.getDictStorage(), __QUALNAME__);
22672267
if (newStore != namespace.getDictStorage()) {
22682268
updatedStorage.enter();
22692269
namespace.setDictStorage(newStore);
22702270
}
22712271
}
22722272

22732273
// set __class__ cell contents
2274-
Object classcell = nslib.getItem(namespace.getDictStorage(), __CLASSCELL__);
2274+
Object classcell = hashingStoragelib.getItem(namespace.getDictStorage(), __CLASSCELL__);
22752275
if (classcell != null) {
22762276
if (classcell instanceof PCell) {
22772277
((PCell) classcell).setRef(newType);
22782278
} else {
22792279
raise(TypeError, ErrorMessages.MUST_BE_A_CELL, "__classcell__");
22802280
}
2281-
if (nslib.hasKey(namespace.getDictStorage(), __CLASSCELL__)) {
2282-
HashingStorage newStore = nslib.delItem(namespace.getDictStorage(), __CLASSCELL__);
2281+
if (hashingStoragelib.hasKey(namespace.getDictStorage(), __CLASSCELL__)) {
2282+
HashingStorage newStore = hashingStoragelib.delItem(namespace.getDictStorage(), __CLASSCELL__);
22832283
if (newStore != namespace.getDictStorage()) {
22842284
updatedStorage.enter();
22852285
namespace.setDictStorage(newStore);
@@ -2325,8 +2325,8 @@ private String getModuleNameFromGlobals(PythonObject globals, HashingStorageLibr
23252325
}
23262326

23272327
private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases, PDict namespace, Object metaclass,
2328-
HashingStorageLibrary nslib, LookupAttributeInMRONode getDictAttrNode, LookupAttributeInMRONode getWeakRefAttrNode, GetBestBaseClassNode getBestBaseNode,
2329-
GetItemsizeNode getItemSize, WriteAttributeToObjectNode writeItemSize) {
2328+
PythonObjectLibrary lib, HashingStorageLibrary hashingStorageLib, LookupAttributeInMRONode getDictAttrNode,
2329+
LookupAttributeInMRONode getWeakRefAttrNode, GetBestBaseClassNode getBestBaseNode, GetItemsizeNode getItemSize, WriteAttributeToObjectNode writeItemSize) {
23302330
Object[] array = ensureGetObjectArrayNode().execute(bases);
23312331

23322332
PythonAbstractClass[] basesArray;
@@ -2360,7 +2360,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
23602360
// 2.) copy the dictionary slots
23612361
Object[] slots = new Object[1];
23622362
boolean[] qualnameSet = new boolean[]{false};
2363-
copyDictSlots(pythonClass, namespace, nslib, slots, qualnameSet);
2363+
copyDictSlots(pythonClass, namespace, lib, hashingStorageLib, slots, qualnameSet);
23642364
if (!qualnameSet[0]) {
23652365
pythonClass.setQualName(name);
23662366
}
@@ -2370,9 +2370,9 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
23702370

23712371
// CPython masks the __hash__ method with None when __eq__ is overriden, but __hash__ is
23722372
// not
2373-
Object hashMethod = nslib.getItem(namespace.getDictStorage(), __HASH__);
2373+
Object hashMethod = hashingStorageLib.getItem(namespace.getDictStorage(), __HASH__);
23742374
if (hashMethod == null) {
2375-
Object eqMethod = nslib.getItem(namespace.getDictStorage(), __EQ__);
2375+
Object eqMethod = hashingStorageLib.getItem(namespace.getDictStorage(), __EQ__);
23762376
if (eqMethod != null) {
23772377
pythonClass.setAttribute(__HASH__, PNone.NONE);
23782378
}
@@ -2456,7 +2456,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
24562456
}
24572457

24582458
// checks for some name errors too
2459-
PTuple newSlots = copySlots(name, slotsStorage, slotlen, addDict, addWeakRef, namespace, nslib);
2459+
PTuple newSlots = copySlots(name, slotsStorage, slotlen, addDict, addWeakRef, namespace, hashingStorageLib);
24602460

24612461
// add native slot descriptors
24622462
if (pythonClass.needsNativeAllocation()) {
@@ -2516,10 +2516,11 @@ private static boolean hasPythonClassBases(PythonAbstractClass[] basesArray) {
25162516
return false;
25172517
}
25182518

2519-
private void copyDictSlots(PythonClass pythonClass, PDict namespace, HashingStorageLibrary nslib, Object[] slots, boolean[] qualnameSet) {
2519+
private void copyDictSlots(PythonClass pythonClass, PDict namespace, PythonObjectLibrary lib, HashingStorageLibrary hashingStorageLib, Object[] slots, boolean[] qualnameSet) {
25202520
// copy the dictionary slots over, as CPython does through PyDict_Copy
25212521
// Also check for a __slots__ sequence variable in dict
2522-
for (DictEntry entry : nslib.entries(namespace.getDictStorage())) {
2522+
PDict typeDict = null;
2523+
for (DictEntry entry : hashingStorageLib.entries(namespace.getDictStorage())) {
25232524
Object key = entry.getKey();
25242525
Object value = entry.getValue();
25252526
if (__SLOTS__.equals(key)) {
@@ -2566,8 +2567,25 @@ private void copyDictSlots(PythonClass pythonClass, PDict namespace, HashingStor
25662567
}
25672568
} else if (SpecialAttributeNames.__CLASSCELL__.equals(key)) {
25682569
// don't populate this attribute
2569-
} else {
2570+
} else if (key instanceof String && typeDict == null) {
25702571
pythonClass.setAttribute(key, value);
2572+
} else {
2573+
// DynamicObjectStorage ignores non-string keys
2574+
typeDict = lib.getDict(pythonClass);
2575+
if (typeDict == null) {
2576+
// 1.) create DynamicObjectStorage based dict from pythonClass
2577+
typeDict = PythonObjectFactory.getUncached().createDictFixedStorage(pythonClass);
2578+
try {
2579+
lib.setDict(pythonClass, typeDict);
2580+
} catch (UnsupportedMessageException ex) {
2581+
CompilerDirectives.transferToInterpreterAndInvalidate();
2582+
throw new IllegalStateException("can't set dict into " + pythonClass, ex);
2583+
}
2584+
}
2585+
// 2.) writing a non string key converts DynamicObjectStorage to
2586+
// EconomicMapStorage
2587+
HashingStorage updatedStore = hashingStorageLib.setItem(typeDict.getDictStorage(), key, value);
2588+
typeDict.setDictStorage(updatedStore);
25712589
}
25722590
}
25732591
}

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ static Object getItemGeneric(EconomicMapStorage self, Object key, ThreadState st
188188
}
189189
}
190190

191+
@ExportMessage
191192
protected static boolean hasSideEffect(EconomicMapStorage self) {
192193
return self.map.hasSideEffect();
193194
}
@@ -351,7 +352,7 @@ static HashingStorage generic(EconomicMapStorage self, HashingStorage other,
351352
}
352353
}
353354

354-
protected static boolean hasSideEffect(Object o, LookupInheritedAttributeNode.Dynamic lookup) {
355+
private static boolean hasDELSideEffect(Object o, LookupInheritedAttributeNode.Dynamic lookup) {
355356
return o instanceof PythonObject && lookup.execute(o, __DEL__) != PNone.NO_VALUE;
356357
}
357358

@@ -377,10 +378,10 @@ static HashingStorage delItemWithStateWithSideEffect(EconomicMapStorage self, Ob
377378
@Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
378379
DictKey newKey = new DictKey(key, getHashWithState(key, lib, state, gotState));
379380
Object value = self.map.removeKey(newKey, lib, otherlib, gotState, state);
380-
if (hasSideEffect(key, lookup)) {
381+
if (hasDELSideEffect(key, lookup)) {
381382
callNode.executeObject(lookup.execute(key, __DEL__), key);
382383
}
383-
if (hasSideEffect(value, lookup)) {
384+
if (hasDELSideEffect(value, lookup)) {
384385
callNode.executeObject(lookup.execute(value, __DEL__), value);
385386
}
386387
return self;
@@ -409,8 +410,8 @@ static HashingStorage clearWithSideEffect(EconomicMapStorage self,
409410
while (advance(cursor)) {
410411
Object key = getKey(cursor);
411412
Object value = getValue(cursor);
412-
entries[i++] = hasSideEffect(key, lookup) ? key : null;
413-
entries[i++] = hasSideEffect(value, lookup) ? value : null;
413+
entries[i++] = hasDELSideEffect(key, lookup) ? key : null;
414+
entries[i++] = hasDELSideEffect(value, lookup) ? value : null;
414415
}
415416
self.map.clear();
416417
for (Object o : entries) {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,16 @@ public final <T> T forEach(HashingStorage self, ForEachNode<T> node, T arg) {
293293
*/
294294
public abstract HashingStorage copy(HashingStorage self);
295295

296+
/**
297+
* Determines if the storage has elements with a potential side effect on access.
298+
*
299+
* @return {@code true} if the storage has elements with a potential side effect, otherwise
300+
* {@code false}.
301+
*/
302+
public boolean hasSideEffect(HashingStorage self) {
303+
return false;
304+
}
305+
296306
/**
297307
* @return {@code true} if the key-value pairs are equal between these storages.
298308
*/

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.oracle.graal.python.builtins.objects.PNone;
3535
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
3636
import com.oracle.graal.python.builtins.objects.dict.PDict;
37+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
3738
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
3839
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
3940
import com.oracle.graal.python.nodes.HiddenAttributes;
@@ -127,6 +128,7 @@ public final Object getAttribute(Object key) {
127128
@SuppressWarnings("deprecation")
128129
@TruffleBoundary
129130
public void setAttribute(Object name, Object value) {
131+
assert name instanceof String || name instanceof HiddenPythonKey || name instanceof HiddenKey : name.getClass().getSimpleName();
130132
CompilerAsserts.neverPartOfCompilation();
131133
DynamicObjectLibrary.getUncached().putWithFlags(getStorage(), name, value, 0);
132134
}

0 commit comments

Comments
 (0)