Refactor: Use getEndOfExprScope instead of getEndOfVarScope (#3855)

lib/astutils.cpp

@@ -514,6 +514,70 @@ const Token* getParentLifetime(const Token* tok)
     return tok;
 }
 
+static std::vector<const Token*> getParentMembers(const Token* tok)
+{
+    if (!tok)
+        return {};
+    if (!Token::simpleMatch(tok->astParent(), "."))
+        return {tok};
+    const Token* parent = tok;
+    while (Token::simpleMatch(parent->astParent(), "."))
+        parent = parent->astParent();
+    std::vector<const Token*> result;
+    for (const Token* tok2 : astFlatten(parent, ".")) {
+        if (Token::simpleMatch(tok2, "(") && Token::simpleMatch(tok2->astOperand1(), ".")) {
+            std::vector<const Token*> sub = getParentMembers(tok2->astOperand1());
+            result.insert(result.end(), sub.begin(), sub.end());
+        }
+        result.push_back(tok2);
+    }
+    return result;
+}
+
+const Token* getParentLifetime(bool cpp, const Token* tok, const Library* library)
+{
+    std::vector<const Token*> members = getParentMembers(tok);
+    if (members.size() < 2)
+        return tok;
+    // Find the first local variable or temporary
+    auto it = std::find_if(members.rbegin(), members.rend(), [&](const Token* tok2) {
+        const Variable* var = tok2->variable();
+        if (var) {
+            return var->isLocal() || var->isArgument();
+        } else {
+            return isTemporary(cpp, tok2, library);
+        }
+    });
+    if (it == members.rend())
+        return tok;
+    // If any of the submembers are borrowed types then stop
+    if (std::any_of(it.base() - 1, members.end() - 1, [&](const Token* tok2) {
+        if (astIsPointer(tok2) || astIsContainerView(tok2) || astIsIterator(tok2))
+            return true;
+        if (!astIsUniqueSmartPointer(tok2)) {
+            if (astIsSmartPointer(tok2))
+                return true;
+            const Token* dotTok = tok2->next();
+            if (!Token::simpleMatch(dotTok, ".")) {
+                const Token* endTok = nextAfterAstRightmostLeaf(tok2);
+                if (!endTok)
+                    dotTok = tok2->next();
+                else if (Token::simpleMatch(endTok, "."))
+                    dotTok = endTok;
+                else if (Token::simpleMatch(endTok->next(), "."))
+                    dotTok = endTok->next();
+            }
+            // If we are dereferencing the member variable then treat it as borrowed
+            if (Token::simpleMatch(dotTok, ".") && dotTok->originalName() == "->")
+                return true;
+        }
+        const Variable* var = tok2->variable();
+        return var && var->isReference();
+    }))
+        return nullptr;
+    return *it;
+}
+
 bool astIsLHS(const Token* tok)
 {
     if (!tok)
@@ -2708,9 +2772,9 @@ const Token* getLHSVariableToken(const Token* tok)
     if (tok->astOperand1()->varId() > 0)
         return tok->astOperand1();
     const Token* vartok = getLHSVariableRecursive(tok->astOperand1());
-    if (!vartok)
+    if (vartok && vartok->variable() && vartok->variable()->nameToken() == vartok)
-        return tok->astOperand1();
         return vartok;
+    return tok->astOperand1();
 }
 
 const Token* findAllocFuncCallToken(const Token *expr, const Library &library)

lib/astutils.h

@@ -149,6 +149,7 @@ const Token* astParentSkipParens(const Token* tok);
 const Token* getParentMember(const Token * tok);
 
 const Token* getParentLifetime(const Token* tok);
+const Token* getParentLifetime(bool cpp, const Token* tok, const Library* library);
 
 bool astIsLHS(const Token* tok);
 bool astIsRHS(const Token* tok);

lib/checkautovariables.cpp

@@ -520,70 +520,6 @@ static bool isAssignedToNonLocal(const Token* tok)
     return !var->isLocal() || var->isStatic();
 }
 
-static std::vector<const Token*> getParentMembers(const Token* tok)
-{
-    if (!tok)
-        return {};
-    if (!Token::simpleMatch(tok->astParent(), "."))
-        return {tok};
-    const Token* parent = tok;
-    while (Token::simpleMatch(parent->astParent(), "."))
-        parent = parent->astParent();
-    std::vector<const Token*> result;
-    for (const Token* tok2 : astFlatten(parent, ".")) {
-        if (Token::simpleMatch(tok2, "(") && Token::simpleMatch(tok2->astOperand1(), ".")) {
-            std::vector<const Token*> sub = getParentMembers(tok2->astOperand1());
-            result.insert(result.end(), sub.begin(), sub.end());
-        }
-        result.push_back(tok2);
-    }
-    return result;
-}
-
-static const Token* getParentLifetime(bool cpp, const Token* tok, const Library* library)
-{
-    std::vector<const Token*> members = getParentMembers(tok);
-    if (members.size() < 2)
-        return tok;
-    // Find the first local variable or temporary
-    auto it = std::find_if(members.rbegin(), members.rend(), [&](const Token* tok2) {
-        const Variable* var = tok2->variable();
-        if (var) {
-            return var->isLocal() || var->isArgument();
-        } else {
-            return isTemporary(cpp, tok2, library);
-        }
-    });
-    if (it == members.rend())
-        return tok;
-    // If any of the submembers are borrowed types then stop
-    if (std::any_of(it.base() - 1, members.end() - 1, [&](const Token* tok2) {
-        if (astIsPointer(tok2) || astIsContainerView(tok2) || astIsIterator(tok2))
-            return true;
-        if (!astIsUniqueSmartPointer(tok2)) {
-            if (astIsSmartPointer(tok2))
-                return true;
-            const Token* dotTok = tok2->next();
-            if (!Token::simpleMatch(dotTok, ".")) {
-                const Token* endTok = nextAfterAstRightmostLeaf(tok2);
-                if (!endTok)
-                    dotTok = tok2->next();
-                else if (Token::simpleMatch(endTok, "."))
-                    dotTok = endTok;
-                else if (Token::simpleMatch(endTok->next(), "."))
-                    dotTok = endTok->next();
-            }
-            // If we are dereferencing the member variable then treat it as borrowed
-            if (Token::simpleMatch(dotTok, ".") && dotTok->originalName() == "->")
-                return true;
-        }
-        const Variable* var = tok2->variable();
-        return var && var->isReference();
-    }))
-        return nullptr;
-    return *it;
-}
-
 void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end)
 {
     const bool printInconclusive = mSettings->certainty.isEnabled(Certainty::inconclusive);

lib/valueflow.cpp

@@ -2168,13 +2168,19 @@ struct ValueFlowAnalyzer : Analyzer {
         const ValueFlow::Value* value = getValue(tok);
         if (!value)
             return Action::None;
-        if (!(value->isIntValue() || value->isFloatValue() || value->isSymbolicValue()))
+        if (!(value->isIntValue() || value->isFloatValue() || value->isSymbolicValue() || value->isLifetimeValue()))
             return Action::None;
         const Token* parent = tok->astParent();
         // Only if its invertible
         if (value->isImpossible() && !Token::Match(parent, "+=|-=|*=|++|--"))
             return Action::None;
-
+        if (value->isLifetimeValue()) {
+            if (value->lifetimeKind != ValueFlow::Value::LifetimeKind::Iterator)
+                return Action::None;
+            if (!Token::Match(parent, "++|--|+="))
+                return Action::None;
+            return Action::Read | Action::Write;
+        }
         if (parent && parent->isAssignmentOp() && astIsLHS(tok) &&
             parent->astOperand2()->hasKnownValue()) {
             const Token* rhs = parent->astOperand2();
@@ -2206,6 +2212,9 @@ struct ValueFlowAnalyzer : Analyzer {
             return;
         if (!tok->astParent())
             return;
+        // Lifetime value doesn't change
+        if (value->isLifetimeValue())
+            return;
         if (tok->astParent()->isAssignmentOp()) {
             const ValueFlow::Value* rhsValue =
                 tok->astParent()->astOperand2()->getKnownValue(ValueFlow::Value::ValueType::INT);
@@ -3459,43 +3468,13 @@ const Token* getEndOfExprScope(const Token* tok, const Scope* defaultScope, bool
     return end;
 }
 
-static const Token* getEndOfVarScope(const Token* tok, const std::vector<const Variable*>& vars)
-{
-    const Token* endOfVarScope = nullptr;
-    for (const Variable* var : vars) {
-        const Scope *varScope = nullptr;
-        if (var && (var->isLocal() || var->isArgument()) && var->typeStartToken()->scope()->type != Scope::eNamespace)
-            varScope = var->typeStartToken()->scope();
-        else if (!endOfVarScope) {
-            varScope = tok->scope();
-            // A "local member" will be a expression like foo.x where foo is a local variable.
-            // A "global member" will be a member that belongs to a global object.
-            const bool globalMember = vars.size() == 1; // <- could check if it's a member here also but it seems redundant
-            if (var && (var->isGlobal() || var->isNamespace() || globalMember)) {
-                // Global variable => end of function
-                while (varScope->isLocal())
-                    varScope = varScope->nestedIn;
-            }
-        }
-        if (varScope && (!endOfVarScope || precedes(endOfVarScope, varScope->bodyEnd))) {
-            endOfVarScope = varScope->bodyEnd;
-        }
-    }
-    return endOfVarScope;
-}
-
 static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings)
 {
     // Forward lifetimes to constructed variable
     if (Token::Match(tok->previous(), "%var% {")) {
         std::list<ValueFlow::Value> values = tok->values();
         values.remove_if(&isNotLifetimeValue);
-        valueFlowForward(nextAfterAstRightmostLeaf(tok),
+        valueFlowForward(nextAfterAstRightmostLeaf(tok), getEndOfExprScope(tok), tok->previous(), values, tokenlist, settings);
-                         getEndOfVarScope(tok, {tok->variable()}),
-                         tok->previous(),
-                         values,
-                         tokenlist,
-                         settings);
         return;
     }
     Token *parent = tok->astParent();
@@ -3512,9 +3491,9 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog
         if (!isLifetimeBorrowed(parent->astOperand2(), settings))
             return;
 
-        std::vector<const Variable*> vars = getLHSVariables(parent);
+        const Token* expr = getLHSVariableToken(parent);
 
-        const Token* endOfVarScope = getEndOfVarScope(tok, vars);
+        const Token* endOfVarScope = getEndOfExprScope(expr);
 
         // Only forward lifetime values
         std::list<ValueFlow::Value> values = parent->astOperand2()->values();
@@ -3523,23 +3502,30 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog
         // Skip RHS
         const Token *nextExpression = nextAfterAstRightmostLeaf(parent);
 
-        if (Token::Match(parent->astOperand1(), ".|[|(") && parent->astOperand1()->exprId() > 0) {
+        if (expr->exprId() > 0) {
-            valueFlowForwardExpression(
+            valueFlowForwardExpression(const_cast<Token*>(nextExpression),
-                const_cast<Token*>(nextExpression), endOfVarScope, parent->astOperand1(), values, tokenlist, settings);
+                                       endOfVarScope->next(),
+                                       expr,
+                                       values,
+                                       tokenlist,
+                                       settings);
 
             for (ValueFlow::Value& val : values) {
                 if (val.lifetimeKind == ValueFlow::Value::LifetimeKind::Address)
                     val.lifetimeKind = ValueFlow::Value::LifetimeKind::SubObject;
             }
+            // TODO: handle `[`
+            if (Token::simpleMatch(parent->astOperand1(), ".")) {
+                const Token* parentLifetime =
+                    getParentLifetime(tokenlist->isCPP(), parent->astOperand1()->astOperand2(), &settings->library);
+                if (parentLifetime && parentLifetime->exprId() > 0) {
+                    valueFlowForward(const_cast<Token*>(nextExpression),
+                                     endOfVarScope,
+                                     parentLifetime,
+                                     values,
+                                     tokenlist,
+                                     settings);
                 }
-        for (const Variable* var : vars) {
-            valueFlowForward(
-                const_cast<Token*>(nextExpression), endOfVarScope, var->nameToken(), values, tokenlist, settings);
-
-            if (tok->astTop() && Token::simpleMatch(tok->astTop()->previous(), "for (") &&
-                Token::simpleMatch(tok->astTop()->link(), ") {")) {
-                Token* start = tok->astTop()->link()->next();
-                valueFlowForward(start, start->link(), var->nameToken(), values, tokenlist, settings);
             }
         }
         // Constructor
@@ -4915,7 +4901,7 @@ static void valueFlowForwardAssign(Token* const tok,
 {
     if (Token::simpleMatch(tok->astParent(), "return"))
         return;
-    const Token* endOfVarScope = getEndOfVarScope(tok, vars);
+    const Token* endOfVarScope = getEndOfExprScope(expr);
     if (std::any_of(values.begin(), values.end(), std::mem_fn(&ValueFlow::Value::isLifetimeValue))) {
         valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
         values.remove_if(std::mem_fn(&ValueFlow::Value::isLifetimeValue));