Conversation
|
Thanks for your contribution. Please refer to ticket 5738 in the PR title.
Just opening a PR should be fine. |
|
Sure, I have added it now, also thanks for the reply. |
| const bool stringPrefix1 = !tok1->isKeyword() && Token::Match(tok1, "%name% %str%"); | ||
| const bool stringPrefix2 = !tok2->isKeyword() && Token::Match(tok2, "%name% %str%"); |
There was a problem hiding this comment.
This is an AI review. Take it with a grain of salt and feel free to reject it by resolving the comment.
Looks good to me. I built the PR and compared it with the merge base. The false positives are gone for n == SRCDIR "/a" || n == SRCDIR "/b", n != SRCDIR "/a" && n != SRCDIR "/b" and b ? PFX "x" : PFX "y" (duplicateExpression / duplicateExpressionTernary). Real duplicates (b ? PFX "x" : PFX "x") are still reported.
Minor nit: isSameExpression() is a hot function that recurses over whole ASTs, and this adds two Token::Match(..., "%name% %str%") calls to every invocation. A cheap pre-check would avoid the pattern matching in the common case, e.g. tok1->isName() && tok1->next() && tok1->next()->tokType() == Token::eString (and similar for tok2), or just Token::Match only after a tok->next()->isLiteral() check.
This fixes the false
duplicateExpressionwarning reported in Trac #5738.I reproduced it with expressions like this:
The
startswith(...)case from this ticket was fixed in July 2018. The direct comparisons above still trigger the warning on current main. This PR addresses those remaining comparisons.When
SRCDIRis undefined, Cppcheck compares the macro names and misses the different strings after them. The patch prevents that incomplete comparison from producing a duplicate warning.I added regression tests for the reported case and checked that real duplicates still produce warnings, including identical
throwexpressions. The three focused tests fail before the fix and pass after it. The full C++ suite passes with 5,323 tests and 355 existing TODOs. These checks ran on Linux/WSL at commitc976cac0ec531e1ff47b8cf78028f1182e84a35e.I found this issue through the bounty page. Is the program still running, and would this fix qualify for the listed $20? I'm based in Türkiye and can also receive crypto. Which payment methods do you support, and roughly how long does payment usually take?
I also have local fixes for Trac #6552 and Trac #4270. How should I request assignment for those?