This is a step onto leveraging the `ThreadExecutor` implementation for
`ProcessExecutor` which is a follow-up to #4870. We need to have the
proper test coverage and the existing implementations working as
expected before we move to the shared code.
Fixes:
- added `--showtime=` tests for all executor implementations
- only print `--showtime=summary` once at the end
- prevents `--showtime=` by multiple threads to be written at the same
time - essentially breaking the output
- reset the timer results before each test
- deprecated `top5` in favor of `top5_file`
- fixed printing for all executors except `ProcessExecutor`
This makes the code much more readable. It also makes it less prone to
errors because we do not need to specify the length of the string to
match and the returnvalue is clear.
The code with the bad returnvalue check was never executed and I added a
test to show that.
Encountered while investigating https://trac.cppcheck.net/ticket/11708.
This has been like this since the introduction of `internalError` in
b6bcdf2936 (almost ten years ago to the
day). Logging internal errors which bail out(!) of the analysis simply
to `std::cout` for them possibly never to be seen (and also not affected
the exitcode) is pretty bad IMO. They should always be visible.
I also removed the filename from the message as it is already available
(and thus redundant) and its existence should be defined by the
template.
Scanning the `cli` folder with `DISABLE_VALUEFLOW=1` `Tokenizer::dump()`
will consume almost 25% of the total Ir count when an addon is
specified. This is mainly caused by the usage of `std::ostream`.
Encountered while profiling #4958.
There was no need for the `Tokenizer` parameter to be a pointer as it
could never be `nullptr` and was also dereferenced without checking
first.
As a reference to the `Settings` was already available via the
`Tokenizer` there was no need to pass it separately. In the production
code there will only be one instance of it but in the tests we could
have accidentally passed a different one.
It was also used inconsistently and seemed to imply there is some
special handling which wasn't the case. It was just an alias for
`std::to_string()` for non-`double` types. So there was no need for it.
---------
Co-authored-by: Robert Reif <reif@earthlink.net>
This avoid lots of unchecked pointer dereferences.
There was a single case which checked it and that looked like a
leftover. The only way this might have been a `nullptr` pointer was
through several default constructors which were not used at all so I
removed them.
* Suppressions: merged `isSuppressedLocal()` into `isSuppressed()`
* avoid some unnecessary copies when adding suppressions
* TestSuppressions: improved readability of multiple line string literals
* supressions.h: got rid of unnecessary copy and assignment operators for `Suppressions::Suppression` - fixes `performance-move-const-arg` clang-tidy warning
* TestSuppressions: cleaned up a variable construction
* moved more code into `HAVE_RULES`
* properly enable all internal and rules code in selfcheck
* updated TODOs in selfcheck
* testrunner.vcxproj.filters: updated
* added missing `Debug-PCRE` and `Release-PCRE` configurations for `testrunner` Visual Studio project
* cppcheck.cpp: fixed `useStlAlgorithm` selfcheck warning
* Preprocessor: cleaned up `missingInclude()`
* Preprocessor: relaxed dependency on `Suppressions` / adjusted `TestPreProcessor::inline_suppression_for_missing_include()` which was not testing production behavior
* test/cli/test-other.py: added test for `missingInclude` and `missingIncludeSystem` inline suppressions
* fixed `constParameterReference` selfcheck warning
* added `Settings::useSingleJob()` and use it instead of checking `jobs` or `jointSuppressionReport`
* extracted single job execution into `SingleExecutor`
* moved `reportStatus()` from `CppCheckExecutor` to Èxecutor
* TestSingleExecutor: improved tests
* added testing of markup extension handling in executors
* cleaned up includes based on `include-what-you-use`
* testsingleexecutor.cpp: suppress `performance-unnecessary-value-param` clang-tidy warnings
* ProcessExecutor: send color via pipe instead of applying it beforehand
* do not unconditionally apply colors to output / disable all colors in tests / adjusted tests for changed output behavior
* fixed precision loss in `Executor::reportStatus()`
* fixed `naming-varname` selfcheck warnings
* dump: Fix concurrency problem with dump files
This adds the process ID for the cppcheck process to the filenames
of the .dump and .ctu-info files that the process generates.
So
lib/cppcheck.cpp.dump
becomes
lib/cppcheck.cpp.<PID>.dump
For example:
lib/cppcheck.cpp.2637871.dump
The reason for this change is that if there is a buildsystem which
supports concurrency, multiple instances of cppcheck may be run for
the same file. For example, if the same file is compiled in multiple
build variants, or for multiple targets.
If running the MISRA plugin over such a project with concurrency
enabled in the buildsystem, the plugin ends up crashing as multiple
jobs attempt to create/trample/delete the same files while other
jobs are using them.
For more information see:
https://sourceforge.net/p/cppcheck/discussion/general/thread/02c757b4af/
* dump: Include pid in filename if dump not explicit
Only change the dump and ctu-info filenames to include the PID if
they are being generated due to an addon.
This means that existing scripts that use `--dump` will still work
if they depend on the previous naming behaviour. The more robust
filenames containing the pid will be used when the dump files are
used as an internal implementation detail for passing data to addons.
However this means that anything that does explicitly use `--dump`
will be susceptible to concurrency problems.
* test: Update addon dump file test to account for pid
This test causes a dump file to be created by enabling the misra
addon. Since the dump files now include the cppcheck process pid
this test had to be updated to account for the change.
Oliver Stöneberg
andymacg
Daniel Marjamäki
chrchr-github
Paul Fultz II
Michael Drake