From a6f4fbae54cb107ff28b7ca676a7c4b97af55613 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Fri, 22 Jan 2021 04:00:57 -0600 Subject: [PATCH] Fix issue 2741: False negative: redundant assignment of x to itself (ref = x) (#3071) --- lib/astutils.cpp | 77 ++++++++++++++++++++++++++++++++++++++++++++++ test/testother.cpp | 6 ++++ 2 files changed, 83 insertions(+) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 1d5df0f78..f9720bec5 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -21,6 +21,7 @@ #include "astutils.h" #include "config.h" +#include "errortypes.h" #include "library.h" #include "mathlib.h" #include "settings.h" @@ -732,6 +733,75 @@ static void followVariableExpressionError(const Token *tok1, const Token *tok2, errors->push_back(item); } +const Token* followReferences(const Token* tok, ErrorPath* errors, int depth = 20) +{ + if (!tok) + return nullptr; + if (depth < 0) + return tok; + const Variable *var = tok->variable(); + if (var && var->declarationId() == tok->varId()) { + if (var->nameToken() == tok) { + return tok; + } else if (var->isReference() || var->isRValueReference()) { + if (!var->declEndToken()) + return tok; + if (var->isArgument()) { + if (errors) + errors->emplace_back(var->declEndToken(), "Passed to reference."); + return tok; + } else if (Token::simpleMatch(var->declEndToken(), "=")) { + if (errors) + errors->emplace_back(var->declEndToken(), "Assigned to reference."); + const Token *vartok = var->declEndToken()->astOperand2(); + if (vartok == tok) + return tok; + if (vartok) + return followReferences(vartok, errors, depth - 1); + } else { + return tok; + } + } + } else if (Token::Match(tok->previous(), "%name% (")) { + const Function *f = tok->previous()->function(); + if (f) { + if (!Function::returnsReference(f)) + return tok; + std::set result; + ErrorPath errorPath; + for (const Token* returnTok : Function::findReturns(f)) { + if (returnTok == tok) + continue; + const Token* argvarTok = followReferences(returnTok, errors, depth - 1); + const Variable* argvar = argvarTok->variable(); + if (!argvar) + return tok; + if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) { + int n = getArgumentPos(argvar, f); + if (n < 0) + return tok; + std::vector args = getArguments(tok->previous()); + if (n >= args.size()) + return tok; + const Token* argTok = args[n]; + ErrorPath er; + er.emplace_back(returnTok, "Return reference."); + er.emplace_back(tok->previous(), "Called function passing '" + argTok->expressionString() + "'."); + const Token* tok2 = followReferences(argTok, &er, depth - 1); + errorPath = er; + result.insert(tok2); + } + } + if (result.size() == 1) { + if (errors) + errors->insert(errors->end(), errorPath.begin(), errorPath.end()); + return *result.begin(); + } + } + } + return tok; +} + static bool isSameLifetime(const Token * const tok1, const Token * const tok2) { ValueFlow::Value v1 = getLifetimeObjValue(tok1); @@ -851,6 +921,13 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 return isSameExpression(cpp, macro, varTok1, varTok2, library, true, followVar, errors); } } + // Follow references + if (tok1->str() != tok2->str()) { + const Token* refTok1 = followReferences(tok1, errors); + const Token* refTok2 = followReferences(tok2, errors); + if (refTok1 != tok1 || refTok2 != tok2) + return isSameExpression(cpp, macro, refTok1, refTok2, library, pure, followVar, errors); + } if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str() || tok1->originalName() != tok2->originalName()) { if ((Token::Match(tok1,"<|>") && Token::Match(tok2,"<|>")) || (Token::Match(tok1,"<=|>=") && Token::Match(tok2,"<=|>="))) { diff --git a/test/testother.cpp b/test/testother.cpp index a8a06b5cb..df04b78b6 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -4000,6 +4000,12 @@ private: check("void f(int i) { i = !!i; }"); ASSERT_EQUALS("", errout.str()); + check("void foo() {\n" + " int x = 1;\n" + " int &ref = x;\n" + " ref = x;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Redundant assignment of 'ref' to itself.\n", errout.str()); } void trac1132() {