Skip to content

Commit cd0c18a

Browse files
msimacekmsimacek
committed
Fix set/frozenset reverse binary operations
1 parent 61dbcbf commit cd0c18a

File tree

5 files changed

+122
-207
lines changed

5 files changed

+122
-207
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonBuiltinClassType.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,6 @@
160160
import com.oracle.graal.python.builtins.objects.property.PropertyBuiltins;
161161
import com.oracle.graal.python.builtins.objects.range.RangeBuiltins;
162162
import com.oracle.graal.python.builtins.objects.set.BaseSetBuiltins;
163-
import com.oracle.graal.python.builtins.objects.set.FrozenSetBuiltins;
164-
import com.oracle.graal.python.builtins.objects.set.SetBuiltins;
165163
import com.oracle.graal.python.builtins.objects.str.StringBuiltins;
166164
import com.oracle.graal.python.builtins.objects.superobject.SuperBuiltins;
167165
import com.oracle.graal.python.builtins.objects.thread.ThreadLocalBuiltins;
@@ -242,7 +240,7 @@ public enum PythonBuiltinClassType implements TruffleObject {
242240
PMap("map", J_BUILTINS),
243241
PFloat("float", J_BUILTINS, FLOAT_M_FLAGS, FloatBuiltins.SLOTS),
244242
PFrame("frame", Flags.PRIVATE_DERIVED_WODICT),
245-
PFrozenSet("frozenset", J_BUILTINS, FROZENSET_M_FLAGS, TpSlots.merge(BaseSetBuiltins.SLOTS, FrozenSetBuiltins.SLOTS)),
243+
PFrozenSet("frozenset", J_BUILTINS, FROZENSET_M_FLAGS, BaseSetBuiltins.SLOTS),
246244
PFunction("function", Flags.PRIVATE_DERIVED_WDICT, FunctionBuiltins.SLOTS),
247245
PGenerator("generator", Flags.PRIVATE_DERIVED_WODICT, GENERATOR_M_FLAGS),
248246
PCoroutine("coroutine", Flags.PRIVATE_DERIVED_WODICT, COROUTINE_M_FLAGS),
@@ -266,7 +264,7 @@ public enum PythonBuiltinClassType implements TruffleObject {
266264
PReferenceType("ReferenceType", "_weakref"),
267265
PSentinelIterator("callable_iterator", Flags.PRIVATE_DERIVED_WODICT),
268266
PReverseIterator("reversed", J_BUILTINS),
269-
PSet("set", J_BUILTINS, SET_M_FLAGS, TpSlots.merge(BaseSetBuiltins.SLOTS, SetBuiltins.SLOTS)),
267+
PSet("set", J_BUILTINS, SET_M_FLAGS, BaseSetBuiltins.SLOTS),
270268
PSlice("slice", J_BUILTINS),
271269
PString("str", J_BUILTINS, STRING_M_FLAGS, StringBuiltins.SLOTS),
272270
PTraceback("traceback"),

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/set/BaseSetBuiltins.java

Lines changed: 95 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@
104104
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
105105
import com.oracle.truffle.api.dsl.Bind;
106106
import com.oracle.truffle.api.dsl.Cached;
107+
import com.oracle.truffle.api.dsl.Cached.Shared;
107108
import com.oracle.truffle.api.dsl.Fallback;
108109
import com.oracle.truffle.api.dsl.GenerateCached;
109110
import com.oracle.truffle.api.dsl.GenerateInline;
@@ -274,22 +275,34 @@ static boolean contains(VirtualFrame frame, PBaseSet self, Object key,
274275
}
275276
}
276277

278+
@GenerateInline
279+
@GenerateCached(false)
280+
abstract static class CreateSetNode extends Node {
281+
public abstract PBaseSet execute(Node inliningTarget, HashingStorage storage, PBaseSet template);
282+
283+
@Specialization
284+
static PBaseSet doSet(HashingStorage storage, @SuppressWarnings("unused") PSet template,
285+
@Shared @Cached(inline = false) PythonObjectFactory factory) {
286+
return factory.createSet(storage);
287+
}
288+
289+
@Specialization
290+
static PBaseSet doFrozenSet(HashingStorage storage, @SuppressWarnings("unused") PFrozenSet template,
291+
@Shared @Cached(inline = false) PythonObjectFactory factory) {
292+
return factory.createFrozenSet(storage);
293+
}
294+
}
295+
277296
@Slot(value = SlotKind.nb_subtract, isComplex = true)
278297
@GenerateNodeFactory
279298
abstract static class SubNode extends BinaryOpBuiltinNode {
280299
@Specialization
281300
static PBaseSet doPBaseSet(@SuppressWarnings("unused") VirtualFrame frame, PBaseSet left, PBaseSet right,
282301
@Bind("this") Node inliningTarget,
283-
@Cached InlinedConditionProfile leftProfile,
284-
@Cached HashingCollectionNodes.GetSetStorageNode getSetStorageNode,
285302
@Cached HashingStorageNodes.HashingStorageDiff diffNode,
286-
@Cached PythonObjectFactory factory) {
287-
HashingStorage storage = diffNode.execute(frame, inliningTarget, left.getDictStorage(), getSetStorageNode.execute(frame, inliningTarget, right));
288-
if (leftProfile.profile(inliningTarget, left instanceof PFrozenSet)) {
289-
return factory.createFrozenSet(storage);
290-
} else {
291-
return factory.createSet(storage);
292-
}
303+
@Cached CreateSetNode createSetNode) {
304+
HashingStorage storage = diffNode.execute(frame, inliningTarget, left.getDictStorage(), right.getDictStorage());
305+
return createSetNode.execute(inliningTarget, storage, left);
293306
}
294307

295308
@Fallback
@@ -299,6 +312,79 @@ static Object doOther(Object self, Object other) {
299312
}
300313
}
301314

315+
@Slot(value = SlotKind.nb_and, isComplex = true)
316+
@GenerateNodeFactory
317+
public abstract static class AndNode extends BinaryOpBuiltinNode {
318+
319+
@Specialization
320+
static PBaseSet doPBaseSet(VirtualFrame frame, PBaseSet self, PBaseSet other,
321+
@Bind("this") Node inliningTarget,
322+
@Cached HashingStorageNodes.HashingStorageLen lenNode,
323+
@Cached HashingStorageNodes.HashingStorageIntersect intersectNode,
324+
@Cached InlinedConditionProfile swapProfile,
325+
@Cached CreateSetNode createSetNode) {
326+
HashingStorage storage1 = self.getDictStorage();
327+
HashingStorage storage2 = other.getDictStorage();
328+
// Try to minimize the number of __eq__ calls
329+
if (swapProfile.profile(inliningTarget, lenNode.execute(inliningTarget, storage2) > lenNode.execute(inliningTarget, storage1))) {
330+
HashingStorage tmp = storage1;
331+
storage1 = storage2;
332+
storage2 = tmp;
333+
}
334+
HashingStorage storage = intersectNode.execute(frame, inliningTarget, storage2, storage1);
335+
return createSetNode.execute(inliningTarget, storage, self);
336+
}
337+
338+
@SuppressWarnings("unused")
339+
@Fallback
340+
Object doOther(Object self, Object other) {
341+
return PNotImplemented.NOT_IMPLEMENTED;
342+
}
343+
}
344+
345+
@Slot(value = SlotKind.nb_or, isComplex = true)
346+
@GenerateNodeFactory
347+
public abstract static class OrNode extends BinaryOpBuiltinNode {
348+
@Specialization
349+
static Object doSet(VirtualFrame frame, PBaseSet self, PBaseSet other,
350+
@Bind("this") Node inliningTarget,
351+
@Cached HashingStorageCopy copyStorage,
352+
@Cached HashingStorageNodes.HashingStorageAddAllToOther addAllToOther,
353+
@Cached CreateSetNode createSetNode) {
354+
HashingStorage resultStorage = copyStorage.execute(inliningTarget, self.getDictStorage());
355+
resultStorage = addAllToOther.execute(frame, inliningTarget, other.getDictStorage(), resultStorage);
356+
return createSetNode.execute(inliningTarget, resultStorage, self);
357+
}
358+
359+
@SuppressWarnings("unused")
360+
@Fallback
361+
Object doOther(Object self, Object other) {
362+
return PNotImplemented.NOT_IMPLEMENTED;
363+
}
364+
}
365+
366+
@Slot(value = SlotKind.nb_xor, isComplex = true)
367+
@GenerateNodeFactory
368+
@ImportStatic(PGuards.class)
369+
public abstract static class XorNode extends BinaryOpBuiltinNode {
370+
371+
@Specialization
372+
static Object doSet(VirtualFrame frame, PBaseSet self, PBaseSet other,
373+
@Bind("this") Node inliningTarget,
374+
@Cached HashingStorageNodes.HashingStorageXor xorNode,
375+
@Cached CreateSetNode createSetNode) {
376+
// TODO: calls __eq__ wrong number of times compared to CPython (GR-42240)
377+
HashingStorage storage = xorNode.execute(frame, inliningTarget, self.getDictStorage(), other.getDictStorage());
378+
return createSetNode.execute(inliningTarget, storage, self);
379+
}
380+
381+
@SuppressWarnings("unused")
382+
@Fallback
383+
Object doOther(Object self, Object other) {
384+
return PNotImplemented.NOT_IMPLEMENTED;
385+
}
386+
}
387+
302388
@Builtin(name = "issubset", minNumOfPositionalArgs = 2)
303389
@GenerateNodeFactory
304390
protected abstract static class BaseIsSubsetNode extends PythonBinaryBuiltinNode {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/set/FrozenSetBuiltins.java

Lines changed: 0 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929

3030
import java.util.List;
3131

32-
import com.oracle.graal.python.annotations.Slot;
33-
import com.oracle.graal.python.annotations.Slot.SlotKind;
3432
import com.oracle.graal.python.builtins.Builtin;
3533
import com.oracle.graal.python.builtins.CoreFunctions;
3634
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
@@ -49,38 +47,29 @@
4947
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorKey;
5048
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorNext;
5149
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageXor;
52-
import com.oracle.graal.python.builtins.objects.type.TpSlots;
53-
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotBinaryOp.BinaryOpBuiltinNode;
5450
import com.oracle.graal.python.lib.PyObjectHashNode;
55-
import com.oracle.graal.python.nodes.ErrorMessages;
5651
import com.oracle.graal.python.nodes.PGuards;
57-
import com.oracle.graal.python.nodes.PRaiseNode;
5852
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5953
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
6054
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
6155
import com.oracle.graal.python.nodes.object.BuiltinClassProfiles.IsAnyBuiltinObjectProfile;
62-
import com.oracle.graal.python.runtime.exception.PythonErrorType;
6356
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
6457
import com.oracle.truffle.api.dsl.Bind;
6558
import com.oracle.truffle.api.dsl.Cached;
6659
import com.oracle.truffle.api.dsl.Cached.Shared;
6760
import com.oracle.truffle.api.dsl.Fallback;
6861
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
69-
import com.oracle.truffle.api.dsl.ImportStatic;
7062
import com.oracle.truffle.api.dsl.NodeFactory;
7163
import com.oracle.truffle.api.dsl.Specialization;
7264
import com.oracle.truffle.api.frame.VirtualFrame;
7365
import com.oracle.truffle.api.nodes.Node;
74-
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
7566

7667
/**
7768
* binary operations are implemented in {@link BaseSetBuiltins}
7869
*/
7970
@CoreFunctions(extendClasses = {PythonBuiltinClassType.PFrozenSet})
8071
public final class FrozenSetBuiltins extends PythonBuiltins {
8172

82-
public static final TpSlots SLOTS = FrozenSetBuiltinsSlotsGen.SLOTS;
83-
8473
@Override
8574
protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFactories() {
8675
return FrozenSetBuiltinsFactory.getFactories();
@@ -103,33 +92,6 @@ static PFrozenSet subFrozensetIdentity(PFrozenSet arg,
10392
}
10493
}
10594

106-
@Slot(value = SlotKind.nb_and, isComplex = true)
107-
@GenerateNodeFactory
108-
@ImportStatic(PGuards.class)
109-
abstract static class AndNode extends BinaryOpBuiltinNode {
110-
111-
@Specialization(guards = "canDoSetBinOp(right)")
112-
static PBaseSet doPBaseSet(@SuppressWarnings("unused") VirtualFrame frame, PFrozenSet left, Object right,
113-
@Bind("this") Node inliningTarget,
114-
@Cached InlinedConditionProfile rightIsSetProfile,
115-
@Cached GetSetStorageNode getSetStorageNode,
116-
@Cached HashingStorageIntersect intersectNode,
117-
@Cached PythonObjectFactory factory) {
118-
HashingStorage storage = intersectNode.execute(frame, inliningTarget, left.getDictStorage(), getSetStorageNode.execute(frame, inliningTarget, right));
119-
if (rightIsSetProfile.profile(inliningTarget, right instanceof PBaseSet)) {
120-
return factory.createFrozenSet(storage);
121-
} else {
122-
return factory.createSet(storage);
123-
}
124-
}
125-
126-
@Fallback
127-
static Object doAnd(Object self, Object other,
128-
@Cached PRaiseNode raiseNode) {
129-
throw raiseNode.raise(PythonErrorType.TypeError, ErrorMessages.UNSUPPORTED_OPERAND_TYPES_FOR_S_P_AND_P, "&", self, other);
130-
}
131-
}
132-
13395
@Builtin(name = "intersection", minNumOfPositionalArgs = 1, takesVarArgs = true)
13496
@GenerateNodeFactory
13597
public abstract static class IntersectNode extends PythonBuiltinNode {
@@ -184,62 +146,6 @@ static PFrozenSet doSet(@SuppressWarnings("unused") VirtualFrame frame, PFrozenS
184146
}
185147
}
186148

187-
@Slot(value = SlotKind.nb_or, isComplex = true)
188-
@GenerateNodeFactory
189-
@ImportStatic(PGuards.class)
190-
abstract static class OrNode extends BinaryOpBuiltinNode {
191-
192-
@Specialization(guards = "canDoSetBinOp(right)")
193-
static PBaseSet doPBaseSet(@SuppressWarnings("unused") VirtualFrame frame, PFrozenSet left, Object right,
194-
@Bind("this") Node inliningTarget,
195-
@Cached InlinedConditionProfile rightIsSetProfile,
196-
@Cached HashingCollectionNodes.GetSetStorageNode getSetStorageNode,
197-
@Cached HashingStorageCopy copyNode,
198-
@Cached HashingStorageAddAllToOther addAllToOther,
199-
@Cached PythonObjectFactory factory) {
200-
HashingStorage storage = left.getDictStorage().union(inliningTarget, getSetStorageNode.execute(frame, inliningTarget, right), copyNode, addAllToOther);
201-
if (rightIsSetProfile.profile(inliningTarget, right instanceof PBaseSet)) {
202-
return factory.createFrozenSet(storage);
203-
} else {
204-
return factory.createSet(storage);
205-
}
206-
}
207-
208-
@Fallback
209-
static Object doOr(Object self, Object other,
210-
@Cached PRaiseNode raiseNode) {
211-
throw raiseNode.raise(PythonErrorType.TypeError, ErrorMessages.UNSUPPORTED_OPERAND_TYPES_FOR_S_P_AND_P, "|", self, other);
212-
}
213-
}
214-
215-
@Slot(value = SlotKind.nb_xor, isComplex = true)
216-
@GenerateNodeFactory
217-
@ImportStatic(PGuards.class)
218-
abstract static class XorNode extends BinaryOpBuiltinNode {
219-
220-
@Specialization(guards = "canDoSetBinOp(right)")
221-
static PBaseSet doPBaseSet(@SuppressWarnings("unused") VirtualFrame frame, PFrozenSet left, Object right,
222-
@Bind("this") Node inliningTarget,
223-
@Cached InlinedConditionProfile rightIsSetProfile,
224-
@Cached GetSetStorageNode getSetStorageNode,
225-
@Cached HashingStorageXor xorNode,
226-
@Cached PythonObjectFactory factory) {
227-
HashingStorage rightStorage = getSetStorageNode.execute(frame, inliningTarget, right);
228-
HashingStorage storage = xorNode.execute(frame, inliningTarget, left.getDictStorage(), rightStorage);
229-
if (rightIsSetProfile.profile(inliningTarget, right instanceof PBaseSet)) {
230-
return factory.createFrozenSet(storage);
231-
} else {
232-
return factory.createSet(storage);
233-
}
234-
}
235-
236-
@Fallback
237-
static Object doOr(Object self, Object other,
238-
@Cached PRaiseNode raiseNode) {
239-
throw raiseNode.raise(PythonErrorType.TypeError, ErrorMessages.UNSUPPORTED_OPERAND_TYPES_FOR_S_P_AND_P, "^", self, other);
240-
}
241-
}
242-
243149
@Builtin(name = "symmetric_difference", minNumOfPositionalArgs = 2)
244150
@GenerateNodeFactory
245151
public abstract static class SymmetricDifferenceNode extends PythonBuiltinNode {

0 commit comments

Comments
 (0)