Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. c++11 range-loop might detach Qt container (QSet) [clazy-range-loop-detach]
Forum Updated to NodeBB v4.3 + New Features

c++11 range-loop might detach Qt container (QSet) [clazy-range-loop-detach]

Scheduled Pinned Locked Moved Solved General and Desktop
12 Posts 3 Posters 4.3k Views 3 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • JonBJ JonB

    @opencode said in c++11 range-loop might detach Qt container (QSet) [clazy-range-loop-detach]:

    Any idea how to solve it?

    1. Since you choose not to show your source code where it's complaining, you'd like someone to tell you what's wrong it without being allowed to see it, right?

    2. To solve it change your code like the error message tells you to. It instructs you exactly what to do. That's why the message is what it is.

    O Offline
    O Offline
    opencode
    wrote on last edited by opencode
    #3

    @JonB
    I am sorry, forgot to paste the code. I don't understand how to fix it from the warning message.

    void Scanner::run()
    {
        qDebug() << "thread" << QThread::thread()->currentThreadId() << "started";
    
        while (m_bRunning)
        {
    
            QSet<QString> arrFiles = QDir(m_strPath).entryList({"*.txt","*.dat"}).toSet();
    
            // report all changes and store the current state
            for (const QString &strFile : (arrFiles - m_arrLastFiles))
            {
                emit Change(QString(tr("File %1 was added")).arg(strFile));
            }
            for (const QString &strFile : (m_arrLastFiles - arrFiles))
            {
                emit Change(QString(tr("File %1 was removed")).arg(strFile));
            }
            m_arrLastFiles = arrFiles;
    
            QThread::sleep(1);
        }
    }
    
    
    JonBJ Paul ColbyP 2 Replies Last reply
    0
    • O opencode

      @JonB
      I am sorry, forgot to paste the code. I don't understand how to fix it from the warning message.

      void Scanner::run()
      {
          qDebug() << "thread" << QThread::thread()->currentThreadId() << "started";
      
          while (m_bRunning)
          {
      
              QSet<QString> arrFiles = QDir(m_strPath).entryList({"*.txt","*.dat"}).toSet();
      
              // report all changes and store the current state
              for (const QString &strFile : (arrFiles - m_arrLastFiles))
              {
                  emit Change(QString(tr("File %1 was added")).arg(strFile));
              }
              for (const QString &strFile : (m_arrLastFiles - arrFiles))
              {
                  emit Change(QString(tr("File %1 was removed")).arg(strFile));
              }
              m_arrLastFiles = arrFiles;
      
              QThread::sleep(1);
          }
      }
      
      
      JonBJ Offline
      JonBJ Offline
      JonB
      wrote on last edited by JonB
      #4

      @opencode
      Well, not that I have tried it, but I believe it is asking you change the right-hand side range expression in your loops:

      for (const QString &strFile : qAsConst(arrFiles - m_arrLastFiles))
      
      for (const QString &strFile : qAsConst(m_arrLastFiles - arrFiles))
      

      Without the qAsConst() marker the danger is that those expressions will cause a "detach" --- i.e. copy --- of the elements comprising the range. This would still work, but is inefficient: the compiler knows from your const iterators that you will only be reading. not altering the range elements so suggests you insert the qAsConst() which will allow the range elements to be accessed in the original container without unnecessary copying.

      1 Reply Last reply
      0
      • O opencode

        @JonB
        I am sorry, forgot to paste the code. I don't understand how to fix it from the warning message.

        void Scanner::run()
        {
            qDebug() << "thread" << QThread::thread()->currentThreadId() << "started";
        
            while (m_bRunning)
            {
        
                QSet<QString> arrFiles = QDir(m_strPath).entryList({"*.txt","*.dat"}).toSet();
        
                // report all changes and store the current state
                for (const QString &strFile : (arrFiles - m_arrLastFiles))
                {
                    emit Change(QString(tr("File %1 was added")).arg(strFile));
                }
                for (const QString &strFile : (m_arrLastFiles - arrFiles))
                {
                    emit Change(QString(tr("File %1 was removed")).arg(strFile));
                }
                m_arrLastFiles = arrFiles;
        
                QThread::sleep(1);
            }
        }
        
        
        Paul ColbyP Offline
        Paul ColbyP Offline
        Paul Colby
        wrote on last edited by
        #5

        Hi @opencode,

        @JonB is correct, those two lines will cause unnecessary deep copies.

        Unfortunately, you can't fix that warning with qAsConst:

        because it cannot be efficiently implemented for rvalues without leaving dangling references.

        For this reason Qt deletes the relevant template instantiation. So, you're left with either explicitly casting the temporary to be const, or assign to a const variable.

        // Warning: c++11 range-loop might detach Qt container (QSet)
        for (const QString &strFile : (arrFiles - m_arrLastFiles));
        
        // Error: Call to deleted function 'qAsConst'
        for (const QString &strFile : qAsConst(arrFiles - m_arrLastFiles));
        
        // Compiles ok.
        for (const QString &strFile : static_cast<const QSet<QString>>(arrFiles - m_arrLastFiles))
        
        // Ok.
        const auto set = (arrFiles - m_arrLastFiles);
        for (const QString &strFile : set);
        

        Cheers.

        JonBJ O 2 Replies Last reply
        3
        • Paul ColbyP Paul Colby

          Hi @opencode,

          @JonB is correct, those two lines will cause unnecessary deep copies.

          Unfortunately, you can't fix that warning with qAsConst:

          because it cannot be efficiently implemented for rvalues without leaving dangling references.

          For this reason Qt deletes the relevant template instantiation. So, you're left with either explicitly casting the temporary to be const, or assign to a const variable.

          // Warning: c++11 range-loop might detach Qt container (QSet)
          for (const QString &strFile : (arrFiles - m_arrLastFiles));
          
          // Error: Call to deleted function 'qAsConst'
          for (const QString &strFile : qAsConst(arrFiles - m_arrLastFiles));
          
          // Compiles ok.
          for (const QString &strFile : static_cast<const QSet<QString>>(arrFiles - m_arrLastFiles))
          
          // Ok.
          const auto set = (arrFiles - m_arrLastFiles);
          for (const QString &strFile : set);
          

          Cheers.

          JonBJ Offline
          JonBJ Offline
          JonB
          wrote on last edited by JonB
          #6

          @Paul-Colby
          Thanks for clarifying, Paul. Shame the error message here suggests something which you cannot actually do :)

          I am just musing on this. If I understand correctly, the two variables in the range are QSets and - (minus) returns the difference set (first set with second set's items removed). I'm not sure how this would ever reference the original container, I would have thought it has to produce a new set anyway?

          Paul ColbyP 1 Reply Last reply
          0
          • O opencode has marked this topic as solved on
          • Paul ColbyP Paul Colby

            Hi @opencode,

            @JonB is correct, those two lines will cause unnecessary deep copies.

            Unfortunately, you can't fix that warning with qAsConst:

            because it cannot be efficiently implemented for rvalues without leaving dangling references.

            For this reason Qt deletes the relevant template instantiation. So, you're left with either explicitly casting the temporary to be const, or assign to a const variable.

            // Warning: c++11 range-loop might detach Qt container (QSet)
            for (const QString &strFile : (arrFiles - m_arrLastFiles));
            
            // Error: Call to deleted function 'qAsConst'
            for (const QString &strFile : qAsConst(arrFiles - m_arrLastFiles));
            
            // Compiles ok.
            for (const QString &strFile : static_cast<const QSet<QString>>(arrFiles - m_arrLastFiles))
            
            // Ok.
            const auto set = (arrFiles - m_arrLastFiles);
            for (const QString &strFile : set);
            

            Cheers.

            O Offline
            O Offline
            opencode
            wrote on last edited by
            #7

            @Paul-Colby The warnning message in Qt Creator desapeared. Thank you. :)

            1 Reply Last reply
            0
            • JonBJ JonB

              @Paul-Colby
              Thanks for clarifying, Paul. Shame the error message here suggests something which you cannot actually do :)

              I am just musing on this. If I understand correctly, the two variables in the range are QSets and - (minus) returns the difference set (first set with second set's items removed). I'm not sure how this would ever reference the original container, I would have thought it has to produce a new set anyway?

              Paul ColbyP Offline
              Paul ColbyP Offline
              Paul Colby
              wrote on last edited by
              #8

              Hi @JonB,

              I would have thought it has to produce a new set anyway?

              That's correct, the compiler creates a temporary QSet, and that's that QSet that the warning is referring to.

              Consider this pseudo-code:

              for (const auto &value: (QSet<T> temporarySet = (setA - setB)));
              

              When the compiler says "range-loop might detach Qt container (QSet)", the QSet it's referring to is the temporarySet, not the variable setA or setB.

              QSet::operator-() returns a non-const QSet<T>, so even if setA and setB are both const, the temporary rvalue temporarySet is non-const, so the compiler picks the non-const QSet::begin() overload and Qt detaches to ensure the returned temporarySet (non-const) iterator can't accidentally modify setA within the loop.

              By simply assigning the temporary set to a const variable first, or casting to add const, the compiler is forced to use the QSet::begin() const overload instead, which returns a QSet::const_iterator, guaranteeing that the set won't be modified inside the loop (not by the iterator, anyway), so Qt doesn't need to detach (can continue implicitly sharing the underlying data).

              I hope that explains it a little better :)

              Cheers.

              JonBJ 1 Reply Last reply
              1
              • Paul ColbyP Paul Colby

                Hi @JonB,

                I would have thought it has to produce a new set anyway?

                That's correct, the compiler creates a temporary QSet, and that's that QSet that the warning is referring to.

                Consider this pseudo-code:

                for (const auto &value: (QSet<T> temporarySet = (setA - setB)));
                

                When the compiler says "range-loop might detach Qt container (QSet)", the QSet it's referring to is the temporarySet, not the variable setA or setB.

                QSet::operator-() returns a non-const QSet<T>, so even if setA and setB are both const, the temporary rvalue temporarySet is non-const, so the compiler picks the non-const QSet::begin() overload and Qt detaches to ensure the returned temporarySet (non-const) iterator can't accidentally modify setA within the loop.

                By simply assigning the temporary set to a const variable first, or casting to add const, the compiler is forced to use the QSet::begin() const overload instead, which returns a QSet::const_iterator, guaranteeing that the set won't be modified inside the loop (not by the iterator, anyway), so Qt doesn't need to detach (can continue implicitly sharing the underlying data).

                I hope that explains it a little better :)

                Cheers.

                JonBJ Offline
                JonBJ Offline
                JonB
                wrote on last edited by JonB
                #9

                @Paul-Colby said in c++11 range-loop might detach Qt container (QSet) [clazy-range-loop-detach]:

                When the compiler says "range-loop might detach Qt container (QSet)", the QSet it's referring to is the temporarySet, not the variable setA or setB.

                Yes, this is what I thought.

                It would be interesting to see (I have not tried it) whether the end result is that the compiler actually generates any different code for const auto set = (arrFiles - m_arrLastFiles); vs auto set = (arrFiles - m_arrLastFiles); followed by the for loop in this case :)

                O 1 Reply Last reply
                0
                • JonBJ JonB

                  @Paul-Colby said in c++11 range-loop might detach Qt container (QSet) [clazy-range-loop-detach]:

                  When the compiler says "range-loop might detach Qt container (QSet)", the QSet it's referring to is the temporarySet, not the variable setA or setB.

                  Yes, this is what I thought.

                  It would be interesting to see (I have not tried it) whether the end result is that the compiler actually generates any different code for const auto set = (arrFiles - m_arrLastFiles); vs auto set = (arrFiles - m_arrLastFiles); followed by the for loop in this case :)

                  O Offline
                  O Offline
                  opencode
                  wrote on last edited by
                  #10

                  @JonB Hi John, how do I find generated code in Qt Creator by compiler? I have no idea. The variable const auto set = (arrFiles - m_arrLastFiles); helped to solve the issue.

                  JonBJ 1 Reply Last reply
                  0
                  • O opencode

                    @JonB Hi John, how do I find generated code in Qt Creator by compiler? I have no idea. The variable const auto set = (arrFiles - m_arrLastFiles); helped to solve the issue.

                    JonBJ Offline
                    JonBJ Offline
                    JonB
                    wrote on last edited by
                    #11

                    @opencode Hey, don't worry about it, it was idle curiosity :) Stick to what you're doing.

                    O 1 Reply Last reply
                    0
                    • JonBJ JonB

                      @opencode Hey, don't worry about it, it was idle curiosity :) Stick to what you're doing.

                      O Offline
                      O Offline
                      opencode
                      wrote on last edited by
                      #12

                      @JonB :]

                      1 Reply Last reply
                      0

                      • Login

                      • Login or register to search.
                      • First post
                        Last post
                      0
                      • Categories
                      • Recent
                      • Tags
                      • Popular
                      • Users
                      • Groups
                      • Search
                      • Get Qt Extensions
                      • Unsolved