refs #10736 - cli/main.cpp: removed conditional (NDEBUG) exception handling
#8122
Conversation
|
It was only enabled in Windows build. And did not make much sense since the called function does not throw. This also makes it easier to detect uncaught exceptions in static analysis. |
firewave
changed the title
NDEBUG) exception handlingNDEBUG) exception handling
firewave
changed the title
NDEBUG) exception handlingNDEBUG) exception handling
|
| return EXIT_FAILURE; | ||
| #endif | ||
| // *INDENT-ON* | ||
| return exec.check(argc, argv); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
2 participants
No description provided.