Track lifetimes of lambdas that capture the 'this' variable (#3594)

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() == "::")

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);
+            for (const LifetimeToken& lt : getLifetimeTokens(argtok)) {
-            if (var && var->isArgument()) {
+                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 = lt.token;
-                value.tokvalue = var->nameToken();
                 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,6 +3818,9 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
                     ls.forward = false;
                     ls.errorPath = v.errorPath;
                     ls.errorPath.emplace_front(returnTok, "Return " + lifetimeType(returnTok, &v) + ".");
+                    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;
                 }
@@ -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<const Scope *> scopes;
             // Avoid capturing a variable twice
             std::set<nonneg int> varids;
+            bool capturedThis = false;
 
             auto isImplicitCapturingVariable = [&](const Token *varTok) {
                 const Variable *var = varTok->variable();
@@ -4063,24 +4083,66 @@ 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;
+                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())
+                if (isImplicitCapturingThis(tok2)) {
-                    continue;
+                    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

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);

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() {

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<int>& 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<int>& f();\n"
               "std::vector<int>& g();\n"