whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Disable moving of QProperty 

The semantics are not very intuitive, and it opens a can of worms
with regards to what should happen with observers that observe
that property.

Change-Id: I6fb00b7693904b968224cc87d098bbd0ea776ba3
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
bb10
Lars Knoll 2020-09-28 10:32:00 +02:00 committed by Fabian Kosmale
parent 6b4e0c5803
commit 04641454be
5 changed files with 54 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
src/corelib/kernel/qproperty.cpp
View File

@ -196,39 +196,6 @@ QMetaType QUntypedPropertyBinding::valueMetaType() const
return d->valueMetaType();
}
QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr)
{
std::swap(d_ptr, other.d_ptr);
QPropertyBindingDataPointer d{this};
d.setFirstObserver(nullptr);
if (auto binding = d.bindingPtr())
binding->setProperty(propertyDataPtr);
}
void QPropertyBindingData::moveAssign(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr)
{
if (&other == this)
return;
QPropertyBindingDataPointer d{this};
auto observer = d.firstObserver();
d.setFirstObserver(nullptr);
if (auto binding = d.bindingPtr()) {
binding->unlinkAndDeref();
d_ptr &= FlagMask;
}
std::swap(d_ptr, other.d_ptr);
if (auto binding = d.bindingPtr())
binding->setProperty(propertyDataPtr);
d.setFirstObserver(observer.ptr);
// The caller will have to notify observers.
}
QPropertyBindingData::~QPropertyBindingData()
{
QPropertyBindingDataPointer d{this};
@ -286,6 +253,12 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
return QUntypedPropertyBinding(oldBinding.data());
}
QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other) : d_ptr(std::exchange(other.d_ptr, 0))
{
QPropertyBindingDataPointer d{this};
d.fixupFirstObserverAfterMove();
}
QPropertyBindingPrivate *QPropertyBindingData::binding() const
{
QPropertyBindingDataPointer d{this};
@ -693,12 +666,6 @@ QString QPropertyBindingError::description() const
\a other was pointing to.
*/
/*!
\fn template <typename T> QProperty<T> &QProperty<T>::operator=(QProperty &&other)
Move-assigns \a other to this QProperty instance.
*/
/*!
\fn template <typename T> QProperty<T>::QProperty(const QPropertyBinding<T> &binding)
@ -1395,7 +1362,7 @@ struct QBindingStoragePrivate
if (index == newData->size)
index = 0;
}
new (pp + index) Pair{p->data, QPropertyBindingData(std::move(p->bindingData), p->data)};
new (pp + index) Pair{p->data, QPropertyBindingData(std::move(p->bindingData))};
}
}
// data has been moved, no need to call destructors on old Pairs

10
src/corelib/kernel/qproperty.h
View File

@ -315,7 +315,6 @@ public:
QProperty() = default;
explicit QProperty(parameter_type initialValue) : QPropertyData<T>(initialValue) {}
explicit QProperty(rvalue_ref initialValue) : QPropertyData<T>(std::move(initialValue)) {}
QProperty(QProperty &&other) : QPropertyData<T>(std::move(other.val)), d(std::move(other.d), this) { notify(); }
explicit QProperty(const QPropertyBinding<T> &binding)
: QProperty()
{ setBinding(binding); }
@ -329,13 +328,6 @@ public:
template <typename Functor>
explicit QProperty(Functor &&f);
#endif
QProperty &operator=(QProperty &&other)
{
this->val = std::move(other.val);
d.moveAssign(std::move(other.d), this);
notify();
return *this;
}
~QProperty() = default;
parameter_type value() const
@ -460,7 +452,7 @@ private:
d.notifyObservers(this);
}
Q_DISABLE_COPY(QProperty)
Q_DISABLE_COPY_MOVE(QProperty)
};
namespace Qt {

12
src/corelib/kernel/qproperty_p.h
View File

@ -81,6 +81,7 @@ struct Q_AUTOTEST_EXPORT QPropertyBindingDataPointer
observer->prev = reinterpret_cast<QPropertyObserver**>(&(ptr->d_ptr));
ptr->d_ptr = (reinterpret_cast<quintptr>(observer) & ~QtPrivate::QPropertyBindingData::FlagMask);
}
void fixupFirstObserverAfterMove() const;
void addObserver(QPropertyObserver *observer);
void setFirstObserver(QPropertyObserver *observer);
QPropertyObserverPointer firstObserver() const;
@ -293,6 +294,17 @@ inline void QPropertyBindingDataPointer::setFirstObserver(QPropertyObserver *obs
ptr->d_ptr = reinterpret_cast<quintptr>(observer) | (ptr->d_ptr & QtPrivate::QPropertyBindingData::FlagMask);
}
inline void QPropertyBindingDataPointer::fixupFirstObserverAfterMove() const
{
// If QPropertyBindingData has been moved, and it has an observer
// we have to adjust the firstObesrver's prev pointer to point to
// the moved to QPropertyBindingData's d_ptr
if (ptr->d_ptr & QtPrivate::QPropertyBindingData::BindingBit)
return; // nothing to do if the observer is stored in the binding
if (auto observer = firstObserver())
observer.ptr->prev = reinterpret_cast<QPropertyObserver**>(&(ptr->d_ptr));
}
inline QPropertyObserverPointer QPropertyBindingDataPointer::firstObserver() const
{
if (auto *binding = bindingPtr())

4
src/corelib/kernel/qpropertyprivate.h
View File

@ -88,12 +88,10 @@ class Q_CORE_EXPORT QPropertyBindingData
Q_DISABLE_COPY(QPropertyBindingData)
public:
QPropertyBindingData() = default;
QPropertyBindingData(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr);
QPropertyBindingData(QPropertyBindingData &&other);
QPropertyBindingData &operator=(QPropertyBindingData &&other) = delete;
~QPropertyBindingData();
void moveAssign(QPropertyBindingData &&other, QUntypedPropertyData *propertyDataPtr);
bool hasBinding() const { return d_ptr & BindingBit; }
QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,

132
tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
View File

@ -45,13 +45,9 @@ private slots:
void bindingAfterUse();
void switchBinding();
void avoidDependencyAllocationAfterFirstEval();
void propertyArrays();
void boolProperty();
void takeBinding();
void replaceBinding();
void swap();
void moveNotifies();
void moveCtor();
void changeHandler();
void propertyChangeHandlerApi();
void subscribe();
@ -238,27 +234,6 @@ void tst_QProperty::avoidDependencyAllocationAfterFirstEval()
QCOMPARE(QPropertyBindingDataPointer::get(propWithBinding).bindingPtr()->dependencyObserverCount, 2u);
}
void tst_QProperty::propertyArrays()
{
std::vector<QProperty<int>> properties;
int expectedSum = 0;
for (int i = 0; i < 10; ++i) {
properties.emplace_back(i);
expectedSum += i;
}
QProperty<int> sum([&]() {
return std::accumulate(properties.begin(), properties.end(), 0);
});
QCOMPARE(sum.value(), expectedSum);
properties[4] = properties[4] + 42;
expectedSum += 42;
QCOMPARE(sum.value(), expectedSum);
}
void tst_QProperty::boolProperty()
{
QProperty<bool> first(true);
@ -311,74 +286,6 @@ void tst_QProperty::replaceBinding()
QCOMPARE(second.value(), 100);
}
void tst_QProperty::swap()
{
QProperty<int> firstDependency(1);
QProperty<int> secondDependency(2);
QProperty<int> first(Qt::makePropertyBinding(firstDependency));
QProperty<int> second(Qt::makePropertyBinding(secondDependency));
QCOMPARE(first.value(), 1);
QCOMPARE(second.value(), 2);
std::swap(first, second);
QCOMPARE(first.value(), 2);
QCOMPARE(second.value(), 1);
secondDependency = 20;
QCOMPARE(first.value(), 20);
QCOMPARE(second.value(), 1);
firstDependency = 100;
QCOMPARE(first.value(), 20);
QCOMPARE(second.value(), 100);
}
void tst_QProperty::moveNotifies()
{
QProperty<int> first(1);
QProperty<int> second(2);
QProperty<int> propertyInTheMiddle(Qt::makePropertyBinding(first));
QProperty<int> finalProp1(Qt::makePropertyBinding(propertyInTheMiddle));
QProperty<int> finalProp2(Qt::makePropertyBinding(propertyInTheMiddle));
QCOMPARE(finalProp1.value(), 1);
QCOMPARE(finalProp2.value(), 1);
QCOMPARE(QPropertyBindingDataPointer::get(propertyInTheMiddle).observerCount(), 2);
QProperty<int> other(Qt::makePropertyBinding(second));
QCOMPARE(other.value(), 2);
QProperty<int> otherDep(Qt::makePropertyBinding(other));
QCOMPARE(otherDep.value(), 2);
QCOMPARE(QPropertyBindingDataPointer::get(other).observerCount(), 1);
propertyInTheMiddle = std::move(other);
QCOMPARE(QPropertyBindingDataPointer::get(other).observerCount(), 0);
QCOMPARE(finalProp1.value(), 2);
QCOMPARE(finalProp2.value(), 2);
}
void tst_QProperty::moveCtor()
{
QProperty<int> first(1);
QProperty<int> intermediate(Qt::makePropertyBinding(first));
QCOMPARE(intermediate.value(), 1);
QCOMPARE(QPropertyBindingDataPointer::get(first).observerCount(), 1);
QProperty<int> targetProp(std::move(first));
QCOMPARE(QPropertyBindingDataPointer::get(targetProp).observerCount(), 0);
}
void tst_QProperty::changeHandler()
{
QProperty<int> testProperty(0);
@ -570,6 +477,11 @@ void tst_QProperty::bindingLoop()
class ReallocTester : public QObject
{
/*
* This class and the realloc test rely on the fact that the internal property hashmap has an
* initial capacity of 8 and a load factor of 0.5. Thus, it is necessary to cause actions which
* allocate 5 different QPropertyBindingData
* */
Q_OBJECT
Q_PROPERTY(int prop1 READ prop1 WRITE setProp1 BINDABLE bindableProp1)
Q_PROPERTY(int prop2 READ prop2 WRITE setProp2 BINDABLE bindableProp2)
@ -595,12 +507,34 @@ public:
void tst_QProperty::realloc()
{
ReallocTester tester;
tester.bindableProp1().setBinding([&](){return tester.prop5();});
tester.bindableProp2().setBinding([&](){return tester.prop5();});
tester.bindableProp3().setBinding([&](){return tester.prop5();});
tester.bindableProp4().setBinding([&](){return tester.prop5();});
tester.bindableProp5().setBinding([&]() -> int{return 42;});
{
// Triggering a reallocation does not crash
ReallocTester tester;
tester.bindableProp1().setBinding([&](){return tester.prop5();});
tester.bindableProp2().setBinding([&](){return tester.prop5();});
tester.bindableProp3().setBinding([&](){return tester.prop5();});
tester.bindableProp4().setBinding([&](){return tester.prop5();});
tester.bindableProp5().setBinding([&]() -> int{return 42;});
QCOMPARE(tester.prop1(), 42);
}
{
// After a reallocation, property observers still work
ReallocTester tester;
int modificationCount = 0;
QPropertyChangeHandler observer {[&](){ ++modificationCount; }};
tester.bindableProp1().observe(&observer);
tester.setProp1(12);
QCOMPARE(modificationCount, 1);
QCOMPARE(tester.prop1(), 12);
tester.bindableProp1().setBinding([&](){return tester.prop5();});
tester.bindableProp2().setBinding([&](){return tester.prop5();});
tester.bindableProp3().setBinding([&](){return tester.prop5();});
tester.bindableProp4().setBinding([&](){return tester.prop5();});
tester.bindableProp5().setBinding([&]() -> int{return 42;});
QEXPECT_FAIL("", "ChangeHandler bug with eager properties", Continue);
QCOMPARE(modificationCount, 2);
}
};
void tst_QProperty::changePropertyFromWithinChangeHandler()