Skip to content

Fix #14660 Handle reverse conditions in isOppositeCond function#8408

Merged
danmar merged 3 commits intocppcheck-opensource:mainfrom
francois-berder:reverse-cond
Apr 12, 2026
Merged

Fix #14660 Handle reverse conditions in isOppositeCond function#8408
danmar merged 3 commits intocppcheck-opensource:mainfrom
francois-berder:reverse-cond

Conversation

@francois-berder
Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Francois Berder <fberder@outlook.fr>
Comment thread lib/astutils.cpp Fixed
Comment thread lib/astutils.cpp Outdated
Comment thread lib/astutils.cpp Outdated
@firewave
Copy link
Copy Markdown
Collaborator

firewave commented Apr 9, 2026

Please file a ticket for this.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Apr 9, 2026

Please file a ticket for this.

not sure if you have a trac account. let us know if you want us to file a ticket for you..

@francois-berder
Copy link
Copy Markdown
Collaborator Author

Please file a ticket for this.

not sure if you have a trac account. let us know if you want us to file a ticket for you..

Indeed, I do not have a trac account and I cannot create one myself so please file a ticket for me.

@chrchr-github chrchr-github changed the title Handle reverse conditions in isOppositeCond function Fix #14660 Handle reverse conditions in isOppositeCond function Apr 9, 2026
@chrchr-github
Copy link
Copy Markdown
Collaborator

Indeed, I do not have a trac account and I cannot create one myself so please file a ticket for me.

See https://trac.cppcheck.net/ticket/14660

@francois-berder
Copy link
Copy Markdown
Collaborator Author

Let me know if you want to squash the commits and include a reference to the ticket number in the commit message.

@sonarqubecloud
Copy link
Copy Markdown

@chrchr-github
Copy link
Copy Markdown
Collaborator

Let me know if you want to squash the commits and include a reference to the ticket number in the commit message.

We always squash, and the PR title becomes the commit message, so it's all good.

Comment thread test/testcondition.cpp
ASSERT_EQUALS("", errout_str());

check("void f1(const std::string &s) { if(42 < s.size()) if(s.empty()) {}}");
ASSERT_EQUALS("[test.cpp:1:39] -> [test.cpp:1:61]: (warning) Opposite inner 'if' condition leads to a dead code block. [oppositeInnerCondition]\n", errout_str());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm I feel it's out of scope for this PR but I feel the warning message is a bit obfuscated. The "leads to a dead code block" is true but it's an indirect consequence. I think it would be better to try to describe the direct consequence that inner condition s.empty() is always false.
I also feel that sometimes opposite inner conditions doesn't necessarily lead to dead code.. i.e.: if (x < 0 || s.empty()) I don't know if we warn but imho it should be OK to warn if the message is clear..

@danmar danmar merged commit 830ef47 into cppcheck-opensource:main Apr 12, 2026
74 of 75 checks passed
@francois-berder francois-berder deleted the reverse-cond branch April 20, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants