From be956baa98f347dbf86fc59df954ebf3b260a966 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 23 Apr 2025 20:09:00 +0200 Subject: [PATCH 1/3] Fix #13794 FN constStatement for missing function calls --- lib/checkother.cpp | 37 ++++++++++++++++++++------------ test/testincompletestatement.cpp | 18 +++++++++++++++- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 3ea60f39d21..f986cca5872 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2022,7 +2022,7 @@ static bool isConstant(const Token* tok) { return tok && (tok->isEnumerator() || Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL")); } -static bool isConstStatement(const Token *tok, bool isNestedBracket = false) +static bool isConstStatement(const Token *tok, const Library& library, bool isNestedBracket = false) { if (!tok) return false; @@ -2044,7 +2044,7 @@ static bool isConstStatement(const Token *tok, bool isNestedBracket = false) tok2 = tok2->astParent(); } if (Token::Match(tok, "&&|%oror%")) - return isConstStatement(tok->astOperand1()) && isConstStatement(tok->astOperand2()); + return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2(), library); if (Token::Match(tok, "!|~|%cop%") && (tok->astOperand1() || tok->astOperand2())) return true; if (Token::simpleMatch(tok->previous(), "sizeof (")) @@ -2052,15 +2052,15 @@ static bool isConstStatement(const Token *tok, bool isNestedBracket = false) if (isCPPCast(tok)) { if (Token::simpleMatch(tok->astOperand1(), "dynamic_cast") && Token::simpleMatch(tok->astOperand1()->linkAt(1)->previous(), "& >")) return false; - return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2()); + return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2(), library); } if (tok->isCast() && tok->next() && tok->next()->isStandardType()) - return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1()); + return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1(), library); if (Token::simpleMatch(tok, ".")) - return isConstStatement(tok->astOperand2()); + return isConstStatement(tok->astOperand2(), library); if (Token::simpleMatch(tok, ",")) { if (tok->astParent()) // warn about const statement on rhs at the top level - return isConstStatement(tok->astOperand1()) && isConstStatement(tok->astOperand2()); + return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2(), library); const Token* lml = previousBeforeAstLeftmostLeaf(tok); // don't warn about matrix/vector assignment (e.g. Eigen) if (lml) @@ -2068,21 +2068,28 @@ static bool isConstStatement(const Token *tok, bool isNestedBracket = false) const Token* stream = lml; while (stream && Token::Match(stream->astParent(), ".|[|(|*")) stream = stream->astParent(); - return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2()); + return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2(), library); } if (Token::simpleMatch(tok, "?") && Token::simpleMatch(tok->astOperand2(), ":")) // ternary operator - return isConstStatement(tok->astOperand1()) && isConstStatement(tok->astOperand2()->astOperand1()) && isConstStatement(tok->astOperand2()->astOperand2()); + return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2()->astOperand1(), library) && isConstStatement(tok->astOperand2()->astOperand2(), library); if (isBracketAccess(tok) && isWithoutSideEffects(tok->astOperand1(), /*checkArrayAccess*/ true, /*checkReference*/ false)) { const bool isChained = succeeds(tok->astParent(), tok); if (Token::simpleMatch(tok->astParent(), "[")) { if (isChained) - return isConstStatement(tok->astOperand2()) && isConstStatement(tok->astParent()); - return isNestedBracket && isConstStatement(tok->astOperand2()); + return isConstStatement(tok->astOperand2(), library) && isConstStatement(tok->astParent(), library); + return isNestedBracket && isConstStatement(tok->astOperand2(), library); } - return isConstStatement(tok->astOperand2(), /*isNestedBracket*/ !isChained); + return isConstStatement(tok->astOperand2(), library, /*isNestedBracket*/ !isChained); } if (!tok->astParent() && findLambdaEndToken(tok)) return true; + tok2 = tok; + if (tok2->str() == "::") + tok2 = tok2->next(); + while (Token::Match(tok2, "%name% ::")) + tok2 = tok2->tokAt(2); + if (Token::Match(tok2, "%name% ;") && (tok2->function() || library.functions().count(tok2->str()) > 0)) + return true; return false; } @@ -2174,7 +2181,7 @@ void CheckOther::checkIncompleteStatement() // Skip statement expressions if (Token::simpleMatch(rtok, "; } )")) continue; - if (!isConstStatement(tok)) + if (!isConstStatement(tok, mSettings->library)) continue; if (isVoidStmt(tok)) continue; @@ -2228,6 +2235,8 @@ void CheckOther::constStatementError(const Token *tok, const std::string &type, msg = "Redundant code: Found unused array access."; else if (tok->str() == "[" && !tok->astParent()) msg = "Redundant code: Found unused lambda."; + else if (Token::Match(tok, "%name%|::")) + msg = "Redundant code: Found unused function."; else if (mSettings->debugwarnings) { reportError(tok, Severity::debug, "debug", "constStatementError not handled."); return; @@ -2365,7 +2374,7 @@ void CheckOther::checkMisusedScopedObject() if (Token::simpleMatch(parTok, "<") && parTok->link()) parTok = parTok->link()->next(); if (const Token* arg = parTok->astOperand2()) { - if (!isConstStatement(arg)) + if (!isConstStatement(arg, mSettings->library)) continue; if (parTok->str() == "(") { if (arg->varId() && !(arg->variable() && arg->variable()->nameToken() != arg)) @@ -2826,7 +2835,7 @@ void CheckOther::checkDuplicateExpression() } else if (tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") { if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()) && !isVariableChanged(tok->astParent(), /*indirect*/ 0, *mSettings) && - isConstStatement(tok->astOperand1()) && isConstStatement(tok->astOperand2())) + isConstStatement(tok->astOperand1(), mSettings->library) && isConstStatement(tok->astOperand2(), mSettings->library)) duplicateValueTernaryError(tok); else if (isSameExpression(true, tok->astOperand1(), tok->astOperand2(), *mSettings, false, true, &errorPath)) duplicateExpressionTernaryError(tok, std::move(errorPath)); diff --git a/test/testincompletestatement.cpp b/test/testincompletestatement.cpp index bc7fb32022b..371153f8e0d 100644 --- a/test/testincompletestatement.cpp +++ b/test/testincompletestatement.cpp @@ -30,7 +30,7 @@ class TestIncompleteStatement : public TestFixture { TestIncompleteStatement() : TestFixture("TestIncompleteStatement") {} private: - const Settings settings = settingsBuilder().severity(Severity::warning).build(); + const Settings settings = settingsBuilder().severity(Severity::warning).library("std.cfg").build(); struct CheckOptions { @@ -84,6 +84,7 @@ class TestIncompleteStatement : public TestFixture { TEST_CASE(archive); // ar & x TEST_CASE(ast); TEST_CASE(oror); // dostuff() || x=32; + TEST_CASE(functioncall); } void test1() { @@ -792,6 +793,21 @@ class TestIncompleteStatement : public TestFixture { "}\n", dinit(CheckOptions, $.inconclusive = true)); ASSERT_EQUALS("", errout_str()); } + + void functioncall() { + check("void g();\n" // #13794 + "void f() {\n" + " g;\n" + " exit;\n" + " std::free;\n" + " ::std::free;\n" + "}\n", dinit(CheckOptions, $.inconclusive = true)); + ASSERT_EQUALS("[test.cpp:3:5]: (warning) Redundant code: Found unused function. [constStatement]\n" + "[test.cpp:4:5]: (warning) Redundant code: Found unused function. [constStatement]\n" + "[test.cpp:5:8]: (warning) Redundant code: Found unused function. [constStatement]\n" + "[test.cpp:6:10]: (warning) Redundant code: Found unused function. [constStatement]\n", + errout_str()); + } }; REGISTER_TEST(TestIncompleteStatement) From b72b7f03118669e8abefaba6c52ec7a8577cc768 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 23 Apr 2025 20:49:37 +0200 Subject: [PATCH 2/3] nolint --- lib/checkother.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index f986cca5872..c2a8c2e1fb2 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2088,7 +2088,7 @@ static bool isConstStatement(const Token *tok, const Library& library, bool isNe tok2 = tok2->next(); while (Token::Match(tok2, "%name% ::")) tok2 = tok2->tokAt(2); - if (Token::Match(tok2, "%name% ;") && (tok2->function() || library.functions().count(tok2->str()) > 0)) + if (Token::Match(tok2, "%name% ;") && (tok2->function() || library.functions().count(tok2->str()) > 0)) // NOLINT(readability-simplify-boolean-expr) return true; return false; } From 2a5cb0abfeded9b3a506e81261e06336624ed665 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 23 Apr 2025 22:07:03 +0200 Subject: [PATCH 3/3] Fix --- lib/checkother.cpp | 16 ++++++++++++---- test/testincompletestatement.cpp | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index c2a8c2e1fb2..8df2b16d342 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2083,13 +2083,21 @@ static bool isConstStatement(const Token *tok, const Library& library, bool isNe } if (!tok->astParent() && findLambdaEndToken(tok)) return true; + tok2 = tok; if (tok2->str() == "::") tok2 = tok2->next(); - while (Token::Match(tok2, "%name% ::")) - tok2 = tok2->tokAt(2); - if (Token::Match(tok2, "%name% ;") && (tok2->function() || library.functions().count(tok2->str()) > 0)) // NOLINT(readability-simplify-boolean-expr) - return true; + if (Token::Match(tok2, "%name% ;")) { + if (tok2->function()) + return true; + std::string funcStr = tok2->str(); + while (tok2->index() > 1 && Token::Match(tok2->tokAt(-2), "%name% ::")) { + funcStr.insert(0, tok2->strAt(-2) + "::"); + tok2 = tok2->tokAt(-2); + } + if (library.functions().count(funcStr) > 0) + return true; + } return false; } diff --git a/test/testincompletestatement.cpp b/test/testincompletestatement.cpp index 371153f8e0d..f8e44e19133 100644 --- a/test/testincompletestatement.cpp +++ b/test/testincompletestatement.cpp @@ -800,7 +800,7 @@ class TestIncompleteStatement : public TestFixture { " g;\n" " exit;\n" " std::free;\n" - " ::std::free;\n" + " ::std::terminate;\n" "}\n", dinit(CheckOptions, $.inconclusive = true)); ASSERT_EQUALS("[test.cpp:3:5]: (warning) Redundant code: Found unused function. [constStatement]\n" "[test.cpp:4:5]: (warning) Redundant code: Found unused function. [constStatement]\n"