diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/Cppyy.h b/bindings/pyroot/cppyy/CPyCppyy/src/Cppyy.h index 5b8c3695926b8..4afc465162cc9 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/Cppyy.h +++ b/bindings/pyroot/cppyy/CPyCppyy/src/Cppyy.h @@ -143,6 +143,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 diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx index 6cd1e1fd80c49..0e48dea853552 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx +++ b/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); @@ -1754,12 +1767,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. 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 (!IsTemplatedSTLClass(name, "vector") && // vector is dealt with below diff --git a/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx b/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx index 84cb3593817d7..b0f6943801bd5 100644 --- a/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx +++ b/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx @@ -1092,6 +1092,18 @@ bool Cppyy::IsAggregate(TCppType_t klass) 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 diff --git a/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/cpp_cppyy.h b/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/cpp_cppyy.h index c3e1785104cdc..3a5967348544c 100644 --- a/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/cpp_cppyy.h +++ b/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/cpp_cppyy.h @@ -119,6 +119,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 diff --git a/bindings/pyroot/cppyy/cppyy/test/pythonizables.h b/bindings/pyroot/cppyy/cppyy/test/pythonizables.h index 1dc56101beee6..b166f18a8456b 100644 --- a/bindings/pyroot/cppyy/cppyy/test/pythonizables.h +++ b/bindings/pyroot/cppyy/cppyy/test/pythonizables.h @@ -166,4 +166,43 @@ class WithCallback3 : public WithCallback2 { void set_int(int i); }; +//=========================================================================== +// 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 diff --git a/bindings/pyroot/cppyy/cppyy/test/stltypes.h b/bindings/pyroot/cppyy/cppyy/test/stltypes.h index c7d8fa8974f3e..a4e51015c9f03 100644 --- a/bindings/pyroot/cppyy/cppyy/test/stltypes.h +++ b/bindings/pyroot/cppyy/cppyy/test/stltypes.h @@ -85,15 +85,6 @@ class stl_like_class2 { value_type& operator[](ptrdiff_t i) { return fData[i]; } }; -template -class stl_like_class3 : public stl_like_class2 { - using stl_like_class2::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 { diff --git a/bindings/pyroot/cppyy/cppyy/test/test_pythonization.py b/bindings/pyroot/cppyy/cppyy/test/test_pythonization.py index fad6a28392e87..725fa09736e86 100644 --- a/bindings/pyroot/cppyy/cppyy/test/test_pythonization.py +++ b/bindings/pyroot/cppyy/cppyy/test/test_pythonization.py @@ -259,6 +259,33 @@ def test09_cpp_side_pythonization(self): assert cppyy.gbl.pyzables.WithCallback2.klass_name == 'pyzables::WithCallback3' + 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__': result = run_pytest(__file__) diff --git a/bindings/pyroot/cppyy/cppyy/test/test_stltypes.py b/bindings/pyroot/cppyy/cppyy/test/test_stltypes.py index ed24707184d9b..7f6514cec4b7b 100644 --- a/bindings/pyroot/cppyy/cppyy/test/test_stltypes.py +++ b/bindings/pyroot/cppyy/cppyy/test/test_stltypes.py @@ -1459,16 +1459,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) diff --git a/graf2d/asimage/inc/TASImage.h b/graf2d/asimage/inc/TASImage.h index 66cea02faadb7..91f718c10fb72 100644 --- a/graf2d/asimage/inc/TASImage.h +++ b/graf2d/asimage/inc/TASImage.h @@ -71,6 +71,7 @@ class TASImage : public TImage { static ASVisual *fgVisual; ///< pointer to visual structure static Bool_t fgInit; ///< global flag to init afterimage only once + static Bool_t fgBatch; ///< global flag to signal if batch mode is active ie fgVisual->dpy was set to nullptr EImageFileTypes GetFileType(const char *ext); void MapFileTypes(EImageFileTypes &type, UInt_t &astype, Bool_t toas = kTRUE); diff --git a/graf2d/asimage/src/TASImage.cxx b/graf2d/asimage/src/TASImage.cxx index 76e4f5f2346d9..1e2e2b0b629c5 100644 --- a/graf2d/asimage/src/TASImage.cxx +++ b/graf2d/asimage/src/TASImage.cxx @@ -123,6 +123,7 @@ extern "C" { ASVisual *TASImage::fgVisual = nullptr; Bool_t TASImage::fgInit = kFALSE; +Bool_t TASImage::fgBatch = kFALSE; static ASFontManager *gFontManager = nullptr; static unsigned long kAllPlanes = ~0; @@ -2200,49 +2201,37 @@ void TASImage::GetZoomPosition(UInt_t &x, UInt_t &y, UInt_t &w, UInt_t &h) const Bool_t TASImage::InitVisual() { - Bool_t inbatch = fgVisual && (fgVisual->dpy == (void*)1); // was in batch - Bool_t noX = gROOT->IsBatch() || gVirtualX->InheritsFrom("TGWin32"); + Bool_t noX = gROOT->IsBatch() || !gVirtualX->InheritsFrom("TGX11"); - // was in batch, but switched to gui - if (inbatch && !noX) { - destroy_asvisual(fgVisual, kFALSE); - fgVisual = nullptr; - } - - if (fgVisual && fgVisual->dpy) { // already initialized + if (fgVisual && (noX == fgBatch)) return kTRUE; - } - // batch or win32 mode - if (!fgVisual && noX) { - fgVisual = create_asvisual(nullptr, 0, 0, nullptr); - fgVisual->dpy = (Display*)1; //fake (not used) - return kTRUE; - } + if (fgVisual) + destroy_asvisual(fgVisual, kFALSE); + fgVisual = nullptr; + fgBatch = false; #ifndef WIN32 -#ifdef R__HAS_COCOA - fgVisual = create_asvisual(nullptr, 0, 0, nullptr); - fgVisual->dpy = (Display*)1; //fake (not used) -#else +#ifndef R__HAS_COCOA Display *disp = (Display*) gVirtualX->GetDisplay(); Int_t screen = gVirtualX->GetScreen(); Int_t depth = gVirtualX->GetDepth(); Visual *vis = (Visual*) gVirtualX->GetVisual(); Colormap cmap = (Colormap) gVirtualX->GetColormap(); - if (!vis || cmap == 0) { - fgVisual = create_asvisual(nullptr, 0, 0, nullptr); - } else { + if (vis && cmap) fgVisual = create_asvisual_for_id(disp, screen, depth, XVisualIDFromVisual(vis), cmap, nullptr); - } #endif -#else - fgVisual = create_asvisual(nullptr, 0, 0, nullptr); - fgVisual->dpy = (Display*)1; //fake (not used) #endif + if (!fgVisual) { + // create dummy fgVisual for batch mode + fgVisual = create_asvisual(nullptr, 0, 0, nullptr); + fgVisual->dpy = nullptr; // fake (not used) + fgBatch = true; + } + return kTRUE; } @@ -6738,4 +6727,3 @@ Int_t TASImage::Idx(Int_t idx) // The size of arrays like fImage->alt.argb32 is fImage->width*fImage->height return TMath::Min(idx,(Int_t)(fImage->width*fImage->height)); } -