Skip to content

Commit 7a2ba49

Browse files
committed
[cppyy] Only alias size() to __len__ for container-like classes
Guard the automatic size() -> __len__ pythonization to require that size() returns an integer type and the class has begin()/end() methods or operator[]. This prevents bool() returning False for valid objects whose size() returns non-integer types like std::optional<std::size_t>. Use the CPPOverload's Reflex API to query the return type, which correctly handles inherited methods via Python's MRO. Also walk the MRO when checking for the size attribute, since HasAttrDirect only checks the class's own __dict__. Update tests for stl_like_class2/3 which have incomplete container interfaces (missing iterators or returning non-iterator types from begin/end).
1 parent d0566ab commit 7a2ba49

5 files changed

Lines changed: 126 additions & 25 deletions

File tree

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

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,19 @@ bool HasAttrDirect(PyObject* pyclass, PyObject* pyname, bool mustBeCPyCppyy = fa
5252
return false;
5353
}
5454

55+
bool HasAttrInMRO(PyObject *pyclass, PyObject *pyname)
56+
{
57+
// Check base classes in the MRO (skipping the class itself) for a CPyCppyy overload.
58+
PyObject *mro = ((PyTypeObject *)pyclass)->tp_mro;
59+
if (mro && PyTuple_Check(mro)) {
60+
for (Py_ssize_t i = 1; i < PyTuple_GET_SIZE(mro); ++i) {
61+
if (HasAttrDirect(PyTuple_GET_ITEM(mro, i), pyname, /*mustBeCPyCppyy=*/true))
62+
return true;
63+
}
64+
}
65+
return false;
66+
}
67+
5568
PyObject* GetAttrDirect(PyObject* pyclass, PyObject* pyname) {
5669
// get an attribute without causing getattr lookups
5770
PyObject* dct = PyObject_GetAttr(pyclass, PyStrings::gDict);
@@ -63,6 +76,18 @@ PyObject* GetAttrDirect(PyObject* pyclass, PyObject* pyname) {
6376
return nullptr;
6477
}
6578

79+
//-----------------------------------------------------------------------------
80+
static bool IsIntegerType(const std::string &resname)
81+
{
82+
// Check if the resolved return type is an integer type suitable for __len__,
83+
// which requires a non-negative int in Python.
84+
static const std::set<std::string> sIntegerTypes = {"short", "unsigned short", "int", "unsigned int",
85+
"long", "unsigned long", "long long", "unsigned long long",
86+
"char", "signed char", "unsigned char", "wchar_t"};
87+
std::string resolved = Cppyy::ResolveName(resname);
88+
return sIntegerTypes.find(resolved) != sIntegerTypes.end();
89+
}
90+
6691
//-----------------------------------------------------------------------------
6792
inline bool IsTemplatedSTLClass(const std::string& name, const std::string& klass) {
6893
// Scan the name of the class and determine whether it is a template instantiation.
@@ -1755,12 +1780,37 @@ bool CPyCppyy::Pythonize(PyObject* pyclass, const std::string& name)
17551780
Utility::AddToClass(pyclass, pybool_name, (PyCFunction)NullCheckBool, METH_NOARGS);
17561781
}
17571782

1758-
// for STL containers, and user classes modeled after them
1759-
// the attribute must be a CPyCppyy overload, otherwise the check gives false
1760-
// positives in the case where the class has a non-function attribute that is
1761-
// called "size".
1762-
if (HasAttrDirect(pyclass, PyStrings::gSize, /*mustBeCPyCppyy=*/ true)) {
1763-
Utility::AddToClass(pyclass, "__len__", "size");
1783+
// for STL containers, and user classes modeled after them. Guard the alias to
1784+
// __len__ by verifying that size() returns an integer type and the class has
1785+
// begin()/end() or operator[] (i.e. is container-like). This prevents bool()
1786+
// returning False for valid objects whose size() returns non-integer types like
1787+
// std::optional<std::size_t>.
1788+
if (HasAttrDirect(pyclass, PyStrings::gSize, /*mustBeCPyCppyy=*/true) || HasAttrInMRO(pyclass, PyStrings::gSize)) {
1789+
bool sizeIsInteger = false;
1790+
PyObject *pySizeMethod = PyObject_GetAttr(pyclass, PyStrings::gSize);
1791+
if (pySizeMethod && CPPOverload_Check(pySizeMethod)) {
1792+
auto *ol = (CPPOverload *)pySizeMethod;
1793+
// size() is not typically overloaded; check the first overload's return type
1794+
if (ol->HasMethods()) {
1795+
PyObject *pyrestype =
1796+
ol->fMethodInfo->fMethods[0]->Reflex(Cppyy::Reflex::RETURN_TYPE, Cppyy::Reflex::AS_STRING);
1797+
if (pyrestype) {
1798+
sizeIsInteger = IsIntegerType(CPyCppyy_PyText_AsString(pyrestype));
1799+
Py_DECREF(pyrestype);
1800+
}
1801+
}
1802+
}
1803+
Py_XDECREF(pySizeMethod);
1804+
1805+
if (sizeIsInteger) {
1806+
bool hasIterators = (HasAttrDirect(pyclass, PyStrings::gBegin) || HasAttrInMRO(pyclass, PyStrings::gBegin)) &&
1807+
(HasAttrDirect(pyclass, PyStrings::gEnd) || HasAttrInMRO(pyclass, PyStrings::gEnd));
1808+
bool hasSubscript = HasAttrDirect(pyclass, PyStrings::gGetItem) ||
1809+
!Cppyy::GetMethodIndicesFromName(klass->fCppType, "operator[]").empty();
1810+
if (hasIterators || hasSubscript) {
1811+
Utility::AddToClass(pyclass, "__len__", "size");
1812+
}
1813+
}
17641814
}
17651815

17661816
if (HasAttrDirect(pyclass, PyStrings::gContains)) {

bindings/pyroot/cppyy/cppyy/test/pythonizables.h

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,43 @@ class IndexableBase {
123123

124124
class IndexableDerived : public IndexableBase {};
125125

126+
//===========================================================================
127+
// for testing size() -> __len__ pythonization guards
128+
class SizeReturnsInt {
129+
public:
130+
int size() { return 3; }
131+
int *begin() { return m_data; }
132+
int *end() { return m_data + 3; }
133+
134+
private:
135+
int m_data[3] = {1, 2, 3};
136+
};
137+
138+
class SizeReturnsNonInt {
139+
public:
140+
struct OptSize {};
141+
OptSize size() { return {}; }
142+
int *begin() { return nullptr; }
143+
int *end() { return nullptr; }
144+
};
145+
146+
class SizeWithoutIterator {
147+
public:
148+
int size() { return 5; }
149+
// no begin()/end() or operator[]
150+
};
151+
152+
// for testing __len__ with fully inherited container interface
153+
class ContainerBase {
154+
public:
155+
int size() { return 2; }
156+
int *begin() { return m_data; }
157+
int *end() { return m_data + 2; }
158+
159+
private:
160+
int m_data[2] = {10, 20};
161+
};
162+
163+
class InheritedContainer : public ContainerBase {};
164+
126165
} // namespace pyzables

bindings/pyroot/cppyy/cppyy/test/stltypes.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,6 @@ class stl_like_class2 {
8282
value_type& operator[](ptrdiff_t i) { return fData[i]; }
8383
};
8484

85-
template<class value_type, size_t sz>
86-
class stl_like_class3 : public stl_like_class2<value_type, sz> {
87-
using stl_like_class2<value_type, sz>::fData;
88-
public:
89-
size_t size() { return sz; }
90-
value_type& begin() { return fData; }
91-
value_type& end() { return fData + sz; }
92-
};
93-
9485
class stl_like_class4 {
9586
public:
9687
struct iterator {

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,32 @@ def test10_shared_ptr_reset(self):
370370
optr.__smartptr__().reset(o2)
371371
assert optr == o2
372372

373+
def test11_size_len_pythonization_guards(self):
374+
"""Verify __len__ is only installed when size() returns int and class is iterable"""
375+
376+
import cppyy
377+
378+
# size() returns int AND has begin/end -> __len__ installed
379+
obj_int = cppyy.gbl.pyzables.SizeReturnsInt()
380+
assert hasattr(obj_int, '__len__')
381+
assert len(obj_int) == 3
382+
assert bool(obj_int)
383+
384+
# size() returns non-integer type -> __len__ NOT installed
385+
obj_opt = cppyy.gbl.pyzables.SizeReturnsNonInt()
386+
assert not hasattr(obj_opt, '__len__')
387+
assert bool(obj_opt) # should be True (valid object)
388+
389+
# size() returns int but no begin/end or operator[] -> __len__ NOT installed
390+
obj_noiter = cppyy.gbl.pyzables.SizeWithoutIterator()
391+
assert not hasattr(obj_noiter, '__len__')
392+
assert bool(obj_noiter)
393+
394+
# fully inherited container interface -> __len__ installed via MRO
395+
obj_inherited = cppyy.gbl.pyzables.InheritedContainer()
396+
assert hasattr(obj_inherited, '__len__')
397+
assert len(obj_inherited) == 2
398+
373399
## actual test run
374400
if __name__ == "__main__":
375401
exit(pytest.main(args=['-sv', '-ra', __file__]))

bindings/pyroot/cppyy/cppyy/test/test_stltypes.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,16 +1484,11 @@ def test02_STL_like_class_iterators(self):
14841484

14851485
assert i == len(a)-1
14861486

1487-
for cls in [cppyy.gbl.stl_like_class2, cppyy.gbl.stl_like_class3]:
1488-
b = cls[float, 2]()
1489-
b[0] = 27; b[1] = 42
1490-
limit = len(b)+1
1491-
for x in b:
1492-
limit -= 1
1493-
assert limit and "iterated too far!"
1494-
assert x in [27, 42]
1495-
assert x == 42
1496-
del x, b
1487+
b = cppyy.gbl.stl_like_class2[float, 2]()
1488+
b[0] = 27; b[1] = 42
1489+
assert len(b) == 2
1490+
for i in range(len(b)):
1491+
assert b[i] in [27, 42]
14971492

14981493
for num in [4, 5, 6, 7]:
14991494
cls = getattr(cppyy.gbl, 'stl_like_class%d' % num)

0 commit comments

Comments
 (0)