This will warn for cases where searching in an associative container happens before insertion, like this:
```cpp
void f1(std::set<unsigned>& s, unsigned x) {
if (s.find(x) == s.end()) {
s.insert(x);
}
}
void f2(std::map<unsigned, unsigned>& m, unsigned x) {
if (m.find(x) == m.end()) {
m.emplace(x, 1);
} else {
m[x] = 1;
}
}
```
In the case of the map it could be written as `m[x] = 1` as it will create the key if it doesnt exist, so the extra search is not necessary.
I have this marked as `performance` as it is mostly concerning performance, but there could be a copy-paste error possibly, although I dont think thats common.
Change the astStringVerbose() recursion to extend a string instead of
returning one. This has the benefit that for tokens where the recursion
runs deep (typically large arrays), the time savings can be substantial
(see comments on benchmarks further down).
The reason is that previously, for each token, the astString of its
operands was constructed, and then appended to this tokens astString.
This led to a lot of unnecessary string copying (and with that
allocations). Instead, by passing the string by reference, the number
of temporary strings is greatly reduced.
Another way of seeing it is that previously, the string was constructed
from end to beginning, but now it is constructed from the beginning to
end. There was no notable speedup by preallocating the entire string
using string::reserve() (at least not on Linux).
To benchmark, the changes and master were tested on Linux using the
commands:
make
time cppcheck --debug --verbose $file >/dev/null
i.e., the cppcheck binary was compiled with the settings in the
Makefile. Printing the output to screen or file will of course take
longer time.
In Trac ticket #8355 which triggered this change, an example file from the
Wine repository was attached. Running the above cppcheck on master took
24 minutes and with the changes in this commmit, took 22 seconds.
Another test made was on lib/tokenlist.cpp in the cppcheck repo, which is
more "normal" file. On that file there was no measurable time difference.
A synthetic benchmark was generated to illustrate the effects on dumping
the ast for arrays of different sizes. The generate code looked as
follows:
const int array[] = {...};
with different number of elements. The results are as follows (times are
in seconds):
N master optimized
10 0.1 0.1
100 0.1 0.1
1000 2.8 0.7
2000 19 1.8
3000 53 3.8
5000 350 10
10000 3215 38
As we can see, for small arrays, there is no time difference, but for
large arrays the time savings are substantial.
* Code changes for Token::mImpl optimisation
* Added new TokenImpl optimisation
Moving members to the TokenImpl struct reduces the size of the Token class, which is a fairly significant optimisation. In my testing on Windows with 32-bit Release-PCRE, this change reduced the size of the Token class from 108 bits to 52 bits and reduced run-time of my test case by around 20%.
* Several optimisations
Deleted some code that ran very slowly and did nothing, as there is no need to change a Token's string to null if you are about to delete it.
Added a frontToken to simplifyCalculations to reduce the amount of work it has to do on already-simplified calculations.
Moved template removal to the end of the list as this reduces redundant iteration and saves time.
* Added tok argument to simplifyCalculations
This means callers can avoid unnecessary work if they know which tokens have already been simplified. Passing nullptr indicates the original behaviour (starting from the front of the list).
* Removed mention of member from another change
* Re-added and optimised some code deleted in error
Changing mTemplateInstantiations to a vector avoids the high cost of doing repeated linear searches. Changing how the code iterates through the array was necessary because the vector can be resized at several points during the loop, which breaks existing references and iterators.
* Changed mTemplateInstantiations to a vector
This is an optimisation that makes repeated linear searches of this collection significantly faster.
Also added a copy constructor to TokenAndName so code can make copies of these objects to keep a reference if a vector gets resized.
* A cleaner optimisation to removing template tokens
This reverts the previous change to made mInstantiatedTemplates a vector and the iterator changes to support this, and makes mTypesUsedInTemplateInstantiation so the eraseTokens logic can be unified.
* Reverted vector to list
Also made mTypesUsedInTemplateInstantiation a vector of TokenAndName objects so it can share the same logic as the other members.
* Added member for template simplifier pointer
This can be used more efficiently than marking Tokens with a flag and then searching through all templates to find the one that matches.
* Turned loop inside out
This means we only have to iterate through the std::list once. std::list is very expensive to iterate through.
* Latest code from danmar and fixed optimisations
In particular I have optimised simplifying template instantiation names as this was incredibly slow because of the number of times it had to iterate through the template instantiation list. Previous optimisations to this weren't very effective and broke some edge cases.
* Added changes from danmar
Made mExplicitInstantiationsToDelete a vector of TokenAndName to be consistent with the rest of the members, which are cleaned up very efficiently.
* Tokens can have many templateSimplifierPointers
* templateSimplifierPointers must be kept in sync
* Fixed#8693 (Template specialization: Constructor detected as normal function (functionStatic error))
Refactor template simplifier to remove the existing full specialization
function expandSpecialized and allow full specializations to use the
existing function expandTemplate. The function expandTemplate was
modified to either expand the template like it originally did by copying
it or to modify the template in place. Both instantiated and
uninstantiated full specializations are modified in place. This also
fixes#8692 and probably other related tickets as well.
The function simplifyTemplates now tries twice to simplify templates so
more templates can be simplified. We should try as many times as
necessary to find all possible templates. We can't do that now because
uninstantiated templates are left unchanged. It is relatively straight
forward to have the new code also expand in place uninstantiated
templates with their symbolic types but namespaces are not handled
properly (ticket #8671) and it would introduce regressions.
* Fix travis warnings.
* Fixed#8683 (Using deleted token with multiple template instantiations.)
* Fixed#8321 (heap use after free: templatesimplifier)
* Add a flag to Token indicating that it has a pointer to it.
* Run dmake
* Fix one source of list pointers to deleted tokens.
Refactor TemplateSimplifier class to get access to template lists.
Remove many function parameters now that they are class variables.
Fix one source of list pointers to deleted tokens.
Add tests with no output to catch crashes.
* Run dmake again.
* Make 2 more functions private.
* Make requested changes.
* Missed one change request.
* Use TokenList rather than Tokenizer.
* Move TokenAndName constructor to cpp file so token.h is not needed in header file.
* Added declaration for deletePrevious function
* Added definition for deletePrevious function
* Fixed crash from deleteThis invalidating pointers
The crash was caused by deleteThis() invalidating the pointer to a constant variable usage. This happened when a usage followed an assignment. This fixes bug #8579.
* Added tokensFront to match tokensBack
This means deletePrevious can set the list's front if necessary.
* Initialised tokensFront in appropriate places
* Switched to using default Token constructor
* Switched to using Token default constructor
* Switched to using default constructor for Token
* Added missing argument to Token constructor
* Changed to use default constructor for Tokens
* Switched to using default constructor for Tokens
* Switched to using default constructor for Token
* Added new test for deleting front Token
Also made sure to use the correct constructor for Token in other tests.
* Syntax error
* Replaced tokensFront and tokensBack with a struct
This decreases the size of the Token class for performance purposes.
* Replaced tokensFront and tokensBack with a struct
* Added tokensFrontBack to destructor
* Reworked to use TokensBackFront struct
Also ran astyle.
* Reworked to use TokenList's TokensFrontBack member
* Reworked to use TokensFrontBack struct
* Reworked to use TokensFrontBack struct
* Reworked to work with TokensFrontBack struct
* Removed unnecessary scope operator
* Added missing parentheses
* Fixed syntax error
* Removed unnecessary constructor
* Default constructor now 0-initialises everything
This is safer for not using a temporary TokensFrontBack object, and doesn't use delegating constructors which aren't supported yet.
* Fixed unsafe null check
* Added missing explicit keyword
* Fixing stylistic nits
Removed default constructor as it has been superseded by the single-argument constructor with a default argument value.
Renamed listEnds to tokensFrontBack.
Fixed if statement that was supposed to be adding safety but would actually cause a crash if tokensFrontBack was null.
* Fixing stylistic nits
Removed default constructor and replaced it with a single-argument constructor with a default value.
* Fixing stylistic nits
Renamed _listEnds to _tokensFrontBack.
* Fixing stylistic nits
Renamed _listEnds to _tokensFrontBack.
- Merged messages exceptThrowInNoexecptFunction, exceptThrowInNoThrowFunction, exceptThrowInAttributeNoThrowFunction and exceptThrowInDeclspecNoThrowFunction into a single message.
- Merged Token::fIsDeclspecNothrow into Token::fIsAttributeNothrow
- Encapsulate parameters passed through AST compiler functions in a struct
-> Reduces maintenance overhead when changing them
-> Contains parameter indicating if the file is C or C++
- Added eKeyword Token type (at the moment, only true for C++ keyword "delete", but should be set for other keywords as well)
-> Performance gain of up to 15% on entire checking time (depends on setup; Result was checked with VS12 (x64), matchcompiled version, ran on tinyxml and cppcheck itself)
- Properly set Token::_type to eVariable, eFunction or eName, depending on _function, _variable or _varId being set.
- Token can't be a variable and a function at a time - put Token::_function and Token::_variable into a union.
Paul Fultz II
Rikard Falkeborn
Daniel Marjamäki
rebnridgway
IOBYTE
amai2012
orbitcowboy
orbitcowboy
Florin Iucha
Ayaz Salikhov
Ayaz Salikhov
Alexander Mai
Matthias Krüger
PKEuS
Frank Zingsheim
Lauri Nurmi
PKEuS
Dmitry-Me
PKEuS
Simon Martin
XhmikosR
Ettl Martin
Zachary Blair