Fix #12925 false negative: redundantCopyLocalConst (#6586)

danmar · Jul 12, 2024 · 818dbee · 818dbee
1 parent 0bd341f
commit 818dbee
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 3 deletions.
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -2920,9 +2920,11 @@ void CheckOther::checkRedundantCopy()
         const Token* startTok = var->nameToken();
         if (startTok->strAt(1) == "=") // %type% %name% = ... ;
             ;
-        else if (Token::Match(startTok->next(), "(|{") && var->isClass() && var->typeScope()) {
+        else if (Token::Match(startTok->next(), "(|{") && var->isClass()) {
+            if (!var->typeScope() && !(var->valueType() && var->valueType()->container))
+                continue;
             // Object is instantiated. Warn if constructor takes arguments by value.
-            if (constructorTakesReference(var->typeScope()))
+            if (var->typeScope() && constructorTakesReference(var->typeScope()))
                 continue;
         } else if (Token::simpleMatch(startTok->next(), ";") && startTok->next()->isSplittedVarDeclEq()) {
             startTok = startTok->tokAt(2);
@@ -2934,7 +2936,7 @@ void CheckOther::checkRedundantCopy()
             continue;
         if (!Token::Match(tok->previous(), "%name% ("))
             continue;
-        if (!Token::Match(tok->link(), ") )| ;")) // bailout for usage like "const A a = getA()+3"
+        if (!Token::Match(tok->link(), ") )|}| ;")) // bailout for usage like "const A a = getA()+3"
             continue;
 
         const Token* dot = tok->astOperand1();

diff --git a/test/testother.cpp b/test/testother.cpp
@@ -9047,6 +9047,22 @@ class TestOther : public TestFixture {
                            "[test.cpp:16] -> [test.cpp:18]: (style) The comparison 'c == m->get()' is always true because 'c' and 'm->get()' represent the same value.\n",
                            errout_str());
 
+        check("struct S {\n" // #12925
+              "    const std::string & f() const { return str; }\n"
+              "    std::string str;\n"
+              "};\n"
+              "void f(const S* s) {\n"
+              "    const std::string v{ s->f() };\n"
+              "    if (v.empty()) {}\n"
+              "}\n"
+              "void g(const S* s) {\n"
+              "    const std::string w(s->f());\n"
+              "    if (w.empty()) {}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:6]: (performance, inconclusive) Use const reference for 'v' to avoid unnecessary data copying.\n"
+                      "[test.cpp:10]: (performance, inconclusive) Use const reference for 'w' to avoid unnecessary data copying.\n",
+                      errout_str());
+
         check("struct T {\n"
               "    std::string s;\n"
               "    const std::string& get() const { return s; }\n"