Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions bindings/pyroot/cppyy/CPyCppyy/src/Cppyy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 43 additions & 6 deletions bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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<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 (!IsTemplatedSTLClass(name, "vector") && // vector is dealt with below
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions bindings/pyroot/cppyy/cppyy/test/pythonizables.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 0 additions & 9 deletions bindings/pyroot/cppyy/cppyy/test/stltypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,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 {
Expand Down
27 changes: 27 additions & 0 deletions bindings/pyroot/cppyy/cppyy/test/test_pythonization.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import py, sys

Check failure on line 1 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E401)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:1:1: E401 Multiple imports on one line help: Split imports
from pytest import mark, raises
from .support import setup_make, pylong

Check failure on line 3 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (F401)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:3:34: F401 `.support.pylong` imported but unused help: Remove unused import: `.support.pylong`

Check failure on line 3 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (I001)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:1:1: I001 Import block is un-sorted or un-formatted help: Organize imports

Expand Down Expand Up @@ -32,8 +32,8 @@
pythonizor3 = pythonizor1

cppyy.py.add_pythonization(pythonizor1)
assert cppyy.py.remove_pythonization(pythonizor2) == False

Check failure on line 35 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E712)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:35:16: E712 Avoid equality comparisons to `False`; use `not cppyy.py.remove_pythonization(pythonizor2):` for false checks help: Replace with `not cppyy.py.remove_pythonization(pythonizor2)`
assert cppyy.py.remove_pythonization(pythonizor3) == True

Check failure on line 36 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E712)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:36:16: E712 Avoid equality comparisons to `True`; use `cppyy.py.remove_pythonization(pythonizor3):` for truth checks help: Replace with `cppyy.py.remove_pythonization(pythonizor3)`

def pythonizor(klass, name):
if name == 'pyzables::SomeDummy1':
Expand Down Expand Up @@ -117,10 +117,10 @@
cppyy.gbl.pyzables.GimeDerived.__creates__ = True

result = cppyy.gbl.pyzables.GimeDerived()
assert type(result) == cppyy.gbl.pyzables.MyDerived

Check failure on line 120 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E721)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:120:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks

cppyy.py.pin_type(cppyy.gbl.pyzables.MyBase)
assert type(result) == cppyy.gbl.pyzables.MyDerived

Check failure on line 123 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E721)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:123:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks


def test04_transparency(self):
Expand All @@ -131,9 +131,9 @@
Countable = cppyy.gbl.pyzables.Countable
mine = cppyy.gbl.pyzables.mine

assert type(mine) == Countable

Check failure on line 134 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E721)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:134:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
assert mine.m_check == 0xcdcdcdcd
assert type(mine.__smartptr__()) == cppyy.gbl.std.shared_ptr(Countable)

Check failure on line 136 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E721)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:136:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
assert mine.__smartptr__().get().m_check == 0xcdcdcdcd
assert mine.say_hi() == "Hi!"

Expand Down Expand Up @@ -171,7 +171,7 @@
Countable = pz.Countable

mine = pz.gime_mine_ptr()
assert type(mine) == Countable

Check failure on line 174 in bindings/pyroot/cppyy/cppyy/test/test_pythonization.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (E721)

bindings/pyroot/cppyy/cppyy/test/test_pythonization.py:174:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
assert mine.m_check == 0xcdcdcdcd
assert type(mine.__smartptr__()) == cppyy.gbl.std.shared_ptr(Countable)
assert mine.__smartptr__().get().m_check == 0xcdcdcdcd
Expand Down Expand Up @@ -259,6 +259,33 @@
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__)
Expand Down
16 changes: 6 additions & 10 deletions bindings/pyroot/cppyy/cppyy/test/test_stltypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions graf2d/asimage/inc/TASImage.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
44 changes: 16 additions & 28 deletions graf2d/asimage/src/TASImage.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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));
}

Loading