Refactorized check for assigning function parameters:

- Fixed false negative: Check is also valid for all non-references, not only for pointers.
- Fixed false negative: Usage before assignment doesn't require bailout
- Fixed false positive #4598 caused by inadequate usage of CheckUninitVar::isVariableUsage
- Made several member functions static

lib/checkautovariables.cpp

@@ -36,6 +36,13 @@ namespace {
 }
 
 
+bool CheckAutoVariables::isPtrArg(const Token *tok)
+{
+    const Variable *var = tok->variable();
+
+    return(var && var->isArgument() && var->isPointer());
+}
+
 bool CheckAutoVariables::isRefPtrArg(const Token *tok)
 {
     const Variable *var = tok->variable();
@@ -43,11 +50,11 @@ bool CheckAutoVariables::isRefPtrArg(const Token *tok)
     return(var && var->isArgument() && var->isReference() && var->isPointer());
 }
 
-bool CheckAutoVariables::isPtrArg(const Token *tok)
+bool CheckAutoVariables::isNonReferenceArg(const Token *tok)
 {
     const Variable *var = tok->variable();
 
-    return(var && var->isArgument() && var->isPointer());
+    return(var && var->isArgument() && !var->isReference() && (var->isPointer() || var->typeStartToken()->isStandardType() || var->type()));
 }
 
 bool CheckAutoVariables::isAutoVar(const Token *tok)
@@ -80,10 +87,14 @@ static bool checkRvalueExpression(const Variable* var, const Token* next)
     return((next->str() != "." || (!var->isPointer() && (!var->isClass() || var->type()))) && next->strAt(2) != ".");
 }
 
-static bool pointerIsDereferencedInScope(const Variable *var, const Scope *scope, const bool cpp)
+static bool variableIsUsedInScope(const Token* start, unsigned int varId, const Scope *scope)
 {
-    for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
+    for (const Token *tok = start; tok != scope->classEnd; tok = tok->next()) {
-        if (tok->varId() == var->varId() && CheckUninitVar::isVariableUsage(tok, true, cpp))
+        if (tok->varId() == varId)
+            return true;
+        if (tok->scope()->type == Scope::eFor || tok->scope()->type == Scope::eDo || tok->scope()->type == Scope::eWhile) // In case of loops, better checking would be necessary
+            return true;
+        if (Token::simpleMatch(tok, "asm ("))
             return true;
     }
     return false;
@@ -110,9 +121,8 @@ void CheckAutoVariables::autoVariables()
                     errorAutoVariableAssignment(tok->next(), false);
             } else if (reportWarnings &&
                        Token::Match(tok, "[;{}] %var% =") &&
-                       isPtrArg(tok->next()) &&
+                       isNonReferenceArg(tok->next()) &&
-                       Token::Match(tok->next()->variable()->typeStartToken(), "struct| %type% * %var% [,)]") &&
+                       !variableIsUsedInScope(Token::findsimplematch(tok->tokAt(2), ";"), tok->next()->varId(), scope)) {
-                       !pointerIsDereferencedInScope(tok->next()->variable(), scope, _tokenizer->isCPP())) {
                 errorUselessAssignmentPtrArg(tok->next());
             } else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) {
                 // TODO: check if the parameter is only changed temporarily (#2969)

lib/checkautovariables.h

@@ -63,10 +63,11 @@ public:
     void returnReference();
 
 private:
-    bool isRefPtrArg(const Token *tok);
+    static bool isPtrArg(const Token *tok);
-    bool isPtrArg(const Token *tok);
+    static bool isRefPtrArg(const Token *tok);
-    bool isAutoVar(const Token *tok);
+    static bool isNonReferenceArg(const Token *tok);
-    bool isAutoVarArray(const Token *tok);
+    static bool isAutoVar(const Token *tok);
+    static bool isAutoVarArray(const Token *tok);
 
     /**
      * Returning a temporary object?

test/testautovariables.cpp

@@ -112,7 +112,7 @@ private:
               "    int num = 2;\n"
               "    res = #\n"
               "}");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str());
 
         check("void func1(int **res)\n"
               "{\n"
@@ -141,7 +141,7 @@ private:
               "    int num = 2;\n"
               "    res = #\n"
               "}");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:7]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str());
 
         check("class Fred {\n"
               "    void func1(int **res);\n"
@@ -247,6 +247,11 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str());
 
+        check("void foo(int b) {\n"
+              "    b = foo(b);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str());
+
         check("void foo(char* p) {\n"
               "    if (!p) p = buf;\n"
               "    *p = 0;\n"
@@ -258,6 +263,38 @@ private:
               "    do_something(p);\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char* p) {\n"
+              "    while (!p) p = buf;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char* p) {\n"
+              "    p = 0;\n"
+              "    asm(\"somecmd\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(Foo* p) {\n"
+              "    p = 0;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str());
+
+        check("class Foo {};\n"
+              "void foo(Foo p) {\n"
+              "    p = 0;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str());
+
+        check("void foo(Foo p) {\n"
+              "    p = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(int& p) {\n"
+              "    p = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void testautovar_array1() {