[Interest] Warning about QHash keys().toVector()

Oliver Knoll till.oliver.knoll at gmail.com
Thu Jul 8 08:41:13 CEST 2021



>    template<typename K, typename V> static QVector<K>
> keysToVector(QHash<K,V> const& hash)
>    {
>        QVector<K> v;
>        v.reserve(hash.size());
> 
>        for (auto it = hash.keyBegin(); it != hash.keyEnd(); ++it)
>            v.append(*it);
> 
>        return v;
>    }
> 
>    QVector<int> CompatibilityInterfaceImpl::getActionIds() const
>    {
>        return keysToVector(_actions);
>    }

Dear all,

I am sorry to kind of hijack this thread with a follow-up question, but I am trying to better understand what‘s going on „under the C++ hood“, especially given the solution above.

When we look at the original code again:

QVector<int> CompatibilityInterfaceImpl::getActionIds() const
   {
       return _actions.keys().toVector();   // _actions is a QHash
   }

And the warning it generates:

  allocating an unneeded temporary container [clazy-container-anti-pattern]

I am having trouble to see the difference of those two solutions, in the context of the warning.

I do understand that in the original solution a vector is allocated within the toVector() method, which is then returned (by value) and hence copied into a „temporary variable“ (it is not assigned to a declared variable, that is my understanding of „temporary“ - correct?). That „temporary variable“ shortly lives in the scope of getActionIds().

As its value needs to be returned by getActionIds() it (the vector) is copied once more, into the final result variable (of the caller of getActionIds().

So I (naively?) count:

* One vector allocation
* Two vector copies by value

Now in the second solution I count (wrongly?) the same operations:

* One vector allocation (in keysToVector)
* Two vector copies (return by value)

So why then is „return keysToVector()“ more efficient than the first „return map.keys().toVector()“? Is this related to the C++11 „move“ operator where the compiler can „optimise away“ the „copy“ operation in the „return by value“? If that is the case, why can‘t the same optimisation be done in the original case?

Or did I simply miscount the number of allocate/copy operations?

Thank you!
  Oliver



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/interest/attachments/20210708/0a17d745/attachment.html>


More information about the Interest mailing list