diff options
| author | Christian Tismer <tismer@stackless.com> | 2022-12-02 13:47:48 +0100 |
|---|---|---|
| committer | Christian Tismer <tismer@stackless.com> | 2022-12-13 11:26:19 +0100 |
| commit | e20e29d1bd03f6ff9e57037d0a7f35bb59604f4e (patch) | |
| tree | 3031f8f9b89879fd27950e63a346651338058ba1 | |
| parent | d09302d50bf64afdf7eae0075134bb554c9d1166 (diff) | |
__feature__: Fix a weird case of false metafunction lookup
PySide implements duck-punching since 2010.
This could create a problem with true_property since 06/2019, because a
meta-function could be found in the instance dict of a QObject class,
although the methods were replaced by a property object.
This was an unexpected reaction of the `getMetaDataFromQObject`
function. Meta methods were created and inserted into the instance
dict, which caused very unrelated side effects like infinite recursion.
The new implementation handles Python properties correctly and looks
up the hidden methods if necessary without side effects.
There are no longer meta functions involved.
The function `getMetaDataFromQObject` is misleading and was replaced
by `getHiddenDataFromQObject`, keeping the old name as an alias.
It will be finally removed in version 6.5 .
[ChangeLog][PySide6] A callback error when using true_property was fixed.
Change-Id: Ie5234eab2106885f6edad24ae7d4c55fff43d62f
Fixes: PYSIDE-1889
Pick-to: 6.4
Task-number: PYSIDE-1019
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
6 files changed, 72 insertions, 6 deletions
diff --git a/sources/pyside6/libpyside/pyside.cpp b/sources/pyside6/libpyside/pyside.cpp index a832db28a..09acfea07 100644 --- a/sources/pyside6/libpyside/pyside.cpp +++ b/sources/pyside6/libpyside/pyside.cpp @@ -490,8 +490,10 @@ void initQApp() setDestroyQApplication(destroyQCoreApplication); } -PyObject *getMetaDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *name) +PyObject *getHiddenDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *name) { + using Shiboken::AutoDecRef; + PyObject *attr = PyObject_GenericGetAttr(self, name); if (!Shiboken::Object::isValid(reinterpret_cast<SbkObject *>(self), false)) return attr; @@ -505,6 +507,7 @@ PyObject *getMetaDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *nam } // Mutate native signals to signal instance type + // Caution: This inserts the signal instance into the instance dict. if (attr && PyObject_TypeCheck(attr, PySideSignal_TypeF())) { auto *inst = Signal::initialize(reinterpret_cast<PySideSignal *>(attr), name, self); PyObject *signalInst = reinterpret_cast<PyObject *>(inst); @@ -517,13 +520,46 @@ PyObject *getMetaDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *nam PyObject *type, *value, *traceback; PyErr_Fetch(&type, &value, &traceback); // This was omitted for a loong time. - const char *cname = Shiboken::String::toCString(name); int flags = currentSelectId(Py_TYPE(self)); int snake_flag = flags & 0x01; + int propFlag = flags & 0x02; + + if (propFlag) { + // PYSIDE-1889: If we have actually a Python property, return f(get|set|del). + // Do not store this attribute in the instance dict, because this + // would create confusion with overload. + // Note: before implementing this property handling, the meta function code + // below created meta functions which was quite wrong. + auto *subdict = _PepType_Lookup(Py_TYPE(self), PyMagicName::property_methods()); + PyObject *propName = PyDict_GetItem(subdict, name); + if (propName) { + // We really have a property name and need to fetch the fget or fset function. + static PyObject *const _fget = Shiboken::String::createStaticString("fget"); + static PyObject *const _fset = Shiboken::String::createStaticString("fset"); + static PyObject *const _fdel = Shiboken::String::createStaticString("fdel"); + static PyObject *const arr[3] = {_fget, _fset, _fdel}; + auto prop = _PepType_Lookup(Py_TYPE(self), propName); + for (int idx = 0; idx < 3; ++idx) { + auto *trial = arr[idx]; + auto *res = PyObject_GetAttr(prop, trial); + if (res) { + AutoDecRef elemName(PyObject_GetAttr(res, PyMagicName::name())); + // Note: This comparison works because of interned strings. + if (elemName == name) + return res; + Py_DECREF(res); + } + PyErr_Clear(); + } + } + } + + const char *cname = Shiboken::String::toCString(name); uint cnameLen = qstrlen(cname); if (std::strncmp("__", cname, 2)) { const QMetaObject *metaObject = cppSelf->metaObject(); QList<QMetaMethod> signalList; + // Caution: This inserts a meta function or a signal into the instance dict. for (int i=0, imax = metaObject->methodCount(); i < imax; i++) { QMetaMethod method = metaObject->method(i); // PYSIDE-1753: Snake case names must be renamed here too, or they will be @@ -560,6 +596,12 @@ PyObject *getMetaDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *nam return attr; } +// PYSIDE-1889: Keeping the old, misleading API for a while. +PyObject *getMetaDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *name) +{ + return getHiddenDataFromQObject(cppSelf, self, name); +} + bool inherits(PyTypeObject *objType, const char *class_name) { if (strcmp(objType->tp_name, class_name) == 0) diff --git a/sources/pyside6/libpyside/pysideqobject.h b/sources/pyside6/libpyside/pysideqobject.h index 96834635d..30dad6e97 100644 --- a/sources/pyside6/libpyside/pysideqobject.h +++ b/sources/pyside6/libpyside/pysideqobject.h @@ -40,12 +40,15 @@ PYSIDE_API bool isQObjectDerived(PyTypeObject *pyType, bool raiseError); /// Convenience to convert a PyObject to QObject PYSIDE_API QObject *convertToQObject(PyObject *object, bool raiseError); -/// Check for properties and signals registered on MetaObject and return these +/// Check for properties and signals registered on MetaObject and return these. +/// Also handle Python properties when true_property was selected. /// \param cppSelf Is the QObject which contains the metaobject /// \param self Python object of cppSelf /// \param name Name of the argument which the function will try retrieve from MetaData /// \return The Python object which contains the Data obtained in metaObject or the Python -/// attribute related with name +/// method pulled out of a Python property. +PYSIDE_API PyObject *getHiddenDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *name); +/// This is an alias, meanwhile misleading: PYSIDE_API PyObject *getMetaDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *name); /// Mutex for accessing QObject memory helpers from multiple threads diff --git a/sources/pyside6/tests/pysidetest/true_property_test.py b/sources/pyside6/tests/pysidetest/true_property_test.py index 671cc4571..0b9a909b7 100644 --- a/sources/pyside6/tests/pysidetest/true_property_test.py +++ b/sources/pyside6/tests/pysidetest/true_property_test.py @@ -33,6 +33,22 @@ class TruePropertyInheritanceTest(UsesQApplication): check = spin_box.sizeHint self.assertEqual(type(check), QSize) + def testHiddenMethods(self): + # PYSIDE-1889: setVisible is no longer a meta function but comes from the Property + widget = QWidget() + self.assertTrue("visible" in QWidget.__dict__) + self.assertFalse("isVisible" in QWidget.__dict__) + self.assertFalse("setVisible" in QWidget.__dict__) + self.assertTrue(hasattr(widget, "isVisible")) + self.assertTrue(hasattr(widget, "setVisible")) + self.assertEqual(widget.isVisible, QWidget.visible.fget) + self.assertEqual(widget.setVisible, QWidget.visible.fset) + # This works with inheritance as well: + class SubClass(QWidget): + pass + sub_widget = SubClass() + self.assertEqual(sub_widget.isVisible, QWidget.visible.fget) + if __name__ == '__main__': unittest.main() diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp index 1b4a04dd4..48006d788 100644 --- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp +++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp @@ -6438,7 +6438,7 @@ QString CppGenerator::qObjectGetAttroFunction() const if (result.isEmpty()) { auto qobjectClass = AbstractMetaClass::findClass(api().classes(), qObjectT()); Q_ASSERT(qobjectClass); - result = u"PySide::getMetaDataFromQObject("_s + result = u"PySide::getHiddenDataFromQObject("_s + cpythonWrapperCPtr(qobjectClass, u"self"_s) + u", self, name)"_s; } diff --git a/sources/shiboken6/libshiboken/bindingmanager.cpp b/sources/shiboken6/libshiboken/bindingmanager.cpp index 9d74e9721..5e884d892 100644 --- a/sources/shiboken6/libshiboken/bindingmanager.cpp +++ b/sources/shiboken6/libshiboken/bindingmanager.cpp @@ -276,6 +276,8 @@ PyObject *BindingManager::getOverride(const void *cptr, auto *obWrapper = reinterpret_cast<PyObject *>(wrapper); auto *wrapper_dict = SbkObject_GetDict_NoRef(obWrapper); if (PyObject *method = PyDict_GetItem(wrapper_dict, pyMethodName)) { + // Note: This special case was implemented for duck-punching, which happens + // in the instance dict. It does not work with properties. Py_INCREF(method); return method; } diff --git a/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/parser.py b/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/parser.py index 5d86b93a5..63127ae93 100644 --- a/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/parser.py +++ b/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/parser.py @@ -70,6 +70,7 @@ def _get_flag_enum_option(): if not isinstance(flag, int): flag = True p = f"\n *** Python is at version {'.'.join(map(str, pyminver or (0,)))} now." + q = f"\n *** PySide is at version {'.'.join(map(str, ver[:2]))} now." # PYSIDE-1797: Emit a warning when we may remove pep384_issue33738.cpp if pyminver and pyminver >= (3, 8): warnings.warn(f"{p} The file pep384_issue33738.cpp should be removed ASAP! ***") @@ -81,7 +82,9 @@ def _get_flag_enum_option(): warnings.warn(f"{p} The files bufferprocs_py37.(cpp|h) should be removed ASAP! ***") # PYSIDE-1735: Emit a warning when we should maybe evict forgiveness mode if ver[:2] >= (7, 0): - warnings.warn(f"{p} Please drop Enum forgiveness mode in `mangled_type_getattro` ***") + warnings.warn(f"{q} Please drop Enum forgiveness mode in `mangled_type_getattro` ***") + if ver[:2] >= (6, 5): + warnings.warn(f"{q} Please drop the misleading function `getMetaDataFromQObject` ***") # normalize the sys attribute setattr(sys, sysname, flag) os.environ[envname] = str(flag) |