[CPyCppyy] Remove pointer-based fallback in CPPInstance equality check

The Python proxy for C++ objects previously implemented a fallback equality
comparison based on proxy type and the held C++ pointer address when no
C++ `operator==` / `operator!=` was available. This behavior was
misleading, because it made expressions like `a == b` appear to perform
a value comparison even though no corresponding C++ operator was
defined.

This patch removes the implicit pointer-based fallback and instead raises
a TypeError when equality is requested between two CPPInstance proxies for
which no C++ equality operator can be resolved. This avoids silently
changing semantics and makes unsupported comparisons explicit.

The only case where the pointer-based fallback is kept is for proxies of
the exact same type when at least one wraps a `nullptr`: in this case,
comparison continues to be performed on the pointer value, because
comparing by value would not be possible. This preserves existing cppyy
behavior in a case that is not semantically ambiguous. Although C++
would also allow comparisons between nullptr pointers of types related
by inheritance, broadening the rule would silently change previous cppyy
behavior, where such comparisons returned `False`.

To summarize: this change prevents ambiguous equality semantics while
avoiding silent behavior changes for existing code by raising a type
error in cases that are ambiguous or where previous behavior was
different from C++ semantics.

Closes #21347.

Author: guitargeek

bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx

@@ -532,22 +532,6 @@ static inline PyObject* eqneq_binop(CPPClass* klass, PyObject* self, PyObject* o
     Py_RETURN_TRUE;
 }
 
-static inline void* cast_actual(void* obj) {
-    void* address = ((CPPInstance*)obj)->GetObject();
-    if (((CPPInstance*)obj)->fFlags & CPPInstance::kIsActual)
-        return address;
-
-    Cppyy::TCppType_t klass = ((CPPClass*)Py_TYPE((PyObject*)obj))->fCppType;
-    Cppyy::TCppType_t clActual = Cppyy::GetActualClass(klass, address);
-    if (clActual && clActual != klass) {
-        intptr_t offset = Cppyy::GetBaseOffset(
-             clActual, klass, address, -1 /* down-cast */, true /* report errors */);
-        if (offset != -1) address = (void*)((intptr_t)address + offset);
-    }
-
-    return address;
-}
-
 
 #define CPYCPPYY_ORDERED_OPERATOR_STUB(op, ometh, label)                      \
     if (!ometh) {                                                             \
@@ -592,25 +576,47 @@ static PyObject* op_richcompare(CPPInstance* self, PyObject* other, int op)
             result = eqneq_binop((CPPClass*)Py_TYPE(other), other, (PyObject*)self, op);
         if (result) return result;
 
-    // default behavior: type + held pointer value defines identity; if both are
-    // CPPInstance objects, perform an additional autocast if need be
-        bool bIsEq = false;
-
-        if ((Py_TYPE(self) == Py_TYPE(other) && \
-                self->GetObject() == ((CPPInstance*)other)->GetObject())) {
-        // direct match
-            bIsEq = true;
-        } else if (CPPInstance_Check(other)) {
-        // try auto-cast match
-            void* addr1 = cast_actual(self);
-            void* addr2 = cast_actual(other);
-            bIsEq = addr1 && addr2 && (addr1 == addr2);
+    // for non-CPPInstance objects, let Python handle dispatch/fallback
+        if (!CPPInstance_Check(other)) {
+            Py_INCREF(Py_NotImplemented);
+            return Py_NotImplemented;
         }
 
-        if ((op == Py_EQ && bIsEq) || (op == Py_NE && !bIsEq))
-            Py_RETURN_TRUE;
+    // if both proxies have the same type and at least one wraps nullptr,
+    // comparing nullness is unambiguous
+        auto ptr1 = self->GetObject();
+        auto ptr2 = ((CPPInstance*)other)->GetObject();
+        if(ptr1 == nullptr || ptr2 == nullptr) {
+        // Take this branch only for exact proxy type matches. Broadening this
+        // to inheritance-related types would be allowed in C++, but it would
+        // silently change previous cppyy behavior, where equality comparison
+        // of different-typed nullptr always resulted in `False`, straying away
+        // from C++.
+            if (Py_TYPE(self) == Py_TYPE(other)) {
+                bool bIsEq = ptr1 == ptr2;
+                if ((op == Py_EQ && bIsEq) || (op == Py_NE && !bIsEq))
+                    Py_RETURN_TRUE;
+                Py_RETURN_FALSE;
+            }
+        }
 
-        Py_RETURN_FALSE;
+    // in the remaining cases, the semantics of the comparison are ambiguous, so we raise an exception
+        const char* opstr = op == Py_EQ ? "==" : "!=";
+        const char* lhs = Py_TYPE(self)->tp_name;
+        const char* rhs = Py_TYPE(other)->tp_name;
+        const char *msg =
+            "\nC++ equality operator '%s' is not defined for objects of type \"%s\" and \"%s\"."
+            "\n\nThe Python proxy no longer falls back to comparing by type and held C++ pointer "
+            "address, because that can be misleading: using `%s` may look like a value comparison "
+            "even though no corresponding C++ operator is available."
+            "\n\nSome comparisons that are well-formed in C++ are still rejected here to avoid "
+            "changing legacy cppyy behavior silently."
+            "\n\nDefine a suitable C++ equality operator for these operands, or compare object identity explicitly "
+            "through another mechanism if that is what you intend."
+            "\n";
+
+        PyErr_Format(PyExc_TypeError, msg, opstr, lhs, rhs, opstr);
+        return NULL;
     }
 
 // ordered comparison operators

bindings/pyroot/cppyy/cppyy/test/test_advancedcpp.py

@@ -445,10 +445,10 @@ def test09_opaque_pointer_passing(self):
         #assert o == cppyy.bind_object(cobj, o.__class__)
         #assert o == cppyy.bind_object(cobj, "some_concrete_class")
         assert cppyy.addressof(o) == cppyy.addressof(cppyy.bind_object(addr, some_concrete_class))
-        assert o == cppyy.bind_object(addr, some_concrete_class)
-        assert o == cppyy.bind_object(addr, type(o))
-        assert o == cppyy.bind_object(addr, o.__class__)
-        assert o == cppyy.bind_object(addr, "some_concrete_class")
+        assert o is cppyy.bind_object(addr, some_concrete_class)
+        assert o is cppyy.bind_object(addr, type(o))
+        assert o is cppyy.bind_object(addr, o.__class__)
+        assert o is cppyy.bind_object(addr, "some_concrete_class")
         raises(TypeError, cppyy.bind_object, addr, "does_not_exist")
         raises(TypeError, cppyy.bind_object, addr, 1)
 
@@ -493,7 +493,6 @@ def test10_object_identity(self):
         r4 = r1.m_other
         assert r3 is r4
 
-        assert r3 == r2
         assert r3 is r2
 
         r3.extra = 42
@@ -531,13 +530,13 @@ def test12_actual_type(self):
         b = base_class()
         d = derived_class()
 
-        assert b == b.cycle(b)
+        assert b is b.cycle(b)
         assert id(b) == id(b.cycle(b))
-        assert b == d.cycle(b)
+        assert b is d.cycle(b)
         assert id(b) == id(d.cycle(b))
-        assert d == b.cycle(d)
+        assert d is b.cycle(d)
         assert id(d) == id(b.cycle(d))
-        assert d == d.cycle(d)
+        assert d is d.cycle(d)
         assert id(d) == id(d.cycle(d))
 
         assert isinstance(b.cycle(b), base_class)

bindings/pyroot/cppyy/cppyy/test/test_datatypes.py

@@ -644,7 +644,7 @@ def test09_global_ptr(self):
 
         d = gbl.get_global_pod()
         assert gbl.is_global_pod(d)
-        assert c == d
+        assert c is d
         assert id(c) == id(d)
 
         e = gbl.CppyyTestPod()
@@ -1008,8 +1008,8 @@ def test19_object_and_pointer_comparisons(self):
         l1 = cppyy.bind_object(0, gbl.FourVector)
         assert not l1
 
-        assert c1 != l1
-        assert l1 != c1
+        # assert c1 != l1
+        # assert l1 != c1
 
         l2 = cppyy.bind_object(0, gbl.FourVector)
         assert l1 == l2

bindings/pyroot/cppyy/cppyy/test/test_regression.py

@@ -253,7 +253,7 @@ def test11_cobject_addressing(self):
         a = cppyy.gbl.CObjA()
         co = cppyy.ll.as_cobject(a)
 
-        assert a == cppyy.bind_object(co, 'CObjA')
+        assert a is cppyy.bind_object(co, 'CObjA')
         assert a.m_int == 42
         assert cppyy.bind_object(co, 'CObjA').m_int == 42
 

bindings/pyroot/pythonizations/test/tobject_comparisonops.py

@@ -19,7 +19,7 @@ def test_eq(self):
         o = ROOT.TObject()
 
         # TObject::IsEqual compares internal address
-        self.assertTrue(o == o)
+        self.assertTrue(o.IsEqual(o))
 
         # Test comparison with no TObject
         self.assertFalse(o == 1)
@@ -31,7 +31,7 @@ def test_ne(self):
         o = ROOT.TObject()
 
         # TObject::IsEqual compares internal address
-        self.assertFalse(o != o)
+        self.assertFalse(!o.IsEqual(o))
 
         # Test comparison with no TObject
         self.assertTrue(o != 1)

roottest/python/basic/PyROOT_datatypetest.py

@@ -598,7 +598,6 @@ def test10_global_ptr(self):
 
         d = gbl.get_global_pod()
         assert gbl.is_global_pod(d)
-        assert c == d
         assert c is d
 
         e = gbl.CppyyTestPod()

roottest/python/cpp/PyROOT_cpptests.py

@@ -230,7 +230,7 @@ def test12VoidPointerPassing( self ):
 
       pZ = Z.getZ(0)
       self.assertEqual( Z.checkAddressOfZ( pZ ), True )
-      self.assertEqual( pZ , Z.getZ(1) )
+      # self.assertEqual( pZ , Z.getZ(1) )
 
       import array
       # Not supported in p2.2
@@ -302,8 +302,11 @@ def test15ObjectAndPointerComparisons( self ):
       l1 = MakeNullPointer( TLorentzVector )
       self.assertFalse( l1 )
 
-      self.assertNotEqual( c1, l1 )
-      self.assertNotEqual( l1, c1 )
+    # Forbidden by the type system
+      with self.assertRaises(TypeError):
+          self.assertNotEqual( c1, l1 )
+      with self.assertRaises(TypeError):
+          self.assertNotEqual( l1, c1 )
 
       l2 = MakeNullPointer( TLorentzVector )
       self.assertEqual( l1, l2 )

roottest/python/regression/PyROOT_regressiontests.py

@@ -360,7 +360,9 @@ def test1IterateWithBaseIterator( self ):
 
       b = a.begin()
       e = a.end()
-      self.assertNotEqual( a, e )
+      # This comparison is forbidden by the type system
+      with self.assertRaises(TypeError):
+          a == e
 
       b.__preinc__()
       self.assertEqual( b, e )