cppcheck
cppcheck copied to clipboard
danmar
LCppC backports: Several optimizations/refactorizations.
As a follow-up to the discussion in PR4143, I am starting to provide a few fixes/improvements from LCppC to cppcheck.
It's quite a large PR. I would feel happier if split it up. I suggest that we only take the first "43d886bd70e0a8d6b7a59d38ec3f2f88a72ba2a1" to start with. I have a pretty good feeling about that.
I feel like these should maybe not be in a single PR but rather two separate ones. I also sometimes pack too much stuff into them but I try to stay topical (I think 🙄).
Also did you profile the container efficiency. I have been profiling a lot and so far and std::list did not come up as something we need to look at.
I do not see a point in splitting up pull requests (as pull requests are a pain in the ass). It is not meant to be squashed to a single commit when being merged. The commits inside are topical.
Regarding container efficiency: std::list is under most circumstances less efficient than std::vector. Only if you want to delete or insert elements at random positions, or you want to sort it, std::list can be a better choice. In particular, as std::list is a double-linked list, it comes with a huge memory overhead (2*8 bytes overhead per element, plus overhead to to an enormous amount of memory allocations), and a performance penalty when iterating through it as the elements are not in contiguous memory. Here is a very interesting synthetic benchmark on this topic: https://baptiste-wicht.com/posts/2012/12/cpp-benchmark-vector-list-deque.html
However, there is no measurable impact here as these occurences are no hotspots.
I do not see a point in splitting up pull requests (as pull requests are a pain in the ass). It is not meant to be squashed to a single commit when being merged. The commits inside are topical.
In our repo they are being squashed - so the PR needs to be topical and not the commits.
There is no need to do so, iirc github gives you the choice between merging, rebasing and squashing.
well I review it as a whole. I hope you can see the value in making it easy to review the PR.
And if I have a review comment would you update the corresponding commit properly or would you add extra "fix review comment" commits?