ValueFlow: various small optimizations

lib/astutils.cpp

@@ -69,7 +69,9 @@ const Token* findExpression(const nonneg int exprid,
 static int findArgumentPosRecursive(const Token* tok, const Token* tokToFind,  bool &found, nonneg int depth=0)
 {
     ++depth;
-    if (!tok || depth >= 100)
+    if (depth >= 100)
+        return -1; // TODO: add bailout message
+    if (!tok)
         return -1;
     if (tok->str() == ",") {
         int res = findArgumentPosRecursive(tok->astOperand1(), tokToFind, found, depth);
@@ -112,8 +114,10 @@ template<class T, class OuputIterator, REQUIRES("T must be a Token class", std::
 static void astFlattenCopy(T* tok, const char* op, OuputIterator out, int depth = 100)
 {
     --depth;
-    if (!tok || depth < 0)
+    if (!tok || depth < 0) {
+        // TODO: add bailout message
         return;
+    }
     if (strcmp(tok->str().c_str(), op) == 0) {
         astFlattenCopy(tok->astOperand1(), op, out, depth);
         astFlattenCopy(tok->astOperand2(), op, out, depth);
@@ -140,8 +144,10 @@ std::vector<Token*> astFlatten(Token* tok, const char* op)
 nonneg int astCount(const Token* tok, const char* op, int depth)
 {
     --depth;
-    if (!tok || depth < 0)
+    if (!tok || depth < 0) {
+        // TODO: add bailout message
         return 0;
+    }
     if (strcmp(tok->str().c_str(), op) == 0)
         return astCount(tok->astOperand1(), op, depth) + astCount(tok->astOperand2(), op, depth);
     return 1;
@@ -1118,6 +1124,7 @@ bool exprDependsOnThis(const Token* expr, bool onVar, nonneg int depth)
         return true;
     if (depth >= 1000)
         // Abort recursion to avoid stack overflow
+        // TODO: add bailout message
         return true;
     ++depth;
 
@@ -1260,6 +1267,7 @@ SmallVector<ReferenceToken> followAllReferences(const Token* tok,
     if (!tok)
         return {};
     if (depth < 0) {
+        // TODO: add bailout message
         SmallVector<ReferenceToken> refs_result;
         refs_result.emplace_back(tok, std::move(errors));
         return refs_result;
@@ -2891,8 +2899,10 @@ static bool isExpressionChangedAt(const F& getExprTok,
                                   const Settings& settings,
                                   int depth)
 {
-    if (depth < 0)
+    if (depth < 0) {
+        // TODO: add bailout message
         return true;
+    }
     if (!isMutableExpression(tok))
         return false;
     if (tok->exprId() != exprid || (!tok->varId() && !tok->isName())) {
@@ -2942,8 +2952,10 @@ Token* findVariableChanged(Token *start, const Token *end, int indirect, const n
 {
     if (!precedes(start, end))
         return nullptr;
-    if (depth < 0)
+    if (depth < 0) {
+        // TODO: add bailout message
         return start;
+    }
     auto getExprTok = utils::memoize([&] {
         return findExpression(start, exprid);
     });
@@ -3041,8 +3053,10 @@ static const Token* findExpressionChangedImpl(const Token* expr,
                                               int depth,
                                               Find find)
 {
-    if (depth < 0)
+    if (depth < 0) {
+        // TODO: add bailout message
         return start;
+    }
     if (!precedes(start, end))
         return nullptr;
     const Token* result = nullptr;

lib/checkstl.cpp

@@ -1087,7 +1087,7 @@ static const ValueFlow::Value* getInnerLifetime(const Token* tok,
                                                 int depth = 4)
 {
     if (depth < 0)
-        return nullptr;
+        return nullptr;// TODO: add bailout message
     if (!tok)
         return nullptr;
     for (const ValueFlow::Value& val : tok->values()) {

lib/forwardanalyzer.cpp

@@ -46,6 +46,7 @@
 namespace {
     struct ForwardTraversal {
         enum class Progress : std::uint8_t { Continue, Break, Skip };
+        // TODO: analyzer is copied
         ForwardTraversal(const ValuePtr<Analyzer>& analyzer, const TokenList& tokenList, ErrorLogger& errorLogger, const Settings& settings)
             : analyzer(analyzer), tokenList(tokenList), errorLogger(errorLogger), settings(settings)
         {}
@@ -577,7 +578,7 @@ namespace {
 
         Progress updateRange(Token* start, const Token* end, int depth = 20) {
             if (depth < 0)
-                return Break(Analyzer::Terminate::Bail);
+                return Break(Analyzer::Terminate::Bail); // TODO: add bailout message
             std::size_t i = 0;
             for (Token* tok = start; precedes(tok, end); tok = tok->next()) {
                 Token* next = nullptr;

lib/programmemory.cpp

@@ -580,7 +580,7 @@ ProgramMemory ProgramMemoryState::get(const Token* tok, const Token* ctx, const
     } else {
         local.removeModifiedVars(ctx);
     }
-    return local.state;
+    return std::move(local.state);
 }
 
 ProgramMemory getProgramMemory(const Token* tok, const Token* expr, const ValueFlow::Value& value, const Settings& settings)
@@ -1365,7 +1365,7 @@ namespace {
 
             nonneg int n = astCount(expr, expr->str().c_str());
             if (n > 50)
-                return unknown();
+                return unknown(); // TODO: add bailout message
             std::vector<const Token*> conditions1 = flattenConditions(expr);
             if (conditions1.empty())
                 return unknown();
@@ -1382,7 +1382,7 @@ namespace {
             }
             if (condValues.size() == conditions1.size() && allNegated)
                 return negatedValue;
-            if (n > 4)
+            if (n > 4) // TODO: add bailout message
                 return unknown();
             if (!sortConditions(conditions1))
                 return unknown();
@@ -1623,6 +1623,7 @@ namespace {
                         return execute(tok);
                     });
                     if (f) {
+                        // TODO: add bailout message
                         if (fdepth >= 0 && !f->isImplicitlyVirtual()) {
                             ProgramMemory functionState;
                             for (std::size_t i = 0; i < args.size(); ++i) {
@@ -1711,7 +1712,7 @@ namespace {
                     depth++;
                 }};
             if (depth < 0)
-                return unknown();
+                return unknown(); // TODO: add bailout message
             ValueFlow::Value v = unknown();
             if (updateValue(v, executeImpl(expr)))
                 return v;

lib/valueflow.cpp

@@ -287,13 +287,12 @@ static void parseCompareEachInt(
         return;
     if (tok->isComparisonOp()) {
         std::vector<ValueFlow::Value> value1 = evaluate(tok->astOperand1());
-        std::vector<ValueFlow::Value> value2 = evaluate(tok->astOperand2());
-        if (!value1.empty() && !value2.empty()) {
-            if (tok->astOperand1()->hasKnownIntValue())
-                value2.clear();
-            if (tok->astOperand2()->hasKnownIntValue())
-                value1.clear();
+        std::vector<ValueFlow::Value> value2;
+        if (value1.empty() || !tok->astOperand1()->hasKnownIntValue()) {
+            value2 = evaluate(tok->astOperand2());
         }
+        if (!value2.empty() && tok->astOperand2()->hasKnownIntValue())
+            value1.clear();
         for (const ValueFlow::Value& v1 : value1) {
             if (isSaturated(v1.intvalue) || astIsFloat(tok->astOperand2(), /*unknown*/ false))
                 continue;
@@ -396,28 +395,28 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val
         result.tokvalue = value1.tokvalue;
     else if (value2.tokvalue)
         result.tokvalue = value2.tokvalue;
-    if (value1.isSymbolicValue()) {
-        result.valueType = value1.valueType;
-        result.tokvalue = value1.tokvalue;
-    }
     if (value2.isSymbolicValue()) {
         result.valueType = value2.valueType;
         result.tokvalue = value2.tokvalue;
     }
-    if (value1.isIteratorValue())
+    else if (value1.isSymbolicValue()) {
         result.valueType = value1.valueType;
+        result.tokvalue = value1.tokvalue;
+    }
     if (value2.isIteratorValue())
         result.valueType = value2.valueType;
+    else if (value1.isIteratorValue())
+        result.valueType = value1.valueType;
     result.condition = value1.condition ? value1.condition : value2.condition;
     result.varId = (value1.varId != 0) ? value1.varId : value2.varId;
     result.varvalue = (result.varId == value1.varId) ? value1.varvalue : value2.varvalue;
     result.errorPath = (value1.errorPath.empty() ? value2 : value1).errorPath;
     result.safe = value1.safe || value2.safe;
     if (value1.bound == ValueFlow::Value::Bound::Point || value2.bound == ValueFlow::Value::Bound::Point) {
-        if (value1.bound == ValueFlow::Value::Bound::Upper || value2.bound == ValueFlow::Value::Bound::Upper)
-            result.bound = ValueFlow::Value::Bound::Upper;
         if (value1.bound == ValueFlow::Value::Bound::Lower || value2.bound == ValueFlow::Value::Bound::Lower)
             result.bound = ValueFlow::Value::Bound::Lower;
+        else if (value1.bound == ValueFlow::Value::Bound::Upper || value2.bound == ValueFlow::Value::Bound::Upper)
+            result.bound = ValueFlow::Value::Bound::Upper;
     }
     if (value1.path != value2.path)
         result.path = -1;
@@ -1216,7 +1215,7 @@ static std::vector<MathLib::bigint> minUnsignedValue(const Token* tok, int depth
     if (!tok)
         return result;
     if (depth < 0)
-        return result;
+        return result; // TODO: add bailout message
     if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::INT)) {
         result = {v->intvalue};
     } else if (!Token::Match(tok, "-|%|&|^") && tok->isConstOp() && tok->astOperand1() && tok->astOperand2()) {
@@ -1715,7 +1714,7 @@ static std::vector<ValueFlow::LifetimeToken> getLifetimeTokens(const Token* tok,
     if (pred(tok))
         return {{tok, std::move(errorPath)}};
     if (depth < 0)
-        return {{tok, std::move(errorPath)}};
+        return {{tok, std::move(errorPath)}}; // TODO: add bailout message
     if (var && var->declarationId() == tok->varId()) {
         if (var->isReference() || var->isRValueReference()) {
             const Token * const varDeclEndToken = var->declEndToken();
@@ -2159,11 +2158,9 @@ static void valueFlowForwardLifetime(Token * tok, const TokenList &tokenlist, Er
         if (!expr)
             return;
 
-        if (expr->exprId() == 0)
+        if (expr->exprId() <= 0)
             return;
 
-        const Token* endOfVarScope = ValueFlow::getEndOfExprScope(expr);
-
         // Only forward lifetime values
         std::list<ValueFlow::Value> values = parent->astOperand2()->values();
         values.remove_if(&isNotLifetimeValue);
@@ -2176,21 +2173,21 @@ static void valueFlowForwardLifetime(Token * tok, const TokenList &tokenlist, Er
 
         // Skip RHS
         Token* nextExpression = nextAfterAstRightmostLeaf(parent);
+        const Token* endOfVarScope = ValueFlow::getEndOfExprScope(expr);
 
-        if (expr->exprId() > 0) {
-            valueFlowForward(nextExpression, endOfVarScope->next(), expr, values, tokenlist, errorLogger, settings);
+        valueFlowForward(nextExpression, endOfVarScope->next(), expr, values, tokenlist, errorLogger, settings);
 
+        // TODO: handle `[`
+        if (Token::simpleMatch(parent->astOperand1(), ".")) {
             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(parent->astOperand1()->astOperand2(), settings.library);
-                if (parentLifetime && parentLifetime->exprId() > 0) {
-                    valueFlowForward(nextExpression, endOfVarScope, parentLifetime, std::move(values), tokenlist, errorLogger, settings);
-                }
+
+            const Token* parentLifetime =
+                getParentLifetime(parent->astOperand1()->astOperand2(), settings.library);
+            if (parentLifetime && parentLifetime->exprId() > 0) {
+                valueFlowForward(nextExpression, endOfVarScope, parentLifetime, std::move(values), tokenlist, errorLogger, settings);
             }
         }
         // Constructor
@@ -2509,7 +2506,7 @@ struct LifetimeStore {
 static bool hasBorrowingVariables(const std::list<Variable>& vars, const std::vector<const Token*>& args, int depth = 10)
 {
     if (depth < 0)
-        return true;
+        return true; // TODO: add bailout message
     return std::any_of(vars.cbegin(), vars.cend(), [&](const Variable& var) {
         if (const ValueType* vt = var.valueType()) {
             if (vt->pointer > 0 &&
@@ -3779,7 +3776,7 @@ static void valueFlowSymbolic(const TokenList& tokenlist, const SymbolDatabase&
 static const Token* isStrlenOf(const Token* tok, const Token* expr, int depth = 10)
 {
     if (depth < 0)
-        return nullptr;
+        return nullptr; // TODO: add bailout message
     if (!tok)
         return nullptr;
     if (!expr)
@@ -3899,10 +3896,10 @@ static void valueFlowSymbolicOperators(const SymbolDatabase& symboldatabase, con
 }
 
 struct SymbolicInferModel : InferModel {
-    const Token* expr;
     explicit SymbolicInferModel(const Token* tok) : expr(tok) {
         assert(expr->exprId() != 0);
     }
+private:
     bool match(const ValueFlow::Value& value) const override
     {
         return value.isSymbolicValue() && value.tokvalue && value.tokvalue->exprId() == expr->exprId();
@@ -3915,6 +3912,7 @@ struct SymbolicInferModel : InferModel {
         result.setKnown();
         return result;
     }
+    const Token* expr;
 };
 
 static void valueFlowSymbolicInfer(const SymbolDatabase& symboldatabase, const Settings& settings)
@@ -5909,7 +5907,7 @@ static const ValueFlow::Value* getKnownValueFromToken(const Token* tok)
     if (!tok)
         return nullptr;
     auto it = std::find_if(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) {
-        return (v.isIntValue() || v.isContainerSizeValue() || v.isFloatValue()) && v.isKnown();
+        return v.isKnown() && (v.isIntValue() || v.isContainerSizeValue() || v.isFloatValue());
     });
     if (it == tok->values().end())
         return nullptr;
@@ -6267,6 +6265,7 @@ static bool isContainerSizeChangedByFunction(const Token* tok,
                 // Argument not used
                 if (!arg->nameToken())
                     return false;
+                // TODO: add bailout message
                 if (depth > 0)
                     return isContainerSizeChanged(arg->nameToken(),
                                                   scope->bodyStart,

lib/vf_analyzers.cpp

@@ -118,7 +118,7 @@ struct ValueFlowAnalyzer : Analyzer {
         if (!tok)
             return {};
         if (depth < 0)
-            return {};
+            return {}; // TODO: add bailout message
         depth--;
         if (analyze(tok, Direction::Forward).isRead()) {
             ConditionState result;
@@ -825,7 +825,7 @@ static bool bifurcateVariableChanged(const Variable* var,
 static bool bifurcate(const Token* tok, const std::set<nonneg int>& varids, const Settings& settings, int depth)
 {
     if (depth < 0)
-        return false;
+        return false; // TODO: add bailout message
     if (!tok)
         return true;
     if (tok->hasKnownIntValue())