Skip to content

Commit 2d32dbf

Browse files
chrisburrguitargeek
authored andcommitted
[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 Cppyy::IsIntegerType (backed by Clang's QualType) to check the return type, which correctly resolves typedefs like size_type. Walk the MRO when checking for size/begin/end/getitem attributes, since HasAttrDirect only checks the class's own __dict__. Skip pythonization if size() has multiple overloads. Update tests for stl_like_class2/3 which have incomplete container interfaces (missing iterators or returning non-iterator types from begin/end). (cherry picked from commit cccbacc)
1 parent 4a3f110 commit 2d32dbf

8 files changed

Lines changed: 131 additions & 25 deletions

File tree

bindings/pyroot/cppyy/CPyCppyy/src/Cppyy.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,8 @@ namespace Cppyy {
143143
CPPYY_IMPORT
144144
bool IsAggregate(TCppType_t type);
145145
CPPYY_IMPORT
146+
bool IsIntegerType(const std::string &type_name);
147+
CPPYY_IMPORT
146148
bool IsDefaultConstructable(TCppType_t type);
147149

148150
CPPYY_IMPORT

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

Lines changed: 43 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);
@@ -1754,12 +1767,36 @@ bool CPyCppyy::Pythonize(PyObject* pyclass, const std::string& name)
17541767
Utility::AddToClass(pyclass, pybool_name, (PyCFunction)NullCheckBool, METH_NOARGS);
17551768
}
17561769

1757-
// for STL containers, and user classes modeled after them
1758-
// the attribute must be a CPyCppyy overload, otherwise the check gives false
1759-
// positives in the case where the class has a non-function attribute that is
1760-
// called "size".
1761-
if (HasAttrDirect(pyclass, PyStrings::gSize, /*mustBeCPyCppyy=*/ true)) {
1762-
Utility::AddToClass(pyclass, "__len__", "size");
1770+
// for STL containers, and user classes modeled after them. Guard the alias to
1771+
// __len__ by verifying that size() returns an integer type and the class has
1772+
// begin()/end() or operator[] (i.e. is container-like). This prevents bool()
1773+
// returning False for valid objects whose size() returns non-integer types like
1774+
// std::optional<std::size_t>. Skip if size() has multiple overloads, as that
1775+
// indicates it is not the simple container-style size() one would map to __len__.
1776+
if (HasAttrDirect(pyclass, PyStrings::gSize, /*mustBeCPyCppyy=*/true) || HasAttrInMRO(pyclass, PyStrings::gSize)) {
1777+
bool sizeIsInteger = false;
1778+
PyObject *pySizeMethod = PyObject_GetAttr(pyclass, PyStrings::gSize);
1779+
if (pySizeMethod && CPPOverload_Check(pySizeMethod)) {
1780+
auto *ol = (CPPOverload *)pySizeMethod;
1781+
if (ol->HasMethods() && ol->fMethodInfo->fMethods.size() == 1) {
1782+
PyObject *pyrestype =
1783+
ol->fMethodInfo->fMethods[0]->Reflex(Cppyy::Reflex::RETURN_TYPE, Cppyy::Reflex::AS_STRING);
1784+
if (pyrestype) {
1785+
sizeIsInteger = Cppyy::IsIntegerType(CPyCppyy_PyText_AsString(pyrestype));
1786+
Py_DECREF(pyrestype);
1787+
}
1788+
}
1789+
}
1790+
Py_XDECREF(pySizeMethod);
1791+
1792+
if (sizeIsInteger) {
1793+
bool hasIterators = (HasAttrDirect(pyclass, PyStrings::gBegin) || HasAttrInMRO(pyclass, PyStrings::gBegin)) &&
1794+
(HasAttrDirect(pyclass, PyStrings::gEnd) || HasAttrInMRO(pyclass, PyStrings::gEnd));
1795+
bool hasSubscript = HasAttrDirect(pyclass, PyStrings::gGetItem) || HasAttrInMRO(pyclass, PyStrings::gGetItem);
1796+
if (hasIterators || hasSubscript) {
1797+
Utility::AddToClass(pyclass, "__len__", "size");
1798+
}
1799+
}
17631800
}
17641801

17651802
if (!IsTemplatedSTLClass(name, "vector") && // vector is dealt with below

bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,6 +1092,18 @@ bool Cppyy::IsAggregate(TCppType_t klass)
10921092
return false;
10931093
}
10941094

1095+
bool Cppyy::IsIntegerType(const std::string &type_name)
1096+
{
1097+
// Test if the named type is an integer type
1098+
TypeInfo_t *ti = gInterpreter->TypeInfo_Factory(type_name.c_str());
1099+
if (!ti)
1100+
return false;
1101+
void *qtp = gInterpreter->TypeInfo_QualTypePtr(ti);
1102+
bool result = qtp ? gInterpreter->IsIntegerType(qtp) : false;
1103+
gInterpreter->TypeInfo_Delete(ti);
1104+
return result;
1105+
}
1106+
10951107
bool Cppyy::IsDefaultConstructable(TCppType_t type)
10961108
{
10971109
// Test if this type has a default constructor or is a "plain old data" type

bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/cpp_cppyy.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ namespace Cppyy {
119119
RPY_EXPORTED
120120
bool IsAggregate(TCppType_t type);
121121
RPY_EXPORTED
122+
bool IsIntegerType(const std::string &type_name);
123+
RPY_EXPORTED
122124
bool IsDefaultConstructable(TCppType_t type);
123125

124126
RPY_EXPORTED

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,43 @@ class WithCallback3 : public WithCallback2 {
166166
void set_int(int i);
167167
};
168168

169+
//===========================================================================
170+
// for testing size() -> __len__ pythonization guards
171+
class SizeReturnsInt {
172+
public:
173+
int size() { return 3; }
174+
int *begin() { return m_data; }
175+
int *end() { return m_data + 3; }
176+
177+
private:
178+
int m_data[3] = {1, 2, 3};
179+
};
180+
181+
class SizeReturnsNonInt {
182+
public:
183+
struct OptSize {};
184+
OptSize size() { return {}; }
185+
int *begin() { return nullptr; }
186+
int *end() { return nullptr; }
187+
};
188+
189+
class SizeWithoutIterator {
190+
public:
191+
int size() { return 5; }
192+
// no begin()/end() or operator[]
193+
};
194+
195+
// for testing __len__ with fully inherited container interface
196+
class ContainerBase {
197+
public:
198+
int size() { return 2; }
199+
int *begin() { return m_data; }
200+
int *end() { return m_data + 2; }
201+
202+
private:
203+
int m_data[2] = {10, 20};
204+
};
205+
206+
class InheritedContainer : public ContainerBase {};
207+
169208
} // namespace pyzables

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

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

88-
template<class value_type, size_t sz>
89-
class stl_like_class3 : public stl_like_class2<value_type, sz> {
90-
using stl_like_class2<value_type, sz>::fData;
91-
public:
92-
size_t size() { return sz; }
93-
value_type& begin() { return fData; }
94-
value_type& end() { return fData + sz; }
95-
};
96-
9788
class stl_like_class4 {
9889
public:
9990
struct iterator {

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,33 @@ def test09_cpp_side_pythonization(self):
259259
assert cppyy.gbl.pyzables.WithCallback2.klass_name == 'pyzables::WithCallback3'
260260

261261

262+
def test11_size_len_pythonization_guards(self):
263+
"""Verify __len__ is only installed when size() returns int and class is iterable"""
264+
265+
import cppyy
266+
267+
# size() returns int AND has begin/end -> __len__ installed
268+
obj_int = cppyy.gbl.pyzables.SizeReturnsInt()
269+
assert hasattr(obj_int, "__len__")
270+
assert len(obj_int) == 3
271+
assert bool(obj_int)
272+
273+
# size() returns non-integer type -> __len__ NOT installed
274+
obj_opt = cppyy.gbl.pyzables.SizeReturnsNonInt()
275+
assert not hasattr(obj_opt, "__len__")
276+
assert bool(obj_opt) # should be True (valid object)
277+
278+
# size() returns int but no begin/end or operator[] -> __len__ NOT installed
279+
obj_noiter = cppyy.gbl.pyzables.SizeWithoutIterator()
280+
assert not hasattr(obj_noiter, "__len__")
281+
assert bool(obj_noiter)
282+
283+
# fully inherited container interface -> __len__ installed via MRO
284+
obj_inherited = cppyy.gbl.pyzables.InheritedContainer()
285+
assert hasattr(obj_inherited, "__len__")
286+
assert len(obj_inherited) == 2
287+
288+
262289
## actual test run
263290
if __name__ == '__main__':
264291
result = run_pytest(__file__)

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,16 +1459,12 @@ def test02_STL_like_class_iterators(self):
14591459

14601460
assert i == len(a)-1
14611461

1462-
for cls in [cppyy.gbl.stl_like_class2, cppyy.gbl.stl_like_class3]:
1463-
b = cls[float, 2]()
1464-
b[0] = 27; b[1] = 42
1465-
limit = len(b)+1
1466-
for x in b:
1467-
limit -= 1
1468-
assert limit and "iterated too far!"
1469-
assert x in [27, 42]
1470-
assert x == 42
1471-
del x, b
1462+
b = cppyy.gbl.stl_like_class2[float, 2]()
1463+
b[0] = 27
1464+
b[1] = 42
1465+
assert len(b) == 2
1466+
for i in range(len(b)):
1467+
assert b[i] in [27, 42]
14721468

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

0 commit comments

Comments
 (0)