Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-89890

correct use of Q_OBJECT_COMPAT_PROPERTY is difficult

    XMLWordPrintable

Details

    • Bug
    • Resolution: Unresolved
    • P1: Critical
    • None
    • 6.0
    • Core: Object Model
    • 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

        For Gerrit Dashboard: QTBUG-89890
        # Subject Branch Project Status CR V

        Activity

          People

            fabiankosmale Fabian Kosmale
            andreasbuhr Andreas Buhr
            Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:

              Gerrit Reviews

                There are no open Gerrit changes