Fix #12740 FP redundantCopyLocalConst with pointer member (#6410)

danmar · May 21, 2024 · 7419e53 · 7419e53
1 parent 9fb2625
commit 7419e53
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 1 deletion.
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -2901,7 +2901,9 @@ void CheckOther::checkRedundantCopy()
 
         const Token* dot = tok->astOperand1();
         if (Token::simpleMatch(dot, ".")) {
-            if (dot->astOperand1() && isVariableChanged(dot->astOperand1()->variable(), *mSettings))
+            const Token* varTok = dot->astOperand1();
+            const int indirect = varTok->valueType() ? varTok->valueType()->pointer : 0;
+            if (isVariableChanged(tok, tok->scope()->bodyEnd, varTok->varId(), indirect, /*globalvar*/ false, *mSettings))
                 continue;
             if (isTemporary(dot, &mSettings->library, /*unknown*/ true))
                 continue;

diff --git a/test/testother.cpp b/test/testother.cpp
@@ -8975,6 +8975,45 @@ class TestOther : public TestFixture {
               "    if (a.x > a.y) {}\n"
               "}\n");
         ASSERT_EQUALS("", errout_str());
+
+        check("struct S {\n" // #12740
+              "    const std::string & get() const { return m; }\n"
+              "    void set(const std::string& v) { m = v; }\n"
+              "    std::string m;\n"
+              "};\n"
+              "struct T {\n"
+              "    void f();\n"
+              "    S* s;\n"
+              "};\n"
+              "void T::f() {\n"
+              "    const std::string o = s->get();\n"
+              "    s->set(\"abc\");\n"
+              "    s->set(o);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check("struct S {\n" // #12196
+              "    std::string s;\n"
+              "    const std::string& get() const { return s; }\n"
+              "};\n"
+              "struct T {\n"
+              "    S* m;\n"
+              "    void f();\n"
+              "};\n"
+              "struct U {\n"
+              "    explicit U(S* p);\n"
+              "    void g();\n"
+              "    S* n;\n"
+              "};\n"
+              "void T::f() {\n"
+              "    U u(m);\n"
+              "    const std::string c = m->get();\n"
+              "    u.g();\n"
+              "    if (c == m->get()) {}\n"
+              "}\n");
+        TODO_ASSERT_EQUALS("",
+                           "[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());
     }
 
     void checkNegativeShift() {