Skip to content

Commit fc700b6

Browse files
firewavefirewave
authored
refs cppcheck-opensource#4452 / refs #11705 - improved --showtime= behavior and testing (cppcheck-opensource#4876)
This is a step onto leveraging the `ThreadExecutor` implementation for `ProcessExecutor` which is a follow-up to cppcheck-opensource#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`
1 parent f1f7408 commit fc700b6

24 files changed

Lines changed: 453 additions & 52 deletions

‎.github/workflows/CI-unixish.yml‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -384,10 +384,6 @@ jobs:
384384
env:
385385
STRICT: 1
386386

387-
- name: Run showtimetop5 tests
388-
run: |
389-
./tools/test_showtimetop5.sh
390-
391387
- name: Run --dump test
392388
run: |
393389
./cppcheck test/testpreprocessor.cpp --dump
@@ -470,7 +466,7 @@ jobs:
470466
471467
- name: Self check
472468
run: |
473-
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5 -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=1 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=gnu --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings --check-level=exhaustive"
469+
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5_summary -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=1 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=gnu --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings --check-level=exhaustive"
474470
ec=0
475471
476472
# TODO: add --check-config

‎.github/workflows/asan.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ jobs:
8989
- name: Self check
9090
if: false
9191
run: |
92-
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5 -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=1 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=cppcheck-lib --library=gnu -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings"
92+
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5_summary -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=1 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=cppcheck-lib --library=gnu -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings"
9393
ec=0
9494
./cmake.output/bin/cppcheck $selfcheck_options --addon=naming.json cli lib || ec=1
9595
./cmake.output/bin/cppcheck $selfcheck_options -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 -DQT_CHARTS_LIB --library=qt --addon=naming.json -Icmake.output/gui -Igui gui/*.cpp cmake.output/gui/*.cpp || ec=1

‎.github/workflows/tsan.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ jobs:
8989
- name: Self check
9090
if: false
9191
run: |
92-
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5 -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=0 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=gnu --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings"
92+
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5_summary -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=0 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=gnu --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings"
9393
ec=0
9494
./cmake.output/bin/cppcheck $selfcheck_options --addon=naming.json -DCHECK_INTERNAL cli lib || ec=1
9595
./cmake.output/bin/cppcheck $selfcheck_options -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 -DQT_CHARTS_LIB --library=qt --addon=naming.json -Icmake.output/gui -Igui gui/*.cpp cmake.output/gui/*.cpp || ec=1

‎.github/workflows/ubsan.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ jobs:
8585
# TODO: only fail the step on sanitizer issues - since we use processes it will only fail the underlying process which will result in an cppcheckError
8686
- name: Self check
8787
run: |
88-
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5 -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=1 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=gnu --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings"
88+
selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5_summary -D__CPPCHECK__ -D__GNUC__ -DCHECK_INTERNAL -DHAVE_RULES --error-exitcode=1 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=gnu --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings"
8989
ec=0
9090
./cmake.output/bin/cppcheck $selfcheck_options --addon=naming.json cli lib || ec=1
9191
./cmake.output/bin/cppcheck $selfcheck_options -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 -DQT_CHARTS_LIB --library=qt --addon=naming.json -Icmake.output/gui -Igui gui/*.cpp cmake.output/gui/*.cpp || ec=1

‎Makefile‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -664,19 +664,19 @@ cli/filelister.o: cli/filelister.cpp cli/filelister.h lib/config.h lib/path.h li
664664
cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h
665665
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/main.cpp
666666

667-
cli/processexecutor.o: cli/processexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
667+
cli/processexecutor.o: cli/processexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
668668
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/processexecutor.cpp
669669

670-
cli/singleexecutor.o: cli/singleexecutor.cpp cli/executor.h cli/singleexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
670+
cli/singleexecutor.o: cli/singleexecutor.cpp cli/executor.h cli/singleexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
671671
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/singleexecutor.cpp
672672

673673
cli/stacktrace.o: cli/stacktrace.cpp cli/stacktrace.h lib/config.h lib/utils.h
674674
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/stacktrace.cpp
675675

676-
cli/threadexecutor.o: cli/threadexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
676+
cli/threadexecutor.o: cli/threadexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
677677
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/threadexecutor.cpp
678678

679-
test/fixture.o: test/fixture.cpp externals/tinyxml2/tinyxml2.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h test/options.h test/redirect.h
679+
test/fixture.o: test/fixture.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h test/options.h test/redirect.h
680680
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/fixture.cpp
681681

682682
test/helpers.o: test/helpers.cpp cli/filelister.h externals/simplecpp/simplecpp.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/helpers.h

‎cli/cmdlineparser.cpp‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -817,12 +817,22 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
817817
mSettings.showtime = SHOWTIME_MODES::SHOWTIME_FILE_TOTAL;
818818
else if (showtimeMode == "summary")
819819
mSettings.showtime = SHOWTIME_MODES::SHOWTIME_SUMMARY;
820-
else if (showtimeMode == "top5")
821-
mSettings.showtime = SHOWTIME_MODES::SHOWTIME_TOP5;
822-
else if (showtimeMode.empty())
820+
else if (showtimeMode == "top5") {
821+
mSettings.showtime = SHOWTIME_MODES::SHOWTIME_TOP5_FILE;
822+
mLogger.printMessage("--showtime=top5 is deprecated and will be removed in Cppcheck 2.14. Please use --showtime=top5_file or --showtime=top5_summary instead.");
823+
}
824+
else if (showtimeMode == "top5_file")
825+
mSettings.showtime = SHOWTIME_MODES::SHOWTIME_TOP5_FILE;
826+
else if (showtimeMode == "top5_summary")
827+
mSettings.showtime = SHOWTIME_MODES::SHOWTIME_TOP5_SUMMARY;
828+
else if (showtimeMode == "none")
823829
mSettings.showtime = SHOWTIME_MODES::SHOWTIME_NONE;
830+
else if (showtimeMode.empty()) {
831+
mLogger.printError("no mode provided for --showtime");
832+
return false;
833+
}
824834
else {
825-
mLogger.printError("unrecognized showtime mode: \"" + showtimeMode + "\". Supported modes: file, file-total, summary, top5.");
835+
mLogger.printError("unrecognized --showtime mode: '" + showtimeMode + "'. Supported modes: file, file-total, summary, top5, top5_file, top5_summary.");
826836
return false;
827837
}
828838
}
@@ -1268,6 +1278,22 @@ void CmdLineParser::printHelp()
12681278
" --rule-file=<file> Use given rule file. For more information, see:\n"
12691279
" http://sourceforge.net/projects/cppcheck/files/Articles/\n"
12701280
#endif
1281+
" --showtime=<mode> Show timing information.\n"
1282+
" The available modes are:\n"
1283+
" * none\n"
1284+
" Show nothing (default)\n"
1285+
" * file\n"
1286+
" Show for each processed file\n"
1287+
" * file-total\n"
1288+
" Show total time only for each processed file\n"
1289+
" * summary\n"
1290+
" Show a summary at the end\n"
1291+
" * top5_file\n"
1292+
" Show the top 5 for each processed file\n"
1293+
" * top5_summary\n"
1294+
" Show the top 5 summary at the end\n"
1295+
" * top5\n"
1296+
" Alias for top5_file (deprecated)\n"
12711297
" --std=<id> Set standard.\n"
12721298
" The available options are:\n"
12731299
" * c89\n"

‎cli/processexecutor.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "importproject.h"
2929
#include "settings.h"
3030
#include "suppressions.h"
31+
#include "timer.h"
3132

3233
#include <algorithm>
3334
#include <numeric>
@@ -375,6 +376,9 @@ unsigned int ProcessExecutor::check()
375376
}
376377
}
377378

379+
// TODO: wee need to get the timing information from the subprocess
380+
if (mSettings.showtime == SHOWTIME_MODES::SHOWTIME_SUMMARY || mSettings.showtime == SHOWTIME_MODES::SHOWTIME_TOP5_SUMMARY)
381+
CppCheck::printTimerResults(mSettings.showtime);
378382

379383
return result;
380384
}

‎cli/singleexecutor.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "importproject.h"
2323
#include "library.h"
2424
#include "settings.h"
25+
#include "timer.h"
2526

2627
#include <cassert>
2728
#include <list>
@@ -110,5 +111,8 @@ unsigned int SingleExecutor::check()
110111
if (mCppcheck.analyseWholeProgram())
111112
result++;
112113

114+
if (mSettings.showtime == SHOWTIME_MODES::SHOWTIME_SUMMARY || mSettings.showtime == SHOWTIME_MODES::SHOWTIME_TOP5_SUMMARY)
115+
CppCheck::printTimerResults(mSettings.showtime);
116+
113117
return result;
114118
}

‎cli/threadexecutor.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "errorlogger.h"
2525
#include "importproject.h"
2626
#include "settings.h"
27+
#include "timer.h"
2728

2829
#include <algorithm>
2930
#include <cassert>
@@ -191,7 +192,12 @@ unsigned int ThreadExecutor::check()
191192
}
192193
}
193194

194-
return std::accumulate(threadFutures.begin(), threadFutures.end(), 0U, [](unsigned int v, std::future<unsigned int>& f) {
195+
unsigned int result = std::accumulate(threadFutures.begin(), threadFutures.end(), 0U, [](unsigned int v, std::future<unsigned int>& f) {
195196
return v + f.get();
196197
});
198+
199+
if (mSettings.showtime == SHOWTIME_MODES::SHOWTIME_SUMMARY || mSettings.showtime == SHOWTIME_MODES::SHOWTIME_TOP5_SUMMARY)
200+
CppCheck::printTimerResults(mSettings.showtime);
201+
202+
return result;
197203
}

‎lib/cppcheck.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,6 @@ CppCheck::~CppCheck()
468468
delete mFileInfo.back();
469469
mFileInfo.pop_back();
470470
}
471-
s_timerResults.showResults(mSettings.showtime);
472471

473472
if (mPlistFile.is_open()) {
474473
mPlistFile << ErrorLogger::plistFooter();
@@ -1101,6 +1100,9 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
11011100

11021101
mErrorList.clear();
11031102

1103+
if (mSettings.showtime == SHOWTIME_MODES::SHOWTIME_FILE || mSettings.showtime == SHOWTIME_MODES::SHOWTIME_TOP5_FILE)
1104+
printTimerResults(mSettings.showtime);
1105+
11041106
return mExitCode;
11051107
}
11061108

@@ -1904,3 +1906,14 @@ void CppCheck::removeCtuInfoFiles(const std::map<std::string, std::size_t> &file
19041906
}
19051907
}
19061908
}
1909+
1910+
// cppcheck-suppress unusedFunction - only used in tests
1911+
void CppCheck::resetTimerResults()
1912+
{
1913+
s_timerResults.reset();
1914+
}
1915+
1916+
void CppCheck::printTimerResults(SHOWTIME_MODES mode)
1917+
{
1918+
s_timerResults.showResults(mode);
1919+
}

0 commit comments

Comments
 (0)