Revert recent changes in checkConstVariable(), add tests (refs #12203) (

#5696)
danmar · Nov 22, 2023 · 331db40 · 331db40
1 parent 83b5cb5
commit 331db40
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 8 deletions.
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -1429,7 +1429,25 @@ void CheckOther::checkConstVariable()
                     }
                 }
                 if (tok->isUnaryOp("&") && Token::Match(tok, "& %varid%", var->declarationId())) {
-                    usedInAssignment = isExpressionChangedAt(tok->next(), tok, 0, false, mSettings, true);
+                    const Token* opTok = tok->astParent();
+                    int argn = -1;
+                    if (opTok && (opTok->isUnaryOp("!") || opTok->isComparisonOp()))
+                        continue;
+                    if (opTok && (opTok->isAssignmentOp() || opTok->isCalculation())) {
+                        if (opTok->isCalculation()) {
+                            if (opTok->astOperand1() != tok)
+                                opTok = opTok->astOperand1();
+                            else
+                                opTok = opTok->astOperand2();
+                        }
+                        if (opTok && opTok->valueType() && var->valueType() && opTok->valueType()->isConst(var->valueType()->pointer))
+                            continue;
+                    } else if (const Token* ftok = getTokenArgumentFunction(tok, argn)) {
+                        bool inconclusive{};
+                        if (var->valueType() && !isVariableChangedByFunctionCall(ftok, var->valueType()->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive)
+                            continue;
+                    }
+                    usedInAssignment = true;
                     break;
                 }
                 if (astIsRangeBasedForDecl(tok) && Token::Match(tok->astParent()->astOperand2(), "%varid%", var->declarationId())) {

diff --git a/test/testother.cpp b/test/testother.cpp
@@ -2794,31 +2794,34 @@ class TestOther : public TestFixture {
               "void a(T& x) {\n"
               "    x.dostuff();\n"
               "    const U * y = dynamic_cast<const U *>(&x);\n"
+              "    y->mutate();\n" // to avoid warnings that y can be const
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
+        TODO_ASSERT_EQUALS("can be const", errout.str(), ""); //Currently taking the address is treated as a non-const operation when it should depend on what we do with it
         check("struct T : public U { void dostuff() const {}};\n"
               "void a(T& x) {\n"
               "    x.dostuff();\n"
               "    U const * y = dynamic_cast<U const *>(&x);\n"
+              "    y->mutate();\n" // to avoid warnings that y can be const
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
+        TODO_ASSERT_EQUALS("can be const", errout.str(), ""); //Currently taking the address is treated as a non-const operation when it should depend on what we do with it
         check("struct T : public U { void dostuff() const {}};\n"
               "void a(T& x) {\n"
               "    x.dostuff();\n"
-              "    U * const y = dynamic_cast<U * const>(&x);\n"
+              "    const U const * const * const * const y = dynamic_cast<const U const * const * const * const>(&x);\n"
               "    y->mutate();\n" // to avoid warnings that y can be const
               "}");
         ASSERT_EQUALS("", errout.str());
         check("struct T : public U { void dostuff() const {}};\n"
               "void a(T& x) {\n"
               "    x.dostuff();\n"
-              "    U const * const *  * const y = dynamic_cast<U const * const *  * const>(&x);\n"
+              "    const U const * const * const * const y = dynamic_cast<const U const * const * const * const>(&x);\n"
+              "    y->mutate();\n" // to avoid warnings that y can be const
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
+        TODO_ASSERT_EQUALS("can be const", errout.str(), ""); //Currently taking the address is treated as a non-const operation when it should depend on what we do with it
         check("struct T : public U { void dostuff() const {}};\n"
               "void a(T& x) {\n"
               "    x.dostuff();\n"
-              "    my::fancy<typename type const *> const * * const y = dynamic_cast<my::fancy<typename type const *> const * * const>(&x);\n"
+              "    const U const * const *  * const y = dynamic_cast<const U const * const *  * const>(&x);\n"
               "    y->mutate();\n" // to avoid warnings that y can be const
               "}");
         ASSERT_EQUALS("", errout.str());
@@ -3371,6 +3374,15 @@ class TestOther : public TestFixture {
         ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 's1' can be declared as reference to const\n"
                       "[test.cpp:1]: (style) Parameter 's2' can be declared as reference to const\n",
                       errout.str());
+
+        check("struct S {\n"
+              "    void f(int& r) { p = &r; }\n"
+              "    int* p;\n"
+              "};\n"
+              "void g(std::vector<int>& v1, std::vector<int*>& v2) {\n"
+              "    std::transform(v1.begin(), v1.end(), v2.begin(), [](auto& x) { return &x; });\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void constParameterCallback() {
@@ -3760,7 +3772,7 @@ class TestOther : public TestFixture {
         check("void f(int& i) {\n"
               "    new (&i) int();\n"
               "}\n");
-        TODO_ASSERT_EQUALS("", "[test.cpp:1]: (style) Parameter 'i' can be declared as reference to const\n", errout.str()); // don't crash
+        ASSERT_EQUALS("", errout.str()); // don't crash
 
         check("void f(int& i) {\n"
               "    int& r = i;\n"