Fix #12214 FN constParameterReference / #12216 FP constParameterReference (#5691)

Author: chrchr-github

lib/astutils.cpp

@@ -2290,6 +2290,8 @@ static T* getTokenArgumentFunctionImpl(T* tok, int& argn)
         tok = tok->astOperand1();
     while (tok && (tok->isUnaryOp("*") || tok->str() == "["))
         tok = tok->astOperand1();
+    if (Token::Match(tok, ". * %name%")) // bailout for pointer to member
+        return tok->tokAt(2);
     while (Token::simpleMatch(tok, "."))
         tok = tok->astOperand2();
     while (Token::simpleMatch(tok, "::")) {
@@ -2630,6 +2632,8 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
         if (!ftok->function() || !ftok->function()->isConst())
             return true;
     }
+    if (Token::Match(tok2->astParent(), ". * %name%")) // bailout
+        return true;
 
     if (Token::simpleMatch(tok2, "[") && astIsContainer(tok) && vt && vt->container && vt->container->stdAssociativeLike)
         return true;

lib/checkother.cpp

@@ -1390,6 +1390,8 @@ void CheckOther::checkConstVariable()
             continue;
         if (var->isVolatile())
             continue;
+        if (var->nameToken()->isExpandedMacro())
+            continue;
         if (isStructuredBindingVariable(var)) // TODO: check all bound variables
             continue;
         if (isVariableChanged(var, mSettings, mTokenizer->isCPP()))
@@ -1461,25 +1463,6 @@ void CheckOther::checkConstVariable()
             if (usedInAssignment)
                 continue;
         }
-        // Skip if we ever cast this variable to a pointer/reference to a non-const type
-        {
-            bool castToNonConst = false;
-            for (const Token* tok = var->nameToken(); tok != scope->bodyEnd && tok != nullptr; tok = tok->next()) {
-                if (tok->isCast()) {
-                    if (!tok->valueType()) {
-                        castToNonConst = true; // safe guess
-                        break;
-                    }
-                    const bool isConst = tok->valueType()->isConst(tok->valueType()->pointer);
-                    if (!isConst) {
-                        castToNonConst = true;
-                        break;
-                    }
-                }
-            }
-            if (castToNonConst)
-                continue;
-        }
 
         constVariableError(var, hasFunction ? function : nullptr);
     }

lib/tokenize.cpp

@@ -619,7 +619,7 @@ namespace {
         bool mUsed = false;
 
     public:
-        TypedefSimplifier(Token* typedefToken, int &num) : mTypedefToken(typedefToken) {
+        explicit TypedefSimplifier(Token* typedefToken) : mTypedefToken(typedefToken) {
             Token* start = typedefToken->next();
             if (Token::simpleMatch(start, "typename"))
                 start = start->next();
@@ -641,7 +641,6 @@ namespace {
                     mRangeTypeQualifiers = rangeQualifiers;
                     Token* typeName = rangeBefore.second->previous();
                     if (typeName->isKeyword()) {
-                        (void)num;
                         // TODO typeName->insertToken("T:" + std::to_string(num++));
                         typeName->insertToken(nameToken->str());
                     }
@@ -1048,16 +1047,14 @@ void Tokenizer::simplifyTypedef()
     std::map<std::string, int> numberOfTypedefs;
     for (Token* tok = list.front(); tok; tok = tok->next()) {
         if (tok->str() == "typedef") {
-            int dummy = 0;
-            TypedefSimplifier ts(tok, dummy);
+            TypedefSimplifier ts(tok);
             if (!ts.fail())
                 numberOfTypedefs[ts.name()]++;
             continue;
         }
     }
 
     int indentlevel = 0;
-    int typeNum = 1;
     std::map<std::string, TypedefSimplifier> typedefs;
     for (Token* tok = list.front(); tok; tok = tok->next()) {
         if (!tok->isName()) {
@@ -1069,7 +1066,7 @@ void Tokenizer::simplifyTypedef()
         }
 
         if (indentlevel == 0 && tok->str() == "typedef") {
-            TypedefSimplifier ts(tok, typeNum);
+            TypedefSimplifier ts(tok);
             if (!ts.fail() && numberOfTypedefs[ts.name()] == 1) {
                 if (mSettings->severity.isEnabled(Severity::portability) && ts.isInvalidConstFunctionType(typedefs))
                     reportError(tok->next(), Severity::portability, "invalidConstFunctionType",

lib/valueflow.cpp

@@ -5238,7 +5238,7 @@ static const Scope* getLoopScope(const Token* tok)
 }
 
 //
-static void valueFlowConditionExpressions(TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings &settings)
+static void valueFlowConditionExpressions(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings &settings)
 {
     for (const Scope * scope : symboldatabase.functionScopes) {
         if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) {
@@ -7482,7 +7482,7 @@ static void valueFlowInjectParameter(const TokenList& tokenlist,
                      settings);
 }
 
-static void valueFlowSwitchVariable(TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
+static void valueFlowSwitchVariable(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
 {
     for (const Scope &scope : symboldatabase.scopeList) {
         if (scope.type != Scope::ScopeType::eSwitch)

test/cfg/std.cpp

@@ -4206,7 +4206,7 @@ void uninivar_istream_read(std::istream &f)
     f.read(buffer, size);
 }
 
-void uninitvar_string_compare(std::string &teststr, std::wstring &testwstr)
+void uninitvar_string_compare(const std::string &teststr, const std::wstring &testwstr)
 {
     const char *pStrUninit;
     // cppcheck-suppress uninitvar

test/cfg/wxwidgets.cpp

@@ -29,7 +29,7 @@
 #include <wx/textctrl.h>
 #include <wx/propgrid/property.h>
 
-void uninitvar_wxRegEx_GetMatch(wxRegEx &obj, size_t *start, size_t *len, size_t index)
+void uninitvar_wxRegEx_GetMatch(const wxRegEx &obj, size_t *start, size_t *len, size_t index)
 {
     size_t s,l;
     size_t *sPtr,*lPtr;

test/testother.cpp

@@ -2873,7 +2873,7 @@ class TestOther : public TestFixture {
               "   x.f();\n"
               "   foo( static_cast<U2>(0) );\n"
               "}");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
 
         check("class a {\n"
               "    void foo(const int& i) const;\n"
@@ -3375,6 +3375,18 @@ class TestOther : public TestFixture {
                       "[test.cpp:1]: (style) Parameter 's2' can be declared as reference to const\n",
                       errout.str());
 
+        check("void f(int& r) {\n" // #12214
+              "    (void)(true);\n"
+              "    if (r) {}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'r' can be declared as reference to const\n", errout.str());
+
+        check("struct S { void f(int&); };\n" // #12216
+              "void g(S& s, int& r, void (S::* p2m)(int&)) {\n"
+              "    (s.*p2m)(r);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
         check("struct S {\n"
               "    void f(int& r) { p = &r; }\n"
               "    int* p;\n"
@@ -10861,7 +10873,9 @@ class TestOther : public TestFixture {
               "        for (auto &j : g(std::move(l))) { (void)j; }\n"
               "    }\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:4]: (warning) Access of moved variable 'l'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (style) Variable 'j' can be declared as reference to const\n"
+                      "[test.cpp:4]: (warning) Access of moved variable 'l'.\n",
+                      errout.str());
     }
 
     void moveCallback()