diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index cfcbdc833..859892d18 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -5277,6 +5277,11 @@ const Function* SymbolDatabase::findFunction(const Token *tok) const const Token* tok1 = tok->previous()->astOperand1(); if (tok1 && tok1->valueType() && tok1->valueType()->typeScope) { return tok1->valueType()->typeScope->findFunction(tok, tok1->valueType()->constness == 1); + } else if (tok1 && Token::Match(tok1->previous(), "%name% (") && tok1->previous()->function() && + tok1->previous()->function()->retDef) { + ValueType vt = ValueType::parseDecl(tok1->previous()->function()->retDef, mSettings); + if (vt.typeScope) + return vt.typeScope->findFunction(tok, vt.constness == 1); } else if (Token::Match(tok1, "%var% .")) { const Variable *var = getVariableFromVarId(tok1->varId()); if (var && var->typeScope()) @@ -6554,6 +6559,16 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to // library type/function else if (tok->previous()) { + // Aggregate constructor + if (Token::Match(tok->previous(), "%name%")) { + ValueType valuetype; + if (parsedecl(tok->previous(), &valuetype, mDefaultSignedness, mSettings)) { + if (valuetype.typeScope) { + setValueType(tok, valuetype); + continue; + } + } + } if (tok->astParent() && Token::Match(tok->astOperand1(), "%name%|::")) { const Token *typeStartToken = tok->astOperand1(); while (typeStartToken && typeStartToken->str() == "::") diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index df3632683..50e9ded78 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -3619,18 +3619,27 @@ struct LifetimeStore { if (argtok->values().empty()) { ErrorPath er; er.emplace_back(argtok, message); - const Variable *var = getLifetimeVariable(argtok, er); - if (var && var->isArgument()) { + for (const LifetimeToken& lt : getLifetimeTokens(argtok)) { + if (!settings->certainty.isEnabled(Certainty::inconclusive) && lt.inconclusive) + continue; ValueFlow::Value value; value.valueType = ValueFlow::Value::ValueType::LIFETIME; - value.lifetimeScope = ValueFlow::Value::LifetimeScope::Argument; - value.tokvalue = var->nameToken(); + value.tokvalue = lt.token; value.errorPath = er; value.lifetimeKind = type; - value.setInconclusive(inconclusive); + value.setInconclusive(inconclusive || lt.inconclusive); + const Variable* var = lt.token->variable(); + if (var && var->isArgument()) { + value.lifetimeScope = ValueFlow::Value::LifetimeScope::Argument; + } else if (exprDependsOnThis(lt.token)) { + value.lifetimeScope = ValueFlow::Value::LifetimeScope::ThisPointer; + } else { + continue; + } // Don't add the value a second time if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end()) - return false; + continue; + ; setTokenValue(tok, value, tokenlist->getSettings()); update = true; } @@ -3798,7 +3807,10 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog continue; if (!v.tokvalue) continue; - if (exprDependsOnThis(v.tokvalue) && memtok) { + if (memtok && + (contains({ValueFlow::Value::LifetimeScope::ThisPointer, ValueFlow::Value::LifetimeScope::ThisValue}, + v.lifetimeScope) || + exprDependsOnThis(v.tokvalue))) { LifetimeStore ls = LifetimeStore{memtok, "Passed to member function '" + tok->expressionString() + "'.", ValueFlow::Value::LifetimeKind::Object}; @@ -3806,7 +3818,10 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog ls.forward = false; ls.errorPath = v.errorPath; ls.errorPath.emplace_front(returnTok, "Return " + lifetimeType(returnTok, &v) + "."); - update |= ls.byRef(tok->next(), tokenlist, errorLogger, settings); + if (v.lifetimeScope == ValueFlow::Value::LifetimeScope::ThisValue) + update |= ls.byVal(tok->next(), tokenlist, errorLogger, settings); + else + update |= ls.byRef(tok->next(), tokenlist, errorLogger, settings); continue; } const Variable *var = v.tokvalue->variable(); @@ -3953,7 +3968,11 @@ struct Lambda { bodyTok = afterArguments; } for (const Token* c:getCaptures()) { - if (c->variable()) { + if (Token::Match(c, "this !!.")) { + explicitCaptures[c->variable()] = std::make_pair(c, Capture::ByReference); + } else if (Token::simpleMatch(c, "* this")) { + explicitCaptures[c->next()->variable()] = std::make_pair(c->next(), Capture::ByValue); + } else if (c->variable()) { explicitCaptures[c->variable()] = std::make_pair(c, Capture::ByValue); } else if (c->isUnaryOp("&") && Token::Match(c->astOperand1(), "%var%")) { explicitCaptures[c->astOperand1()->variable()] = std::make_pair(c->astOperand1(), Capture::ByReference); @@ -4023,6 +4042,7 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger std::set scopes; // Avoid capturing a variable twice std::set varids; + bool capturedThis = false; auto isImplicitCapturingVariable = [&](const Token *varTok) { const Variable *var = varTok->variable(); @@ -4063,23 +4083,65 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger } }; + auto captureThisVariable = [&](const Token* tok2, Lambda::Capture c) { + ValueFlow::Value value; + value.valueType = ValueFlow::Value::ValueType::LIFETIME; + if (c == Lambda::Capture::ByReference) + value.lifetimeScope = ValueFlow::Value::LifetimeScope::ThisPointer; + else if (c == Lambda::Capture::ByValue) + value.lifetimeScope = ValueFlow::Value::LifetimeScope::ThisValue; + value.tokvalue = tok2; + value.errorPath.push_back({tok2, "Lambda captures the 'this' variable here."}); + value.lifetimeKind = ValueFlow::Value::LifetimeKind::Lambda; + capturedThis = true; + // Don't add the value a second time + if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end()) + return; + setTokenValue(tok, value, tokenlist->getSettings()); + update |= true; + }; + // Handle explicit capture for (const auto& p:lam.explicitCaptures) { const Variable* var = p.first; - if (!var) - continue; const Token* tok2 = p.second.first; Lambda::Capture c = p.second.second; - captureVariable(tok2, c, [](const Token*) { - return true; - }); - varids.insert(var->declarationId()); + if (Token::Match(tok2, "this !!.")) { + captureThisVariable(tok2, c); + } else if (var) { + captureVariable(tok2, c, [](const Token*) { + return true; + }); + varids.insert(var->declarationId()); + } } + auto isImplicitCapturingThis = [&](const Token* tok2) { + if (capturedThis) + return false; + if (Token::simpleMatch(tok2, "this")) { + return true; + } else if (tok2->variable()) { + if (Token::simpleMatch(tok2->previous(), ".")) + return false; + const Variable* var = tok2->variable(); + if (var->isLocal()) + return false; + if (var->isArgument()) + return false; + return exprDependsOnThis(tok2); + } else if (Token::simpleMatch(tok2, "(")) { + return exprDependsOnThis(tok2); + } + return false; + }; + for (const Token * tok2 = lam.bodyTok; tok2 != lam.bodyTok->link(); tok2 = tok2->next()) { - if (!tok2->variable()) - continue; - captureVariable(tok2, lam.implicitCapture, isImplicitCapturingVariable); + if (isImplicitCapturingThis(tok2)) { + captureThisVariable(tok2, Lambda::Capture::ByReference); + } else if (tok2->variable()) { + captureVariable(tok2, lam.implicitCapture, isImplicitCapturingVariable); + } } if (update) valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); @@ -4188,7 +4250,7 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger valueFlowLifetimeConstructor(tok->next(), tokenlist, errorLogger, settings); } // Check function calls - else if (Token::Match(tok, "%name% (")) { + else if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->next()->link(), ") {")) { valueFlowLifetimeFunction(tok, tokenlist, errorLogger, settings); } // Unique pointer lifetimes diff --git a/lib/valueflow.h b/lib/valueflow.h index 7a9a384de..1904bf5a8 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -376,7 +376,7 @@ namespace ValueFlow { Address } lifetimeKind; - enum class LifetimeScope { Local, Argument, SubFunction } lifetimeScope; + enum class LifetimeScope { Local, Argument, SubFunction, ThisPointer, ThisValue } lifetimeScope; static const char* toString(MoveKind moveKind); static const char* toString(LifetimeKind lifetimeKind); diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 2c627139a..246e3fbdf 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -2954,7 +2954,7 @@ private: " return by_value(v.begin());\n" "}\n"); ASSERT_EQUALS( - "[test.cpp:7] -> [test.cpp:7] -> [test.cpp:3] -> [test.cpp:3] -> [test.cpp:2] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning object that points to local variable 'v' that will be invalid when returning.\n", + "[test.cpp:7] -> [test.cpp:7] -> [test.cpp:3] -> [test.cpp:3] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning object that points to local variable 'v' that will be invalid when returning.\n", errout.str()); check("auto by_ref(int& x) {\n" @@ -3350,6 +3350,44 @@ private: ASSERT_EQUALS( "[test.cpp:9] -> [test.cpp:9] -> [test.cpp:14] -> [test.cpp:13] -> [test.cpp:14]: (error) Returning pointer to local variable 'fred' that will be invalid when returning.\n", errout.str()); + + check("struct A {\n" + " int i;\n" + " auto f() const {\n" + " return [=]{ return i; };\n" + " }\n" + "};\n" + "auto g() {\n" + " return A().f();\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:4] -> [test.cpp:8] -> [test.cpp:8]: (error) Returning object that will be invalid when returning.\n", + errout.str()); + + check("struct A {\n" + " int i;\n" + " auto f() const {\n" + " return [*this]{ return i; };\n" + " }\n" + "};\n" + "auto g() {\n" + " return A().f();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A {\n" + " int* i;\n" + " auto f() const {\n" + " return [*this]{ return i; };\n" + " }\n" + "};\n" + "auto g() {\n" + " int i = 0;\n" + " return A{&i}.f();\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:9] -> [test.cpp:9] -> [test.cpp:9] -> [test.cpp:4] -> [test.cpp:4] -> [test.cpp:8] -> [test.cpp:9]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n", + errout.str()); } void invalidLifetime() { diff --git a/test/teststl.cpp b/test/teststl.cpp index 2609fd599..ccbe4af11 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1588,7 +1588,9 @@ private: "void foo() {\n" " (void)std::find(A().f().begin(), A().g().end(), 0);\n" "}"); - ASSERT_EQUALS("[test.cpp:6]: (warning) Iterators to containers from different expressions 'A().f()' and 'A().g()' are used together.\n", errout.str()); + ASSERT_EQUALS( + "[test.cpp:6]: (error) Iterators of different containers 'A().f()' and 'A().g()' are used together.\n", + errout.str()); check("struct A {\n" " std::vector& f();\n" @@ -1597,7 +1599,9 @@ private: "void foo() {\n" " (void)std::find(A{} .f().begin(), A{} .g().end(), 0);\n" "}"); - ASSERT_EQUALS("[test.cpp:6]: (warning) Iterators to containers from different expressions 'A{}.f()' and 'A{}.g()' are used together.\n", errout.str()); + ASSERT_EQUALS( + "[test.cpp:6]: (error) Iterators of different containers 'A{}.f()' and 'A{}.g()' are used together.\n", + errout.str()); check("std::vector& f();\n" "std::vector& g();\n"