CppCheck: removed duplicated exception handling from checkInternal()
#8019
Conversation
|
Best reviewed with whitespaces ignored. |
| // Check normal tokens | ||
| checkNormalTokens(tokenizer, analyzerInformation.get(), currentConfig); | ||
| } catch (const InternalError &e) { | ||
| ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, file.spath()); |
There was a problem hiding this comment.
If we have this InternalError then do we want to abort the loop? Maybe current configuration leads to an InternalError but another configuration is successfully checked?
There was a problem hiding this comment.
I do not understand what you mean. Could you elaborate a bit?
I also did not change the behavior. It was a try-catch inside another try-catch with the same catch handlers.
There was a problem hiding this comment.
This try you removed is inside a for loop that starts at line 1088:
for (const std::string &currCfg : configurations) {
....
try {
code
} catch ...
....
}
if the code would throw an exception then that would have been handled in the loop and the loop could continue to process other configurations..
after your changes it seems to me that the exception is handled below the loop and that the loop is aborted as a consequence.
There was a problem hiding this comment.
Ugh. You are right, of course. But it seems we are lacking testing for this. I was curious why none of the test results changed and still would have added a test which triggers it.
The TerminateException is still redundant though.
So I will restore the InternalError and add tests and more documentation. A follow-up will introduce some noexcept (mostly driven by upcoming clang-tidy improvements).
firewave
force-pushed
the
preproc-out-2
branch
from
70e0ae5 to
7920029
Compare
|
2 participants
No description provided.