Bug 359305

Summary: false positives in container-anti-pattern
Product: [Developer tools] clazy Reporter: Nyall Dawson <nyall.dawson>
Component: generalAssignee: Unassigned bugs mailing-list <unassigned-bugs>
Status: RESOLVED NOT A BUG    
Severity: normal CC: smartins
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Nyall Dawson 2016-02-12 09:32:23 UTC
I'm just testing the new container-anti-pattern check and I get false positives for code like:

 Q_FOREACH ( int id, someMap.keys() )
  {
   ....
  }

and 

someMap.keys().toSet()

I believe these are both false positives 



Reproducible: Always
Comment 1 Sergio Martins 2016-02-12 10:57:25 UTC
Your first example allocates once (due to keys() call), and it should allocate 0.
Use instead: for (auto it = someMap.cbegin(), end = someMap.cend(); it != end; ++it)

Your second example allocates twice (due to keys call) and it should allocate once, construct the set manually instead.

This is what happens when you call keys():
template <class Key, class T>
Q_OUTOFLINE_TEMPLATE QList<Key> QHash<Key, T>::keys() const
{
    QList<Key> res;
    res.reserve(size());
    const_iterator i = begin();
    while (i != end()) {
        res.append(i.key());
        ++i;
    }
    return res;
}

You would be iterating twice instead of iterating the map directly.
I can update the README and make it more clear.
Comment 2 Nyall Dawson 2016-02-12 11:27:54 UTC
Ah, ok. I get it now... Thanks for the pointers!