Skip to content

Commit 98bcc9d

Browse files
steve-ssteve-s
committed
[GR-35958] Fix GetSlotNamesNode was not getting the right value of __slots__ and other small fixes.
PullRequest: graalpython/2089
2 parents c412103 + 7414834 commit 98bcc9d

File tree

6 files changed

+90
-23
lines changed

6 files changed

+90
-23
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,22 @@ def assert_raises(err, fn, *args, **kwargs):
4747
assert raised
4848

4949

50+
def test_reduce_ex_with_slots():
51+
# Adapted from test_desc.test_issue24097
52+
class A:
53+
__slotnames__ = ['spam']
54+
55+
def __getattr__(self, attr):
56+
if attr == 'spam':
57+
return 42
58+
else:
59+
raise AttributeError
60+
61+
import copyreg
62+
expected = (copyreg.__newobj__, (A,), (None, {'spam': 42}), None, None)
63+
assert A().__reduce_ex__(2) == expected
64+
65+
5066
def test_set_dict_attr_builtin_extension():
5167
class MyList(list):
5268
pass

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2021, Oracle and/or its affiliates.
2+
* Copyright (c) 2017, 2022, Oracle and/or its affiliates.
33
* Copyright (c) 2013, Regents of the University of California
44
*
55
* All rights reserved.
@@ -173,8 +173,8 @@
173173
import com.oracle.graal.python.builtins.objects.type.PythonClass;
174174
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
175175
import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
176-
import com.oracle.graal.python.builtins.objects.type.TypeFlags;
177176
import com.oracle.graal.python.builtins.objects.type.TypeBuiltins;
177+
import com.oracle.graal.python.builtins.objects.type.TypeFlags;
178178
import com.oracle.graal.python.builtins.objects.type.TypeNodes;
179179
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetBestBaseClassNode;
180180
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetItemsizeNode;
@@ -1626,10 +1626,11 @@ private Object callNativeGenericNewNode(Object self, Object[] varargs, PKeyword[
16261626
}
16271627
if (toSulongNodes == null) {
16281628
CompilerDirectives.transferToInterpreterAndInvalidate();
1629-
toSulongNodes = new CExtNodes.ToSulongNode[4];
1630-
for (int i = 0; i < toSulongNodes.length; i++) {
1631-
toSulongNodes[i] = insert(CExtNodesFactory.ToSulongNodeGen.create());
1629+
CExtNodes.ToSulongNode[] newToSulongNodes = new CExtNodes.ToSulongNode[4];
1630+
for (int i = 0; i < newToSulongNodes.length; i++) {
1631+
newToSulongNodes[i] = CExtNodesFactory.ToSulongNodeGen.create();
16321632
}
1633+
toSulongNodes = insert(newToSulongNodes);
16331634
}
16341635
if (asPythonObjectNode == null) {
16351636
CompilerDirectives.transferToInterpreterAndInvalidate();

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ public static SetItemNode create() {
107107
}
108108

109109
@ImportStatic({PGuards.class})
110-
public abstract static class SetValueHashingStorageNode extends PNodeWithContext {
111-
public abstract HashingStorage execute(VirtualFrame frame, HashingStorage iterator, Object value);
110+
abstract static class SetValueHashingStorageNode extends PNodeWithContext {
111+
abstract HashingStorage execute(VirtualFrame frame, HashingStorage iterator, Object value);
112112

113113
@Specialization
114114
static HashingStorage doEconomicStorage(VirtualFrame frame, EconomicMapStorage map, Object value,
@@ -139,6 +139,10 @@ protected static boolean isEconomicMapStorage(Object o) {
139139
}
140140
}
141141

142+
/**
143+
* Gets clone of the keys of the storage with all values either set to given value or with no
144+
* guarantees about the values if {@link PNone#NO_VALUE} is passed as {@code value}.
145+
*/
142146
@ImportStatic({PGuards.class, PythonOptions.class})
143147
public abstract static class GetClonedHashingStorageNode extends PNodeWithContext {
144148
@Child private PRaiseNode raise;
@@ -229,6 +233,10 @@ HashingStorage fail(Object other, @SuppressWarnings("unused") Object value) {
229233
}
230234
}
231235

236+
/**
237+
* Returns {@link HashingStorage} with the same keys as the given iterator. There is no
238+
* guarantee about the values!
239+
*/
232240
@ImportStatic({SpecialMethodNames.class, PGuards.class})
233241
public abstract static class GetHashingStorageNode extends PNodeWithContext {
234242

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ PNone init(Object self, Object[] arguments, PKeyword[] keywords,
234234
throw raise(TypeError, ErrorMessages.INIT_TAKES_ONE_ARG_OBJECT);
235235
}
236236

237-
if (overridesInit.profile(!overridesBuiltinMethod(type, profileInit, lookupNew, profileNewFactory, BuiltinConstructorsFactory.ObjectNodeFactory.class))) {
237+
if (overridesInit.profile(!overridesBuiltinMethod(type, profileNew, lookupNew, profileNewFactory, BuiltinConstructorsFactory.ObjectNodeFactory.class))) {
238238
throw raise(TypeError, ErrorMessages.INIT_TAKES_ONE_ARG, type);
239239
}
240240
}
@@ -682,7 +682,7 @@ private static Object getDescrFromBuiltinBase(Object type, GetBaseClassNode getB
682682
return null;
683683
}
684684

685-
@Specialization(guards = {"!isNoValue(mapping)", "!isDict(mapping)"})
685+
@Specialization(guards = {"!isNoValue(mapping)", "!isDict(mapping)", "!isDeleteMarker(mapping)"})
686686
Object dict(@SuppressWarnings("unused") Object self, Object mapping) {
687687
throw raise(TypeError, ErrorMessages.DICT_MUST_BE_SET_TO_DICT, mapping);
688688
}

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

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@
8080
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
8181
import com.oracle.graal.python.builtins.objects.cext.PythonNativeVoidPtr;
8282
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
83-
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
8483
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
8584
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
8685
import com.oracle.graal.python.builtins.objects.common.SequenceNodes;
@@ -90,6 +89,7 @@
9089
import com.oracle.graal.python.builtins.objects.floats.PFloat;
9190
import com.oracle.graal.python.builtins.objects.ints.PInt;
9291
import com.oracle.graal.python.builtins.objects.list.PList;
92+
import com.oracle.graal.python.builtins.objects.mappingproxy.PMappingproxy;
9393
import com.oracle.graal.python.builtins.objects.object.ObjectNodesFactory.GetFullyQualifiedNameNodeGen;
9494
import com.oracle.graal.python.builtins.objects.set.PFrozenSet;
9595
import com.oracle.graal.python.builtins.objects.str.PString;
@@ -119,18 +119,22 @@
119119
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
120120
import com.oracle.graal.python.nodes.object.IsForeignObjectNode;
121121
import com.oracle.graal.python.nodes.statement.ImportNode;
122+
import com.oracle.graal.python.nodes.subscript.GetItemNode;
122123
import com.oracle.graal.python.nodes.util.CannotCastException;
123124
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
124125
import com.oracle.graal.python.runtime.PythonContext;
125126
import com.oracle.graal.python.runtime.PythonOptions;
127+
import com.oracle.graal.python.runtime.exception.PException;
126128
import com.oracle.graal.python.runtime.object.IDUtils;
127129
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
128130
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
129131
import com.oracle.graal.python.util.PythonUtils;
130132
import com.oracle.truffle.api.Assumption;
131133
import com.oracle.truffle.api.CompilerDirectives;
134+
import com.oracle.truffle.api.dsl.Bind;
132135
import com.oracle.truffle.api.dsl.Cached;
133136
import com.oracle.truffle.api.dsl.Cached.Shared;
137+
import com.oracle.truffle.api.dsl.Fallback;
134138
import com.oracle.truffle.api.dsl.GenerateUncached;
135139
import com.oracle.truffle.api.dsl.ImportStatic;
136140
import com.oracle.truffle.api.dsl.Specialization;
@@ -524,21 +528,59 @@ Pair<Object, Object> doHasNeither(@SuppressWarnings("unused") PNone getNewArgsEx
524528

525529
@ImportStatic({PythonOptions.class, PGuards.class})
526530
abstract static class GetSlotNamesNode extends Node {
527-
public abstract Object execute(VirtualFrame frame, Object cls, Object copyReg);
531+
@Child PyObjectLookupAttr lookupAttr = PyObjectLookupAttr.create();
532+
533+
public final Object execute(VirtualFrame frame, Object cls, Object copyReg) {
534+
Object clsDict = lookupAttr.execute(frame, cls, __DICT__);
535+
return executeInternal(frame, cls, clsDict, copyReg);
536+
}
537+
538+
abstract Object executeInternal(VirtualFrame frame, Object cls, Object clsDict, Object copyReg);
528539

529540
@Specialization
530-
Object dispatch(VirtualFrame frame, Object cls, Object copyReg,
531-
@Cached GetSlotNamesInternalNode getSlotNamesInternalNode,
532-
@Cached HashingCollectionNodes.GetHashingStorageNode getHashingStorageNode,
533-
@Cached PyObjectLookupAttr lookupAttr,
534-
@CachedLibrary(limit = "1") HashingStorageLibrary hashLib) {
541+
Object dispatchDict(VirtualFrame frame, Object cls, PDict clsDict, Object copyReg,
542+
@Shared("internal") @Cached GetSlotNamesInternalNode getSlotNamesInternalNode,
543+
@Shared("storageLib") @CachedLibrary(limit = "3") HashingStorageLibrary storageLib) {
544+
Object slotNames = storageLib.getItem(clsDict.getDictStorage(), __SLOTNAMES__);
545+
slotNames = slotNames == null ? PNone.NO_VALUE : slotNames;
546+
return getSlotNamesInternalNode.execute(frame, cls, copyReg, slotNames);
547+
}
548+
549+
// Fast paths for a common case of PMappingproxy and NO_VALUE
550+
@Specialization(guards = "isDict(mapping)")
551+
Object dispatchMappingProxy(VirtualFrame frame, Object cls, @SuppressWarnings("unused") PMappingproxy clsDict, Object copyReg,
552+
@Shared("internal") @Cached GetSlotNamesInternalNode getSlotNamesInternalNode,
553+
@Bind("clsDict.getMapping()") Object mapping,
554+
@Shared("storageLib") @CachedLibrary(limit = "3") HashingStorageLibrary storageLib) {
555+
PDict mappingDict = (PDict) mapping;
556+
return dispatchDict(frame, cls, mappingDict, copyReg, getSlotNamesInternalNode, storageLib);
557+
}
558+
559+
@Specialization(guards = "isNoValue(noValue)")
560+
Object dispatchNoValue(VirtualFrame frame, Object cls, PNone noValue, Object copyReg,
561+
@Shared("internal") @Cached GetSlotNamesInternalNode getSlotNamesInternalNode) {
562+
return getSlotNamesInternalNode.execute(frame, cls, copyReg, noValue);
563+
}
564+
565+
@Fallback
566+
Object dispatchGeneric(VirtualFrame frame, Object cls, Object clsDict, Object copyReg,
567+
@Shared("internal") @Cached GetSlotNamesInternalNode getSlotNamesInternalNode,
568+
@Cached GetItemNode getItemNode,
569+
@Cached IsBuiltinClassProfile isBuiltinClassProfile) {
570+
/*
571+
* CPython looks at tp_dict of the type and assumes that it must be a builtin
572+
* dictionary, otherwise it fails with type error. We do not have tp_dict for managed
573+
* classes and what comes here is __dict__, so among other things it may be a mapping
574+
* proxy, but also tp_dict for native classes. We "over-approximate" what CPython does
575+
* here a bit and just use __getitem__.
576+
*/
577+
535578
Object slotNames = PNone.NO_VALUE;
536-
Object clsDict = lookupAttr.execute(frame, cls, __DICT__);
537579
if (!PGuards.isNoValue(clsDict)) {
538-
HashingStorage hashingStorage = getHashingStorageNode.execute(frame, clsDict);
539-
Object item = hashLib.getItem(hashingStorage, __SLOTNAMES__);
540-
if (item != null) {
541-
slotNames = item;
580+
try {
581+
slotNames = getItemNode.execute(frame, clsDict, __SLOTNAMES__);
582+
} catch (PException ex) {
583+
ex.expect(PythonBuiltinClassType.KeyError, isBuiltinClassProfile);
542584
}
543585
}
544586
return getSlotNamesInternalNode.execute(frame, cls, copyReg, slotNames);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/superobject/SuperBuiltins.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -312,7 +312,7 @@ private Object getClassFromTarget(VirtualFrame frame, PFrame target, HashingStor
312312
private CellBuiltins.GetRefNode getGetRefNode() {
313313
if (getRefNode == null) {
314314
CompilerDirectives.transferToInterpreterAndInvalidate();
315-
getRefNode = CellBuiltins.GetRefNode.create();
315+
getRefNode = insert(CellBuiltins.GetRefNode.create());
316316
}
317317
return getRefNode;
318318
}

0 commit comments

Comments
 (0)