Skip to content

Commit e7f6cce

Browse files
tomasstupkatomasstupka
committed
use HiddenPythonKey instead of truffle HiddenKey for slot keys.
HiddenKey has comparison based on object identity, but slots have to be interchangeable between classes - e.g. when changing the __class__ value.
1 parent a6e5044 commit e7f6cce

File tree

8 files changed

+135
-29
lines changed

8 files changed

+135
-29
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
import com.oracle.graal.python.builtins.objects.function.PFunction;
131131
import com.oracle.graal.python.builtins.objects.function.PKeyword;
132132
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenKeyDescriptor;
133+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
133134
import com.oracle.graal.python.builtins.objects.ints.PInt;
134135
import com.oracle.graal.python.builtins.objects.iterator.PZip;
135136
import com.oracle.graal.python.builtins.objects.list.PList;
@@ -218,7 +219,6 @@
218219
import com.oracle.truffle.api.library.CachedLibrary;
219220
import com.oracle.truffle.api.nodes.ExplodeLoop;
220221
import com.oracle.truffle.api.nodes.UnexpectedResultException;
221-
import com.oracle.truffle.api.object.HiddenKey;
222222
import com.oracle.truffle.api.profiles.BranchProfile;
223223
import com.oracle.truffle.api.profiles.ConditionProfile;
224224

@@ -2337,7 +2337,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
23372337
} else {
23382338
// TODO: check for __weakref__
23392339
// TODO avoid if native slots are inherited
2340-
HiddenKey hiddenSlotKey = new HiddenKey(slotName);
2340+
HiddenPythonKey hiddenSlotKey = new HiddenPythonKey(slotName);
23412341
HiddenKeyDescriptor slotDesc = factory().createHiddenKeyDescriptor(hiddenSlotKey, pythonClass);
23422342
pythonClass.setAttribute(slotName, slotDesc);
23432343
}

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

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.ForEachNode;
4848
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
4949
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
50+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
5051
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
5152
import com.oracle.graal.python.builtins.objects.str.PString;
5253
import com.oracle.graal.python.nodes.PGuards;
@@ -72,6 +73,7 @@
7273
import com.oracle.truffle.api.object.Shape;
7374
import com.oracle.truffle.api.profiles.BranchProfile;
7475
import com.oracle.truffle.api.profiles.ConditionProfile;
76+
import java.util.ArrayList;
7577
import java.util.List;
7678

7779
/**
@@ -101,8 +103,30 @@ public DynamicObjectStorage(DynamicObject store, MroSequenceStorage mro) {
101103
this.mro = mro;
102104
}
103105

104-
protected static Object[] keyList(DynamicObjectStorage self) {
105-
return self.store.getShape().getKeyList().toArray();
106+
protected static Object[] keyArray(DynamicObjectStorage self) {
107+
return DynamicObjectStorage.keyArray(self.store.getShape());
108+
}
109+
110+
protected static Object[] keyArray(Shape shape) {
111+
List<Object> keyList = keyList(shape);
112+
return keyList.toArray(new Object[keyList.size()]);
113+
}
114+
115+
protected static List<Object> keyList(Shape shape) {
116+
return filter(shape.getKeyList());
117+
}
118+
119+
@TruffleBoundary
120+
private static List<Object> filter(List<Object> l) {
121+
ArrayList<Object> keyList = new ArrayList<>(l.size());
122+
Iterator<Object> it = l.iterator();
123+
while (it.hasNext()) {
124+
Object n = it.next();
125+
if (!(n instanceof HiddenPythonKey)) {
126+
keyList.add(n);
127+
}
128+
}
129+
return keyList;
106130
}
107131

108132
@ExportMessage
@@ -112,7 +136,7 @@ static class Length {
112136
@ExplodeLoop
113137
static int cachedLen(DynamicObjectStorage self,
114138
@SuppressWarnings("unused") @Cached("self.store.getShape()") Shape cachedShape,
115-
@Cached(value = "keyList(self)", dimensions = 1) Object[] keys,
139+
@Cached(value = "keyArray(self)", dimensions = 1) Object[] keys,
116140
@Cached ReadAttributeFromDynamicObjectNode readNode) {
117141
int len = 0;
118142
for (int i = 0; i < keys.length; i++) {
@@ -126,7 +150,7 @@ static int cachedLen(DynamicObjectStorage self,
126150
@Specialization(replaces = "cachedLen")
127151
static int length(DynamicObjectStorage self,
128152
@Exclusive @Cached ReadAttributeFromDynamicObjectNode readNode) {
129-
return cachedLen(self, self.store.getShape(), keyList(self), readNode);
153+
return cachedLen(self, self.store.getShape(), keyArray(self), readNode);
130154
}
131155

132156
private static boolean hasStringKey(DynamicObjectStorage self, String key, ReadAttributeFromDynamicObjectNode readNode) {
@@ -169,7 +193,7 @@ static Object pstring(DynamicObjectStorage self, PString key, ThreadState state,
169193
static Object notString(DynamicObjectStorage self, Object key, ThreadState state,
170194
@Shared("readKey") @Cached ReadAttributeFromDynamicObjectNode readKey,
171195
@Exclusive @Cached("self.store.getShape()") Shape cachedShape,
172-
@Exclusive @Cached(value = "cachedShape.getKeyList().toArray()", dimensions = 1) Object[] keyList,
196+
@Exclusive @Cached(value = "keyArray(cachedShape)", dimensions = 1) Object[] keyList,
173197
@Shared("builtinStringProfile") @Cached IsBuiltinClassProfile profile,
174198
@CachedLibrary(limit = "2") PythonObjectLibrary lib,
175199
@Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState,
@@ -310,7 +334,7 @@ static class ForEachUntyped {
310334
@ExplodeLoop
311335
static Object cachedLen(DynamicObjectStorage self, ForEachNode<Object> node, Object firstValue,
312336
@Exclusive @SuppressWarnings("unused") @Cached("self.store.getShape()") Shape cachedShape,
313-
@Cached(value = "keyList(self)", dimensions = 1) Object[] keys,
337+
@Cached(value = "keyArray(self)", dimensions = 1) Object[] keys,
314338
@Shared("readNodeInject") @Cached ReadAttributeFromDynamicObjectNode readNode) {
315339
Object result = firstValue;
316340
for (int i = 0; i < keys.length; i++) {
@@ -323,7 +347,7 @@ static Object cachedLen(DynamicObjectStorage self, ForEachNode<Object> node, Obj
323347
@Specialization(replaces = "cachedLen")
324348
static Object addAll(DynamicObjectStorage self, ForEachNode<Object> node, Object firstValue,
325349
@Shared("readNodeInject") @Cached ReadAttributeFromDynamicObjectNode readNode) {
326-
return cachedLen(self, node, firstValue, self.store.getShape(), keyList(self), readNode);
350+
return cachedLen(self, node, firstValue, self.store.getShape(), keyArray(self), readNode);
327351
}
328352

329353
private static Object runNode(DynamicObjectStorage self, Object key, Object acc, ReadAttributeFromDynamicObjectNode readNode, ForEachNode<Object> node) {
@@ -410,7 +434,7 @@ private static final class KeysIterator extends AbstractKeysIterator {
410434

411435
public KeysIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode readNode) {
412436
super(store, readNode);
413-
this.keyIter = store.getShape().getKeys().iterator();
437+
this.keyIter = keyList(store.getShape()).iterator();
414438
}
415439

416440
@Override
@@ -430,7 +454,7 @@ private static final class ReverseKeysIterator extends AbstractKeysIterator {
430454

431455
public ReverseKeysIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode readNode) {
432456
super(store, readNode);
433-
this.keyList = store.getShape().getKeyList();
457+
keyList = keyList(store.getShape());
434458
this.index = keyList.size() - 1;
435459
}
436460

@@ -466,7 +490,7 @@ public EntriesIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode r
466490

467491
@TruffleBoundary
468492
private static Iterator<Object> getIterator(Shape shape) {
469-
return shape.getKeys().iterator();
493+
return keyList(shape).iterator();
470494
}
471495

472496
@TruffleBoundary

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/getsetdescriptor/HiddenKeyDescriptor.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,18 @@
4242

4343
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4444
import com.oracle.graal.python.builtins.objects.object.PythonBuiltinObject;
45-
import com.oracle.truffle.api.object.HiddenKey;
4645

4746
public final class HiddenKeyDescriptor extends PythonBuiltinObject {
48-
private final HiddenKey key;
47+
private final HiddenPythonKey key;
4948
private final Object type;
5049

51-
public HiddenKeyDescriptor(HiddenKey key, Object type) {
50+
public HiddenKeyDescriptor(HiddenPythonKey key, Object type) {
5251
super(PythonBuiltinClassType.GetSetDescriptor, PythonBuiltinClassType.GetSetDescriptor.newInstance());
5352
this.key = key;
5453
this.type = type;
5554
}
5655

57-
public HiddenKey getKey() {
56+
public HiddenPythonKey getKey() {
5857
return key;
5958
}
6059

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/getsetdescriptor/HiddenPythonKey.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.graal.python.builtins.objects.getsetdescriptor;
42+
43+
import com.oracle.truffle.api.object.HiddenKey;
44+
45+
/**
46+
* Use instead of {@link HiddenKey} which does comparison based on object identity.
47+
*/
48+
public class HiddenPythonKey {
49+
private final String name;
50+
51+
public HiddenPythonKey(String name) {
52+
assert name != null;
53+
this.name = name;
54+
}
55+
56+
public String getName() {
57+
return name;
58+
}
59+
60+
@Override
61+
public String toString() {
62+
return name;
63+
}
64+
65+
@Override
66+
public boolean equals(Object obj) {
67+
if (this == obj) {
68+
return true;
69+
}
70+
if (obj == null || getClass() != obj.getClass()) {
71+
return false;
72+
}
73+
final HiddenPythonKey other = (HiddenPythonKey) obj;
74+
return name.equals(other.name);
75+
}
76+
77+
@Override
78+
public int hashCode() {
79+
return name.hashCode();
80+
}
81+
82+
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonBuiltinClass.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import com.oracle.truffle.api.library.CachedLibrary;
3939
import com.oracle.truffle.api.library.ExportLibrary;
4040
import com.oracle.truffle.api.library.ExportMessage;
41-
import com.oracle.truffle.api.object.HiddenKey;
41+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
4242

4343
/**
4444
* A Python built-in class that is immutable.
@@ -55,7 +55,7 @@ public PythonBuiltinClass(PythonBuiltinClassType builtinClass, PythonAbstractCla
5555
@Override
5656
public void setAttribute(Object name, Object value) {
5757
CompilerAsserts.neverPartOfCompilation();
58-
if (name instanceof HiddenKey || !PythonLanguage.getCore().isInitialized()) {
58+
if (name instanceof HiddenPythonKey || !PythonLanguage.getCore().isInitialized()) {
5959
setAttributeUnsafe(name, value);
6060
} else {
6161
throw PythonLanguage.getCore().raise(TypeError, ErrorMessages.CANT_SET_ATTRIBUTES_OF_TYPE_S, this);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeBuiltins.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,19 +128,19 @@
128128
import com.oracle.truffle.api.interop.InteropLibrary;
129129
import com.oracle.truffle.api.interop.UnsupportedMessageException;
130130
import com.oracle.truffle.api.library.CachedLibrary;
131-
import com.oracle.truffle.api.object.HiddenKey;
131+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
132132
import com.oracle.truffle.api.profiles.BranchProfile;
133133
import com.oracle.truffle.api.profiles.ConditionProfile;
134134

135135
@CoreFunctions(extendClasses = PythonBuiltinClassType.PythonClass)
136136
public class TypeBuiltins extends PythonBuiltins {
137137

138-
public static final HiddenKey TYPE_DICTOFFSET = new HiddenKey(__DICTOFFSET__);
139-
public static final HiddenKey TYPE_ITEMSIZE = new HiddenKey(__ITEMSIZE__);
140-
public static final HiddenKey TYPE_BASICSIZE = new HiddenKey(__BASICSIZE__);
141-
public static final HiddenKey TYPE_ALLOC = new HiddenKey(__ALLOC__);
142-
public static final HiddenKey TYPE_DEALLOC = new HiddenKey("__dealloc__");
143-
public static final HiddenKey TYPE_FREE = new HiddenKey("__free__");
138+
public static final HiddenPythonKey TYPE_DICTOFFSET = new HiddenPythonKey(__DICTOFFSET__);
139+
public static final HiddenPythonKey TYPE_ITEMSIZE = new HiddenPythonKey(__ITEMSIZE__);
140+
public static final HiddenPythonKey TYPE_BASICSIZE = new HiddenPythonKey(__BASICSIZE__);
141+
public static final HiddenPythonKey TYPE_ALLOC = new HiddenPythonKey(__ALLOC__);
142+
public static final HiddenPythonKey TYPE_DEALLOC = new HiddenPythonKey("__dealloc__");
143+
public static final HiddenPythonKey TYPE_FREE = new HiddenPythonKey("__free__");
144144

145145
@Override
146146
protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFactories() {

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2020, 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
@@ -53,6 +53,7 @@
5353
import com.oracle.graal.python.runtime.PythonOptions;
5454
import com.oracle.truffle.api.dsl.ImportStatic;
5555
import com.oracle.truffle.api.nodes.NodeCost;
56+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
5657
import com.oracle.truffle.api.object.HiddenKey;
5758
import com.oracle.truffle.api.object.Location;
5859
import com.oracle.truffle.api.object.Property;
@@ -77,7 +78,7 @@ protected static Location getLocationOrNull(Property prop) {
7778
}
7879

7980
protected static boolean isHiddenKey(Object key) {
80-
return key instanceof HiddenKey;
81+
return key instanceof HiddenPythonKey || key instanceof HiddenKey;
8182
}
8283

8384
@Override

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PythonObjectFactory.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@
152152
import com.oracle.truffle.api.interop.TruffleObject;
153153
import com.oracle.truffle.api.nodes.Node;
154154
import com.oracle.truffle.api.object.DynamicObject;
155-
import com.oracle.truffle.api.object.HiddenKey;
155+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
156156
import com.oracle.truffle.api.object.Shape;
157157

158158
@GenerateUncached
@@ -443,7 +443,7 @@ public GetSetDescriptor createGetSetDescriptor(Object get, Object set, String na
443443
return trace(new GetSetDescriptor(get, set, name, type, allowsDelete));
444444
}
445445

446-
public HiddenKeyDescriptor createHiddenKeyDescriptor(HiddenKey key, Object type) {
446+
public HiddenKeyDescriptor createHiddenKeyDescriptor(HiddenPythonKey key, Object type) {
447447
return trace(new HiddenKeyDescriptor(key, type));
448448
}
449449

0 commit comments

Comments
 (0)