Skip to content

Commit 4eaf83a

Browse files
committed
[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.
1 parent f1a9a19 commit 4eaf83a

1 file changed

Lines changed: 38 additions & 32 deletions

File tree

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

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -532,22 +532,6 @@ static inline PyObject* eqneq_binop(CPPClass* klass, PyObject* self, PyObject* o
532532
Py_RETURN_TRUE;
533533
}
534534

535-
static inline void* cast_actual(void* obj) {
536-
void* address = ((CPPInstance*)obj)->GetObject();
537-
if (((CPPInstance*)obj)->fFlags & CPPInstance::kIsActual)
538-
return address;
539-
540-
Cppyy::TCppType_t klass = ((CPPClass*)Py_TYPE((PyObject*)obj))->fCppType;
541-
Cppyy::TCppType_t clActual = Cppyy::GetActualClass(klass, address);
542-
if (clActual && clActual != klass) {
543-
intptr_t offset = Cppyy::GetBaseOffset(
544-
clActual, klass, address, -1 /* down-cast */, true /* report errors */);
545-
if (offset != -1) address = (void*)((intptr_t)address + offset);
546-
}
547-
548-
return address;
549-
}
550-
551535

552536
#define CPYCPPYY_ORDERED_OPERATOR_STUB(op, ometh, label) \
553537
if (!ometh) { \
@@ -592,25 +576,47 @@ static PyObject* op_richcompare(CPPInstance* self, PyObject* other, int op)
592576
result = eqneq_binop((CPPClass*)Py_TYPE(other), other, (PyObject*)self, op);
593577
if (result) return result;
594578

595-
// default behavior: type + held pointer value defines identity; if both are
596-
// CPPInstance objects, perform an additional autocast if need be
597-
bool bIsEq = false;
598-
599-
if ((Py_TYPE(self) == Py_TYPE(other) && \
600-
self->GetObject() == ((CPPInstance*)other)->GetObject())) {
601-
// direct match
602-
bIsEq = true;
603-
} else if (CPPInstance_Check(other)) {
604-
// try auto-cast match
605-
void* addr1 = cast_actual(self);
606-
void* addr2 = cast_actual(other);
607-
bIsEq = addr1 && addr2 && (addr1 == addr2);
579+
// for non-CPPInstance objects, let Python handle dispatch/fallback
580+
if (!CPPInstance_Check(other)) {
581+
Py_INCREF(Py_NotImplemented);
582+
return Py_NotImplemented;
608583
}
609584

610-
if ((op == Py_EQ && bIsEq) || (op == Py_NE && !bIsEq))
611-
Py_RETURN_TRUE;
585+
// if both proxies have the same type and at least one wraps nullptr,
586+
// comparing nullness is unambiguous
587+
auto ptr1 = self->GetObject();
588+
auto ptr2 = ((CPPInstance*)other)->GetObject();
589+
if(ptr1 == nullptr || ptr2 == nullptr) {
590+
// Take this branch only for exact proxy type matches. Broadening this
591+
// to inheritance-related types would be allowed in C++, but it would
592+
// silently change previous cppyy behavior, where equality comparison
593+
// of different-typed nullptr always resulted in `False`, straying away
594+
// from C++.
595+
if (Py_TYPE(self) == Py_TYPE(other)) {
596+
bool bIsEq = ptr1 == ptr2;
597+
if ((op == Py_EQ && bIsEq) || (op == Py_NE && !bIsEq))
598+
Py_RETURN_TRUE;
599+
Py_RETURN_FALSE;
600+
}
601+
}
612602

613-
Py_RETURN_FALSE;
603+
// in the remaining cases, the semantics of the comparison are ambiguous, so we raise an exception
604+
const char* opstr = op == Py_EQ ? "==" : "!=";
605+
const char* lhs = Py_TYPE(self)->tp_name;
606+
const char* rhs = Py_TYPE(other)->tp_name;
607+
const char *msg =
608+
"\nC++ equality operator '%s' is not defined for objects of type \"%s\" and \"%s\"."
609+
"\n\nThe Python proxy no longer falls back to comparing by type and held C++ pointer "
610+
"address, because that can be misleading: using `%s` may look like a value comparison "
611+
"even though no corresponding C++ operator is available."
612+
"\n\nSome comparisons that are well-formed in C++ are still rejected here to avoid "
613+
"changing legacy cppyy behavior silently."
614+
"\n\nDefine a suitable C++ equality operator for these operands, or compare object identity explicitly "
615+
"through another mechanism if that is what you intend."
616+
"\n";
617+
618+
PyErr_Format(PyExc_TypeError, msg, opstr, lhs, rhs, opstr);
619+
return NULL;
614620
}
615621

616622
// ordered comparison operators

0 commit comments

Comments
 (0)