Skip to content

refs #10736 - cli/main.cpp: removed conditional (NDEBUG) exception handling #8122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
firewave wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refs #10736 - cli/main.cpp: removed conditional (NDEBUG) exception handling #8122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ cli/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.h lib/checkers.h l
cli/filelister.o: cli/filelister.cpp cli/filelister.h lib/config.h lib/filesettings.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/standards.h lib/utils.h
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/filelister.cpp

cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/errortypes.h lib/filesettings.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h
cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/filesettings.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/main.cpp

cli/processexecutor.o: cli/processexecutor.cpp cli/executor.h cli/processexecutor.h lib/addoninfo.h lib/check.h lib/checkers.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
Expand Down
27 changes: 1 addition & 26 deletions cli/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,8 @@
* When errors are found, they are reported back to the CppCheckExecutor through the ErrorLogger interface.
*/


#include "cppcheckexecutor.h"

#ifdef NDEBUG
#include "errortypes.h"

#include <cstdlib>
#include <exception>
#include <iostream>
#endif

/**
* Main function of cppcheck
*
Expand All @@ -82,21 +73,5 @@ int main(int argc, char* argv[])
#endif

CppCheckExecutor exec;

// *INDENT-OFF*
#ifdef NDEBUG
try {
#endif
return exec.check(argc, argv);
#ifdef NDEBUG
} catch (const InternalError& e) {
std::cout << e.errorMessage << std::endl;
} catch (const std::exception& error) {
std::cout << error.what() << std::endl;
} catch (...) {
std::cout << "Unknown exception" << std::endl;
}
return EXIT_FAILURE;
#endif
// *INDENT-ON*
return exec.check(argc, argv);
Copy link
Owner

@danmar danmar Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the ifdef is questionable. but can there be uncaught exceptions from exec.check now? I fear it is UB if we don't catch it. Having a try catch here seems like a good idea unless we are extremely sure there can't be exceptions.

Copy link
Collaborator Author

@firewave firewave Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't catch it, it will lead to an abnormal process termination. If we annotate the check() function with noexcept (which I considered) that would have the same result. And if we catch it that would not provide much more value since the message would not be available in a head-less scenario (i.e. CI) and to have a parsable result it needs to be caught beforehand and we have catch handlers in place there.

Also potential cases of exceptions slipping through will be detected by static analysis - like danmar/simplecpp#616 was actually reported by Coverity. And upcoming improvements in the bugprone-exception-escape clang-tidy check will also help with this. More usage of noexcept after llvm/llvm-project#168324 has been landed will make things even better. More such annotation will also help detection on our side and the compiler ifself.

}
Loading