[cppyy] Only alias size() to __len__ for container-like classes

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

@@ -139,6 +139,8 @@ namespace Cppyy {
     CPPYY_IMPORT
     bool IsAggregate(TCppType_t type);
     CPPYY_IMPORT
+    bool IsIntegerType(const std::string &type_name);
+    CPPYY_IMPORT
     bool IsDefaultConstructable(TCppType_t type);
 
     CPPYY_IMPORT

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

@@ -52,6 +52,19 @@ bool HasAttrDirect(PyObject* pyclass, PyObject* pyname, bool mustBeCPyCppyy = fa
     return false;
 }
 
+bool HasAttrInMRO(PyObject *pyclass, PyObject *pyname)
+{
+   // Check base classes in the MRO (skipping the class itself) for a CPyCppyy overload.
+   PyObject *mro = ((PyTypeObject *)pyclass)->tp_mro;
+   if (mro && PyTuple_Check(mro)) {
+      for (Py_ssize_t i = 1; i < PyTuple_GET_SIZE(mro); ++i) {
+         if (HasAttrDirect(PyTuple_GET_ITEM(mro, i), pyname, /*mustBeCPyCppyy=*/true))
+            return true;
+      }
+   }
+   return false;
+}
+
 PyObject* GetAttrDirect(PyObject* pyclass, PyObject* pyname) {
 // get an attribute without causing getattr lookups
     PyObject* dct = PyObject_GetAttr(pyclass, PyStrings::gDict);
@@ -1755,12 +1768,36 @@ bool CPyCppyy::Pythonize(PyObject* pyclass, const std::string& name)
         Utility::AddToClass(pyclass, pybool_name, (PyCFunction)NullCheckBool, METH_NOARGS);
     }
 
-// for STL containers, and user classes modeled after them
-// the attribute must be a CPyCppyy overload, otherwise the check gives false
-// positives in the case where the class has a non-function attribute that is
-// called "size".
-    if (HasAttrDirect(pyclass, PyStrings::gSize, /*mustBeCPyCppyy=*/ true)) {
-        Utility::AddToClass(pyclass, "__len__", "size");
+    // for STL containers, and user classes modeled after them. Guard the alias to
+    // __len__ by verifying that size() returns an integer type and the class has
+    // begin()/end() or operator[] (i.e. is container-like). This prevents bool()
+    // returning False for valid objects whose size() returns non-integer types like
+    // std::optional<std::size_t>. Skip if size() has multiple overloads, as that
+    // indicates it is not the simple container-style size() one would map to __len__.
+    if (HasAttrDirect(pyclass, PyStrings::gSize, /*mustBeCPyCppyy=*/true) || HasAttrInMRO(pyclass, PyStrings::gSize)) {
+       bool sizeIsInteger = false;
+       PyObject *pySizeMethod = PyObject_GetAttr(pyclass, PyStrings::gSize);
+       if (pySizeMethod && CPPOverload_Check(pySizeMethod)) {
+          auto *ol = (CPPOverload *)pySizeMethod;
+          if (ol->HasMethods() && ol->fMethodInfo->fMethods.size() == 1) {
+             PyObject *pyrestype =
+                ol->fMethodInfo->fMethods[0]->Reflex(Cppyy::Reflex::RETURN_TYPE, Cppyy::Reflex::AS_STRING);
+             if (pyrestype) {
+                sizeIsInteger = Cppyy::IsIntegerType(CPyCppyy_PyText_AsString(pyrestype));
+                Py_DECREF(pyrestype);
+             }
+          }
+       }
+       Py_XDECREF(pySizeMethod);
+
+       if (sizeIsInteger) {
+          bool hasIterators = (HasAttrDirect(pyclass, PyStrings::gBegin) || HasAttrInMRO(pyclass, PyStrings::gBegin)) &&
+                              (HasAttrDirect(pyclass, PyStrings::gEnd) || HasAttrInMRO(pyclass, PyStrings::gEnd));
+          bool hasSubscript = HasAttrDirect(pyclass, PyStrings::gGetItem) || HasAttrInMRO(pyclass, PyStrings::gGetItem);
+          if (hasIterators || hasSubscript) {
+             Utility::AddToClass(pyclass, "__len__", "size");
+          }
+       }
     }
 
     if (HasAttrDirect(pyclass, PyStrings::gContains)) {

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

@@ -1247,6 +1247,18 @@ bool Cppyy::IsAggregate(TCppType_t type)
     return false;
 }
 
+bool Cppyy::IsIntegerType(const std::string &type_name)
+{
+   // Test if the named type is an integer type
+   TypeInfo_t *ti = gInterpreter->TypeInfo_Factory(type_name.c_str());
+   if (!ti)
+      return false;
+   void *qtp = gInterpreter->TypeInfo_QualTypePtr(ti);
+   bool result = qtp ? gInterpreter->IsIntegerType(qtp) : false;
+   gInterpreter->TypeInfo_Delete(ti);
+   return result;
+}
+
 bool Cppyy::IsDefaultConstructable(TCppType_t type)
 {
 // 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

@@ -136,6 +136,8 @@ namespace Cppyy {
     RPY_EXPORTED
     bool IsAggregate(TCppType_t type);
     RPY_EXPORTED
+    bool IsIntegerType(const std::string &type_name);
+    RPY_EXPORTED
     bool IsDefaultConstructable(TCppType_t type);
 
     RPY_EXPORTED

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

@@ -123,4 +123,43 @@ class IndexableBase {
 
 class IndexableDerived : public IndexableBase {};
 
+//===========================================================================
+// for testing size() -> __len__ pythonization guards
+class SizeReturnsInt {
+public:
+   int size() { return 3; }
+   int *begin() { return m_data; }
+   int *end() { return m_data + 3; }
+
+private:
+   int m_data[3] = {1, 2, 3};
+};
+
+class SizeReturnsNonInt {
+public:
+   struct OptSize {};
+   OptSize size() { return {}; }
+   int *begin() { return nullptr; }
+   int *end() { return nullptr; }
+};
+
+class SizeWithoutIterator {
+public:
+   int size() { return 5; }
+   // no begin()/end() or operator[]
+};
+
+// for testing __len__ with fully inherited container interface
+class ContainerBase {
+public:
+   int size() { return 2; }
+   int *begin() { return m_data; }
+   int *end() { return m_data + 2; }
+
+private:
+   int m_data[2] = {10, 20};
+};
+
+class InheritedContainer : public ContainerBase {};
+
 } // namespace pyzables

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

@@ -82,15 +82,6 @@ class stl_like_class2 {
     value_type& operator[](ptrdiff_t i) { return fData[i]; }
 };
 
-template<class value_type, size_t sz>
-class stl_like_class3 : public stl_like_class2<value_type, sz> {
-    using stl_like_class2<value_type, sz>::fData;
-public:
-    size_t size() { return sz; }
-    value_type& begin() { return fData; }
-    value_type& end() { return fData + sz; }
-};
-
 class stl_like_class4 {
 public:
     struct iterator {

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

@@ -1,4 +1,4 @@
 import sys, pytest, os
 from pytest import mark, raises
 from support import setup_make, pylong
 
@@ -28,8 +28,8 @@
         pythonizor3 = pythonizor1
 
         cppyy.py.add_pythonization(pythonizor1)
         assert cppyy.py.remove_pythonization(pythonizor2) == False
         assert cppyy.py.remove_pythonization(pythonizor3) == True
 
         def pythonizor(klass, name):
             if name == 'pyzables::SomeDummy1':
@@ -113,10 +113,10 @@
         cppyy.gbl.pyzables.GimeDerived.__creates__ = True
 
         result = cppyy.gbl.pyzables.GimeDerived()
         assert type(result) == cppyy.gbl.pyzables.MyDerived
 
         cppyy.py.pin_type(cppyy.gbl.pyzables.MyBase)
         assert type(result) == cppyy.gbl.pyzables.MyDerived
 
 
     def test04_transparency(self):
@@ -370,6 +370,33 @@
         optr.__smartptr__().reset(o2)
         assert optr == o2
 
+    def test11_size_len_pythonization_guards(self):
+        """Verify __len__ is only installed when size() returns int and class is iterable"""
+
+        import cppyy
+
+        # size() returns int AND has begin/end -> __len__ installed
+        obj_int = cppyy.gbl.pyzables.SizeReturnsInt()
+        assert hasattr(obj_int, "__len__")
+        assert len(obj_int) == 3
+        assert bool(obj_int)
+
+        # size() returns non-integer type -> __len__ NOT installed
+        obj_opt = cppyy.gbl.pyzables.SizeReturnsNonInt()
+        assert not hasattr(obj_opt, "__len__")
+        assert bool(obj_opt)  # should be True (valid object)
+
+        # size() returns int but no begin/end or operator[] -> __len__ NOT installed
+        obj_noiter = cppyy.gbl.pyzables.SizeWithoutIterator()
+        assert not hasattr(obj_noiter, "__len__")
+        assert bool(obj_noiter)
+
+        # fully inherited container interface -> __len__ installed via MRO
+        obj_inherited = cppyy.gbl.pyzables.InheritedContainer()
+        assert hasattr(obj_inherited, "__len__")
+        assert len(obj_inherited) == 2
+
+
 ## actual test run
 if __name__ == "__main__":
     exit(pytest.main(args=['-sv', '-ra', __file__]))

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

@@ -1484,16 +1484,12 @@ def test02_STL_like_class_iterators(self):
 
         assert i == len(a)-1
 
-        for cls in [cppyy.gbl.stl_like_class2, cppyy.gbl.stl_like_class3]:
-            b = cls[float, 2]()
-            b[0] = 27; b[1] = 42
-            limit = len(b)+1
-            for x in b:
-                limit -= 1
-                assert limit and "iterated too far!"
-                assert x in [27, 42]
-            assert x == 42
-            del x, b
+        b = cppyy.gbl.stl_like_class2[float, 2]()
+        b[0] = 27
+        b[1] = 42
+        assert len(b) == 2
+        for i in range(len(b)):
+            assert b[i] in [27, 42]
 
         for num in [4, 5, 6, 7]:
             cls = getattr(cppyy.gbl, 'stl_like_class%d' % num)