In a loop with changing content (which seems critical to this and still could be ambiguous) and you want the insert only done if not in the container the first time, saying to skip the find is not always correct.
performance: stlFindInsert - Searching before insertion is not necessary.
Interesting but we are stuck with C++ 14 for quite a while to come but I'd rather have a better check since technically our code is doing what we want.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
In a loop with changing content (which seems critical to this and still could be ambiguous) and you want the insert only done if not in the container the first time, saying to skip the find is not always correct.
performance: stlFindInsert - Searching before insertion is not necessary.
The code as stated above does nothing, since
stringList
is and remains empty. However, this avoids the duplicate lookup and silences the warning:https://godbolt.org/z/Gx1YGf1Kr
I'm sorry, I had a typo above and I should have used == instead of !=
this way stringList does get populated but only once. Both however do produce the same warning.
What you are pointing out about the list always being empty seems like a different problem too that maybe should be caught by some other check?
Ok, in this case the warning can be avoided by using
try_emplace()
(available from C++17).Interesting but we are stuck with C++ 14 for quite a while to come but I'd rather have a better check since technically our code is doing what we want.
We do check for that the key is somewhat trivial in C++14, but we should probably check this on the value as well:
https://github.com/danmar/cppcheck/blob/main/lib/checkstl.cpp#L1571
I don't immediately understand why we check that the key is complex. But do you think that matters in this case?
I have the feeling that we should not warn if the search is in a loop and the map variable is declared before the loop.
It was to fix this issue:
https://trac.cppcheck.net/ticket/9218
But it doesnt actually fix the issue entirely, since it only checks the value.
Yes of course, it's a false positive. I have created https://trac.cppcheck.net/ticket/10558
I have tried to clarify the error message. it will suggest to use map::insert or map::emplace instead. then the check is redundant.
that typo illustrates that it would be good to use insert/emplace. :-)
Last edit: Daniel Marjamäki 2021-10-24