Details
-
Bug
-
Resolution: Unresolved
-
P1: Critical
-
None
-
6.0
-
None
Description
Q_OBJECT_COMPAT_PROPERTY should help porting old properties to the new property system. It is intended to be used when complex logic is in setter functions. The problem is the following. Writing some value to a bindable properties is guaranteed to disconnect any existing binding. This is done by the underlying QObjectCompatProperty in its assignment operator (qproperty_p.h:445):
void setValue(parameter_type t) { QBindingStorage *storage = qGetBindingStorage(owner()); auto *bd = storage->bindingData(this); // make sure we don't remove the binding if called from the bindingWrapper const bool inWrapper = inBindingWrapper(storage); if (bd && !inWrapper) bd->removeBinding(); if constexpr (QTypeTraits::has_operator_equal_v<T>) if (this->val == t) return; this->val = t; if (!inWrapper) notify(bd); } QObjectCompatProperty &operator=(parameter_type newValue) { setValue(newValue); return *this; }
However, complex setters might not write to the underlying QObjectCompatProperty. Then bindings are not removed. The following code example illustrates this.
#include <QObject> #include <QDebug> #include <private/qproperty_p.h> class Foo : public QObject { Q_OBJECT Q_PROPERTY(int a READ a WRITE setA BINDABLE bindableA) public: int a() const { return Adata; } void setA(int value) { if(value == Adata) return; Adata = value; // do some important updates after a changed } QBindable<int> bindableA() { return &Adata; } private: Q_OBJECT_COMPAT_PROPERTY(Foo, int, Adata, &Foo::setA) }; int main(){ QProperty<int> refvalue(5); Foo myfoo; myfoo.bindableA().setBinding([&](){ return refvalue.value(); }); // change refvalue, myfoo.a follows refvalue = 6; assert(myfoo.a() == 6); // set myfoo.a to 6, should disconnect property binding (but doesn't) myfoo.setA(6); refvalue = 8; assert(myfoo.a() == 6); // this fails } #include "main.moc"
Most of the time we mess this up when using Q_OBJECT_COMPAT_PROPERTY. I think the problem exists in my patches:
https://codereview.qt-project.org/c/qt/qtbase/+/327044/5
in setModel()
https://codereview.qt-project.org/c/qt/qtbase/+/328471/2
in setWatchedServices()
It is in Sona's patches:
https://codereview.qt-project.org/c/qt/qtbase/+/326866/4/
in setDuration()
https://codereview.qt-project.org/c/qt/qtbase/+/327428/2/
in setDirection()
https://codereview.qt-project.org/c/qt/qtbase/+/326626/8/
in setMaxThreadCount
In Ivan's patches:
https://codereview.qt-project.org/c/qt/qtbase/+/327112/5
in
setSortCaseSensitivity()
setSortRole()
setSortLocaleAware()
setFilterRole()
setRecursiveFilteringEnabled()
setAutoAcceptChildRow()
https://codereview.qt-project.org/c/qt/qtbase/+/327038/6
in setSourceModel()
If no one gets this right, maybe we should reevaluate the design of Q_OBJECT_COMPAT_PROPERTY.
My proposal would be that any write to a bindable property just assigns the underlying QObjectCompatProperty. The old setters become private functions which are then, in turn, called by the QObjectCompatProperty.
Attachments
Gerrit Reviews
For Gerrit Dashboard: QTBUG-89890 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
330521,1 | WIP: Proposal for safer Q_OBJECT_COMPAT_PROPERTY | dev | qt/qtbase | Status: ABANDONED | -2 | 0 |