From c8bc6f5f0ef5c4aaed61045a1749fb2ab0b0651d Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Thu, 18 Jan 2024 10:22:41 +0100 Subject: [PATCH] Fix #12349, #12350, #12351 FN constParameterReference when taking const reference (#5882) --- lib/astutils.cpp | 5 +++++ lib/checkother.cpp | 2 +- lib/symboldatabase.cpp | 2 +- test/testnullpointer.cpp | 2 +- test/testother.cpp | 41 ++++++++++++++++++++++++++++++++++++++++ test/teststl.cpp | 2 +- test/testuninitvar.cpp | 2 +- 7 files changed, 51 insertions(+), 5 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 58f421bf1d4..5b66f61ffc6 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2474,6 +2474,11 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti return true; } + if (const Variable* var = tok->variable()) { + if (tok == var->nameToken() && (!var->isReference() || var->isConst()) && (var->isStlType() || !var->isClass())) // const ref or passed to (copy) ctor + return false; + } + std::vector args = getArgumentVars(tok, argnr); bool conclusive = false; for (const Variable *arg:args) { diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 8e23a0617d9..4c88092638b 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1547,7 +1547,7 @@ void CheckOther::checkConstPointer() else if (Token::Match(gparent, "%assign%") && parent == gparent->astOperand2()) { bool takingRef = false, nonConstPtrAssignment = false; const Token *lhs = gparent->astOperand1(); - if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs->variable()->nameToken() == lhs) + if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs->variable()->nameToken() == lhs && !lhs->variable()->isConst()) takingRef = true; if (lhs && lhs->valueType() && lhs->valueType()->pointer && (lhs->valueType()->constness & 1) == 0 && parent->valueType() && parent->valueType()->pointer) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 277c2848133..bffafe3eaa3 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -2381,7 +2381,7 @@ void Variable::evaluate(const Settings* settings) std::string strtype = mTypeStartToken->str(); for (const Token *typeToken = mTypeStartToken; Token::Match(typeToken, "%type% :: %type%"); typeToken = typeToken->tokAt(2)) strtype += "::" + typeToken->strAt(2); - setFlag(fIsClass, !lib->podtype(strtype) && !mTypeStartToken->isStandardType() && !isEnumType() && !isPointer() && !isReference() && strtype != "..."); + setFlag(fIsClass, !lib->podtype(strtype) && !mTypeStartToken->isStandardType() && !isEnumType() && !isPointer() && strtype != "..."); setFlag(fIsStlType, Token::simpleMatch(mTypeStartToken, "std ::")); setFlag(fIsStlString, ::isStlStringType(mTypeStartToken)); setFlag(fIsSmartPointer, mTypeStartToken->isCpp() && lib->isSmartPointer(mTypeStartToken)); diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index d67cd8329d8..5f76195af10 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -3717,7 +3717,7 @@ class TestNullPointer : public TestFixture { ASSERT_EQUALS("[test.cpp:9]: (error) Null pointer dereference: p\n" "[test.cpp:10]: (error) Null pointer dereference: p\n" "[test.cpp:11]: (error) Null pointer dereference: p\n" - "[test.cpp:12]: (warning, inconclusive) Possible null pointer dereference: p\n" + "[test.cpp:12]: (error) Null pointer dereference: p\n" "[test.cpp:3]: (error) Null pointer dereference\n" "[test.cpp:5]: (error) Null pointer dereference\n" "[test.cpp:7]: (error) Null pointer dereference\n" diff --git a/test/testother.cpp b/test/testother.cpp index 6bce0a87d2f..4f12bf4f622 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -3479,6 +3479,47 @@ class TestOther : public TestFixture { ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared as reference to const\n" "[test.cpp:8]: (style) Parameter 'v' can be declared as reference to const\n", errout.str()); + + check("void cb(const std::string&);\n" // #12349, #12350, #12351 + "void f(std::string& s) {\n" + " const std::string& str(s);\n" + " cb(str);\n" + "}\n" + "void g(std::string& s) {\n" + " const std::string& str{ s };\n" + " cb(str);\n" + "}\n" + "void h(std::string* s) {\n" + " const std::string& str(*s);\n" + " cb(str);\n" + "}\n" + "void k(std::string* s) {\n" + " const std::string& str = *s;\n" + " cb(str);\n" + "}\n" + "void m(std::string& s) {\n" + " const std::string str(s);\n" + " cb(str);\n" + "}\n" + "void n(std::string* s) {\n" + " const std::string& str(*s);\n" + " cb(str);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 's' can be declared as reference to const\n" + "[test.cpp:6]: (style) Parameter 's' can be declared as reference to const\n" + "[test.cpp:18]: (style) Parameter 's' can be declared as reference to const\n" + "[test.cpp:10]: (style) Parameter 's' can be declared as pointer to const\n" + "[test.cpp:14]: (style) Parameter 's' can be declared as pointer to const\n" + "[test.cpp:22]: (style) Parameter 's' can be declared as pointer to const\n", + errout.str()); + + check("struct S {\n" + " S(std::string& r);\n" + "};\n" + "void f(std::string& str) {\n" + " const S& s(str);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void constParameterCallback() { diff --git a/test/teststl.cpp b/test/teststl.cpp index fb87b1c0a3b..05d0b8f070d 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -925,7 +925,7 @@ class TestStl : public TestFixture { " std::string_view sv(s);\n" " return s[2];\n" "}\n"); - TODO_ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in expression 's[2]' because 's' is empty.\n", "", errout.str()); + ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in expression 's[2]' because 's' is empty.\n", errout.str()); } void outOfBoundsSymbolic() diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index a4c7a78c5a6..57209f0f9bf 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -5562,7 +5562,7 @@ class TestUninitVar : public TestFixture { " p = new S(io);\n" " p->Write();\n" "}"); - ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:10]: (warning) Uninitialized variable: p.rIo\n", errout.str()); + ASSERT_EQUALS("", errout.str()); // Unknown types {