[GR-42240] Fix set and dict view xor side effects.

graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/builtin/HashingTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2026, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -197,8 +197,9 @@ public void dictEqualTest3() {
 
     @Test
     public void setAndTest() {
-        assertPrints("{2}\n", "print({2, 3} ^ {3})\n");
-        assertPrints("{'c', 'b'}\n", "print({'a', 'c'} ^ frozenset({'a', 'b'}))\n");
-        assertPrints("frozenset({'b'})\n", "print(frozenset({'a', 'c'}) ^ {'a', 'b', 'c'})\n");
+        String source = "assert {2, 3} ^ {3} == {2}\n" +
+                        "assert {'a', 'c'} ^ frozenset({'a', 'b'}) == {'b', 'c'}\n" +
+                        "assert frozenset({'a', 'c'}) ^ {'a', 'b', 'c'} == frozenset({'b'})\n";
+        assertPrints("", source);
     }
 }

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

@@ -1549,6 +1549,46 @@ def test_eq_side_effects():
     assert key.eq_calls == 1
 
 
+def test_keys_xor_side_effects():
+    key1 = TrackingKey('foo')
+    key2 = TrackingKey('foo')
+    d1 = {key1: 1}
+    d2 = {key2: 2}
+    key1.clear_observations()
+    key2.clear_observations()
+
+    assert d1.keys() ^ d2.keys() == set()
+    assert key1.eq_calls == 1
+    assert key1.hash_calls == 0
+    assert key2.eq_calls == 0
+    assert key2.hash_calls == 1
+
+
+def test_items_xor_side_effects():
+    log = []
+
+    class Key:
+        def __init__(self, name):
+            self.name = name
+
+        def __hash__(self):
+            log.append(("hash", self.name))
+            return 42
+
+        def __eq__(self, other):
+            log.append(("eq", self.name, other.name))
+            return True
+
+    key1 = Key('left')
+    key2 = Key('right')
+    d1 = {key1: 1}
+    d2 = {key2: 1}
+    log.clear()
+
+    assert d1.items() ^ d2.items() == set()
+    assert log == [("eq", "left", "right"), ("eq", "left", "right")]
+
+
 # TODO: GR-40680
 # def test_iteration_and_del():
 #     def test_iter(get_iterable):

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

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # The Universal Permissive License (UPL), Version 1.0
@@ -596,29 +596,68 @@ def key1_eq_call(key1, key2):
     test_op(operator.__and__, key1_eq_call)
     test_op(operator.__iand__, key1_eq_call)
 
-    # TODO: GR-42240
-    #
-    # def symmetric_difference_check(key1, key2):
-    #     assert key1.eq_calls == 0
-    #     assert key1.hash_calls == 0
-    #     assert key2.eq_calls == 2
-    #     assert key2.hash_calls == 0
-    #
-    # test_op(set.symmetric_difference, symmetric_difference_check)
-    # test_op(operator.__xor__, symmetric_difference_check)
-    # test_op(operator.__ixor__, symmetric_difference_check)
-    #
-    # def symmetric_difference_update_check(key1, key2):
-    #     assert key1.eq_calls == 2
-    #     assert key1.hash_calls == 0
-    #     assert key2.eq_calls == 0
-    #     assert key2.hash_calls == 0
-    #
-    # test_op(set.symmetric_difference_update, symmetric_difference_update_check)
-    #
+    def symmetric_difference_check(key1, key2):
+        assert key1.eq_calls == 0
+        assert key1.hash_calls == 0
+        assert key2.eq_calls == 2
+        assert key2.hash_calls == 0
+
+    test_op(set.symmetric_difference, symmetric_difference_check)
+    test_op(operator.__xor__, symmetric_difference_check)
+
+    def symmetric_difference_update_check(key1, key2):
+        assert key1.eq_calls == 2
+        assert key1.hash_calls == 0
+        assert key2.eq_calls == 0
+        assert key2.hash_calls == 0
+
+    test_op(operator.__ixor__, symmetric_difference_update_check)
+    test_op(set.symmetric_difference_update, symmetric_difference_update_check)
+
     # TODO: intersection, intersection_update
 
 
+def test_symmetric_difference_dict_keys_side_effects():
+    def test_op(op, check):
+        key1 = TrackingKey('foo', hash=42)
+        key2 = TrackingKey('bar', hash=42)
+        s = {key1}
+        d = {key2: 1}
+        key1.clear_observations()
+        key2.clear_observations()
+        op(s, d.keys())
+        check(key1, key2)
+
+    def symmetric_difference_check(key1, key2):
+        assert key1.eq_calls == 0
+        assert key1.hash_calls == 0
+        assert key2.eq_calls == 2
+        assert key2.hash_calls == 1
+
+    def symmetric_difference_update_check(key1, key2):
+        assert key1.eq_calls == 2
+        assert key1.hash_calls == 0
+        assert key2.eq_calls == 0
+        assert key2.hash_calls == 1
+
+    test_op(set.symmetric_difference, symmetric_difference_check)
+    test_op(set.symmetric_difference_update, symmetric_difference_update_check)
+
+
+def test_symmetric_difference_update_empty_side_effects():
+    key1 = TrackingKey('foo', hash=42)
+    key2 = TrackingKey('bar', hash=42)
+    s = {key1, key2}
+    key1.clear_observations()
+    key2.clear_observations()
+
+    s.symmetric_difference_update(set())
+    assert key1.eq_calls == 0
+    assert key1.hash_calls == 0
+    assert key2.eq_calls == 0
+    assert key2.hash_calls == 0
+
+
 def test_pop_side_effects():
     class TrackingKey:
         def __init__(self):

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -319,4 +319,39 @@ static HashingStorage doGeneric(VirtualFrame frame, Node inliningTarget, Object
             return getHashingStorageNode.getForSets(frame, inliningTarget, other);
         }
     }
+
+    /**
+     * CPython's set symmetric_difference_update uses stored hashes for exact sets and dicts, but
+     * first materializes other iterables, including dict views, as a temporary set.
+     */
+    @GenerateInline(inlineByDefault = true)
+    public abstract static class GetSetStorageForXorNode extends PNodeWithContext {
+
+        public abstract HashingStorage execute(VirtualFrame frame, Node inliningTarget, Object iterator);
+
+        @Specialization
+        static HashingStorage doHashingCollection(PHashingCollection other) {
+            return other.getDictStorage();
+        }
+
+        @Specialization(guards = "!isPHashingCollection(other)")
+        @InliningCutoff
+        static HashingStorage doGeneric(VirtualFrame frame, Node inliningTarget, Object other,
+                        @Cached PyObjectGetIter getIter,
+                        @Cached PyIterNextNode nextNode,
+                        @Exclusive @Cached HashingStorageSetItem setStorageItem) {
+            HashingStorage curStorage = EmptyStorage.INSTANCE;
+            Object iterator = getIter.execute(frame, inliningTarget, other);
+            while (true) {
+                Object key;
+                try {
+                    key = nextNode.execute(frame, inliningTarget, iterator);
+                } catch (IteratorExhausted e) {
+                    return curStorage;
+                }
+                curStorage = setStorageItem.execute(frame, inliningTarget, curStorage, key, PNone.NONE);
+            }
+        }
+    }
+
 }

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

@@ -1370,23 +1370,23 @@ public ResultAndOther(ObjectHashMap result, HashingStorage other) {
     @GenerateInline
     @GenerateCached(false)
     @ImportStatic({PGuards.class})
-    public abstract static class HashingStorageXorCallback extends HashingStorageForEachCallback<ResultAndOther> {
+    public abstract static class HashingStorageXorCallback extends HashingStorageForEachCallback<EconomicMapStorage> {
 
         @Override
-        public abstract ResultAndOther execute(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, ResultAndOther accumulator);
+        public abstract EconomicMapStorage execute(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, EconomicMapStorage accumulator);
 
         @Specialization
-        static ResultAndOther doGeneric(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, ResultAndOther acc,
+        static EconomicMapStorage doGeneric(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, EconomicMapStorage acc,
                         @Cached PutNode putResultNode,
-                        @Cached HashingStorageGetItemWithHash getFromOther,
+                        @Cached ObjectHashMap.RemoveNode removeResultNode,
                         @Cached HashingStorageIteratorKey iterKey,
                         @Cached HashingStorageIteratorValue iterValue,
                         @Cached HashingStorageIteratorKeyHash iterHash) {
             Object key = iterKey.execute(inliningTarget, storage, it);
             long hash = iterHash.execute(frame, inliningTarget, storage, it);
-            Object otherValue = getFromOther.execute(frame, inliningTarget, acc.other, key, hash);
-            if (otherValue == null) {
-                putResultNode.put(frame, inliningTarget, acc.result, key, hash, iterValue.execute(inliningTarget, storage, it));
+            Object removedValue = removeResultNode.execute(frame, inliningTarget, acc, key, hash);
+            if (removedValue == null) {
+                putResultNode.put(frame, inliningTarget, acc, key, hash, iterValue.execute(inliningTarget, storage, it));
             }
             return acc;
         }
@@ -1397,22 +1397,41 @@ static ResultAndOther doGeneric(Frame frame, Node inliningTarget, HashingStorage
     @GenerateCached(false)
     @ImportStatic({PGuards.class})
     public abstract static class HashingStorageXor extends Node {
-        public abstract HashingStorage execute(Frame frame, Node inliningTarget, HashingStorage a, HashingStorage b);
+        abstract HashingStorage execute(Frame frame, Node inliningTarget, HashingStorage left, HashingStorage right, boolean leftMayBeMutated);
+
+        public final HashingStorage executePreservingLeft(Frame frame, Node inliningTarget, HashingStorage left, HashingStorage right) {
+            return execute(frame, inliningTarget, left, right, false);
+        }
+
+        public final HashingStorage executeMutatingLeft(Frame frame, Node inliningTarget, HashingStorage left, HashingStorage right) {
+            return execute(frame, inliningTarget, left, right, true);
+        }
 
         @Specialization
-        static HashingStorage doIt(Frame frame, Node inliningTarget, HashingStorage aStorage, HashingStorage bStorage,
-                        @Cached HashingStorageForEach forEachA,
-                        @Cached HashingStorageForEach forEachB,
-                        @Cached HashingStorageXorCallback callbackA,
-                        @Cached HashingStorageXorCallback callbackB) {
-            final EconomicMapStorage result = EconomicMapStorage.createWithSideEffects();
-            ObjectHashMap resultMap = result;
+        static HashingStorage doEconomicLeft(Frame frame, Node inliningTarget, EconomicMapStorage leftStorage, HashingStorage rightStorage,
+                        boolean leftMayBeMutated,
+                        @Exclusive @Cached HashingStorageForEach forEachRight,
+                        @Exclusive @Cached HashingStorageXorCallback callback) {
+            if (leftStorage == rightStorage) {
+                return EmptyStorage.INSTANCE;
+            }
+            EconomicMapStorage result = leftMayBeMutated ? leftStorage : (EconomicMapStorage) leftStorage.copy();
+            forEachRight.execute(frame, inliningTarget, rightStorage, callback, result);
+            return result;
+        }
 
-            ResultAndOther accA = new ResultAndOther(resultMap, bStorage);
-            forEachA.execute(frame, inliningTarget, aStorage, callbackA, accA);
+        @Specialization(replaces = "doEconomicLeft")
+        static HashingStorage doIt(Frame frame, Node inliningTarget, HashingStorage leftStorage, HashingStorage rightStorage,
+                        @SuppressWarnings("unused") boolean leftMayBeMutated,
+                        @Exclusive @Cached HashingStorageForEach forEachLeft,
+                        @Exclusive @Cached HashingStorageForEach forEachRight,
+                        @Exclusive @Cached HashingStorageTransferItem transferItem,
+                        @Exclusive @Cached HashingStorageXorCallback callback) {
+            final EconomicMapStorage result = EconomicMapStorage.createWithSideEffects();
 
-            ResultAndOther accB = new ResultAndOther(resultMap, aStorage);
-            forEachB.execute(frame, inliningTarget, bStorage, callbackB, accB);
+            HashingStorage copiedLeft = forEachLeft.execute(frame, inliningTarget, leftStorage, transferItem, result);
+            assert copiedLeft == result;
+            forEachRight.execute(frame, inliningTarget, rightStorage, callback, result);
 
             return result;
         }