From c0716994f3a089b41b5a3d05372f4fc2fb4f51c9 Mon Sep 17 00:00:00 2001
From: Ivan Solovev <ivan.solovev@qt.io>
Date: Wed, 21 Apr 2021 12:52:49 +0200
Subject: [PATCH] Bindable property docs: mention virtual setters and getters

Update the bindable property docs to explain how to deal with virtual
getters and setters.

Task-number: QTBUG-92994
Pick-to: 6.2
Change-Id: I6c29011817e83623414b39afee0f39ad4cc5c1c9
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
---
 .../src/objectmodel/bindableproperties.qdoc   | 33 +++++++++++++++++++
 src/corelib/kernel/qproperty.cpp              | 25 ++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/src/corelib/doc/src/objectmodel/bindableproperties.qdoc b/src/corelib/doc/src/objectmodel/bindableproperties.qdoc
index 1bb20935d0f..5c9685656a2 100644
--- a/src/corelib/doc/src/objectmodel/bindableproperties.qdoc
+++ b/src/corelib/doc/src/objectmodel/bindableproperties.qdoc
@@ -167,6 +167,39 @@
     Here, code triggered in change handlers might use the circle, while it has
     the new radius, but still the old area.
 
+    \section1 Bindable Properties with Virtual Setters and Getters
+
+    Property setters and getters should normally be minimal and do nothing but
+    setting the property; hence it is not normally appropriate for such setters
+    and getters to be virtual. There is nothing it makes sense for the derived
+    class to do.
+
+    However some Qt classes can have properties with virtual setters. When
+    subclassing such a Qt class, overriding such setters requires special care.
+
+    In any case the base implementation \e must be called for the binding to
+    work correctly.
+
+    The following illustrates this approach.
+
+    \badcode
+    void DerivedClass::setValue(int val)
+    {
+        // do something
+        BaseClass::setValue(val);
+        // probably do something else
+    }
+    \endcode
+
+    All the common rules and recomendations regarding writing to bindable
+    properties also apply here. As soon as the base class implementation is
+    called, all the observers are notified about the change to the property.
+    This means that class invariants must be met before calling the base
+    implementation.
+
+    In the rare case where such virtual getters or setters are necessary, the
+    base class should document the requirements it imposes on overrides.
+
     \section1 Formulating a Property Binding
 
     Any C++ expression evaluating to the correct type can be used as a binding
diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index bff65cd165c..d290f0a16ce 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -1425,6 +1425,31 @@ QString QPropertyBindingError::description() const
   be analyzed carefully to comply with the rules given in
   \l {Writing to a Bindable Property}.
 
+  \section2 Properties with Virtual Setters
+
+  Some of the pre-existing Qt classes (for example, \l QAbstractProxyModel)
+  have properties with virtual setters. Special care must be taken when
+  making such properties bindable.
+
+  For the binding to work properly, the property must be correctly handled in
+  all reimplemented methods of each derived class.
+
+  Unless the derived class has access to the underlying property object, the
+  base implementation \e must be called for the binding to work correctly.
+
+  If the derived class can directly access the property instance, there is no
+  need to explicitly call the base implementation, but the property's value
+  \e must be correctly updated.
+
+  Refer to \l {Bindable Properties with Virtual Setters and Getters} for more
+  details.
+
+  In both cases the expected behavior \e must be documented in the property's
+  documentation, so that users can correctly override the setter.
+
+  Properties for which these conditions cannot be met should not be made
+  bindable.
+
   \sa Q_OBJECT_COMPAT_PROPERTY, QObjectBindableProperty, {Qt's Property System}, {Qt Bindable
   Properties}
 */