consolidated ValueFlow tuning options into ValueFlowOptions

cli/cmdlineparser.cpp

@@ -894,12 +894,12 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
             // Experimental: limit execution time for extended valueflow analysis. basic valueflow analysis
             // is always executed.
             else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) {
-                if (!parseNumberArg(argv[i], 33, mSettings.performanceValueFlowMaxTime, true))
+                if (!parseNumberArg(argv[i], 33, mSettings.vfOptions.maxTime, true))
                     return Result::Fail;
             }
 
             else if (std::strncmp(argv[i], "--performance-valueflow-max-if-count=", 37) == 0) {
-                if (!parseNumberArg(argv[i], 37, mSettings.performanceValueFlowMaxIfCount, true))
+                if (!parseNumberArg(argv[i], 37, mSettings.vfOptions.maxIfCount, true))
                     return Result::Fail;
             }
 
@@ -1323,7 +1323,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
             }
 
             else if (std::strncmp(argv[i], "--valueflow-max-iterations=", 27) == 0) {
-                if (!parseNumberArg(argv[i], 27, mSettings.valueFlowMaxIterations))
+                if (!parseNumberArg(argv[i], 27, mSettings.vfOptions.maxIterations))
                     return Result::Fail;
             }
 

lib/forwardanalyzer.cpp

@@ -648,7 +648,7 @@ namespace {
                     }
                 } else if (tok->isControlFlowKeyword() && Token::Match(tok, "if|while|for (") &&
                            Token::simpleMatch(tok->next()->link(), ") {")) {
-                    if (settings.checkLevel == Settings::CheckLevel::normal && ++branchCount > 4) {
+                    if ((settings.vfOptions.maxForwardBranches > 0) && (++branchCount > settings.vfOptions.maxForwardBranches)) {
                         // TODO: should be logged on function-level instead of file-level
                         reportError(Severity::information, "normalCheckLevelMaxBranches", "Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.");
                         return Break(Analyzer::Terminate::Bail);

lib/settings.cpp

@@ -290,14 +290,18 @@ void Settings::setCheckLevel(CheckLevel level)
     if (level == CheckLevel::normal) {
         // Checking should finish in reasonable time.
         checkLevel = level;
-        performanceValueFlowMaxSubFunctionArgs = 8;
-        performanceValueFlowMaxIfCount = 100;
+        vfOptions.maxSubFunctionArgs = 8;
+        vfOptions.maxIfCount = 100;
+        vfOptions.doConditionExpressionAnalysis = false;
+        vfOptions.maxForwardBranches = 4;
     }
     else if (level == CheckLevel::exhaustive) {
         // Checking can take a little while. ~ 10 times slower than normal analysis is OK.
         checkLevel = CheckLevel::exhaustive;
-        performanceValueFlowMaxIfCount = -1;
-        performanceValueFlowMaxSubFunctionArgs = 256;
+        vfOptions.maxIfCount = -1;
+        vfOptions.maxSubFunctionArgs = 256;
+        vfOptions.doConditionExpressionAnalysis = true;
+        vfOptions.maxForwardBranches = -1;
     }
 }
 

lib/settings.h

@@ -254,15 +254,6 @@ class CPPCHECKLIB WARN_UNUSED Settings {
 
     Platform platform;
 
-    /** @brief Experimental: --performance-valueflow-max-time=T */
-    int performanceValueFlowMaxTime = -1;
-
-    /** @brief --performance-valueflow-max-if-count=C */
-    int performanceValueFlowMaxIfCount = -1;
-
-    /** @brief max number of sets of arguments to pass to subfuncions in valueflow */
-    int performanceValueFlowMaxSubFunctionArgs = 256;
-
     /** @brief pid of cppcheck. Intention is that this is set in the main process. */
     int pid;
 
@@ -391,8 +382,42 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief forced includes given by the user */
     std::list<std::string> userIncludes;
 
-    /** @brief the maximum iterations of valueflow (--valueflow-max-iterations=T) */
-    std::size_t valueFlowMaxIterations = 4;
+    // TODO: adjust all options so 0 means "disabled" and -1 "means "unlimited"
+    struct ValueFlowOptions
+    {
+        /** @brief the maximum iterations to execute */
+        std::size_t maxIterations = 4;
+
+        /** @brief maximum numer if-branches */
+        int maxIfCount = -1;
+
+        /** @brief maximum number of sets of arguments to pass to subfuncions */
+        int maxSubFunctionArgs = 256;
+
+        /** @brief Experimental: maximum execution time */
+        int maxTime = -1;
+
+        /** @brief Control if condition expression analysis is performed */
+        bool doConditionExpressionAnalysis = true;
+
+        /** @brief Maximum performed for-loop count */
+        int maxForLoopCount = 10000;
+
+        /** @brief Maximum performed forward branches */
+        int maxForwardBranches = -1;
+
+        /** @brief Maximum performed alignof recursion */
+        int maxAlignOfRecursion = 100;
+
+        /** @brief Maximum performed sizeof recursion */
+        int maxSizeOfRecursion = 100;
+
+        /** @brief Maximum expression varid depth */
+        int maxExprVarIdDepth = 4;
+    };
+
+    /** @brief The ValueFlow options */
+    ValueFlowOptions vfOptions;
 
     /** @brief Is --verbose given? */
     bool verbose{};

lib/symboldatabase.cpp

@@ -3713,6 +3713,8 @@ bool Variable::arrayDimensions(const Settings& settings, bool& isContainer)
         // check for empty array dimension []
         if (dim->next()->str() != "]") {
             dimension_.tok = dim->astOperand2();
+            // TODO: only perform when ValueFlow is enabled
+            // TODO: collect timing information for this call?
             ValueFlow::valueFlowConstantFoldAST(const_cast<Token *>(dimension_.tok), settings);
             if (dimension_.tok && dimension_.tok->hasKnownIntValue()) {
                 dimension_.num = dimension_.tok->getKnownIntValue();

lib/tokenize.cpp

@@ -3414,6 +3414,7 @@ bool Tokenizer::simplifyTokens1(const std::string &configuration)
     if (!mSettings.buildDir.empty())
         Summaries::create(*this, configuration);
 
+    // TODO: apply this through Settings::ValueFlowOptions
     // TODO: do not run valueflow if no checks are being performed at all - e.g. unusedFunctions only
     const char* disableValueflowEnv = std::getenv("DISABLE_VALUEFLOW");
     const bool doValueFlow = !disableValueflowEnv || (std::strcmp(disableValueflowEnv, "1") != 0);

lib/valueflow.cpp

@@ -1161,8 +1161,10 @@ static size_t bitCeil(size_t x)
 
 static size_t getAlignOf(const ValueType& vt, const Settings& settings, int maxRecursion = 0)
 {
-    if (maxRecursion == 100)
+    if (maxRecursion == settings.vfOptions.maxAlignOfRecursion) {
+        // TODO: add bailout message
         return 0;
+    }
     if (vt.pointer || vt.reference != Reference::None || vt.isPrimitive()) {
         auto align = ValueFlow::getSizeOf(vt, settings);
         return align == 0 ? 0 : bitCeil(align);
@@ -1196,8 +1198,10 @@ static nonneg int getSizeOfType(const Token *typeTok, const Settings &settings)
 
 size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, int maxRecursion)
 {
-    if (maxRecursion == 100)
+    if (maxRecursion == settings.vfOptions.maxSizeOfRecursion) {
+        // TODO: add bailout message
         return 0;
+    }
     if (vt.pointer || vt.reference != Reference::None)
         return settings.platform.sizeof_pointer;
     if (vt.type == ValueType::Type::BOOL || vt.type == ValueType::Type::CHAR)
@@ -3294,9 +3298,10 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
     }
 
     void setupExprVarIds(const Token* start, int depth = 0) {
-        constexpr int maxDepth = 4;
-        if (depth > maxDepth)
+        if (depth > settings.vfOptions.maxExprVarIdDepth) {
+            // TODO: add bailout message
             return;
+        }
         visitAstNodes(start, [&](const Token* tok) {
             const bool top = depth == 0 && tok == start;
             const bool ispointer = astIsPointer(tok) || astIsSmartPointer(tok) || astIsIterator(tok);
@@ -5399,7 +5404,7 @@ static const Scope* getLoopScope(const Token* tok)
 //
 static void valueFlowConditionExpressions(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger &errorLogger, const Settings &settings)
 {
-    if (!settings.daca && (settings.checkLevel == Settings::CheckLevel::normal))
+    if (!settings.daca && !settings.vfOptions.doConditionExpressionAnalysis)
     {
         if (settings.debugwarnings) {
             ErrorMessage::FileLocation loc(tokenlist.getSourceFilePath(), 0, 0);
@@ -5416,7 +5421,7 @@ static void valueFlowConditionExpressions(const TokenList &tokenlist, const Symb
             continue;
         }
 
-        if (settings.daca && (settings.checkLevel == Settings::CheckLevel::normal))
+        if (settings.daca && !settings.vfOptions.doConditionExpressionAnalysis)
             continue;
 
         for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
@@ -7195,13 +7200,14 @@ static bool valueFlowForLoop2(const Token *tok,
     ProgramMemory startMemory(programMemory);
     ProgramMemory endMemory;
 
-    int maxcount = 10000;
+    int maxcount = settings.vfOptions.maxForLoopCount;
     while (result != 0 && !error && --maxcount > 0) {
         endMemory = programMemory;
         execute(thirdExpression, programMemory, &result, &error, settings);
         if (!error)
             execute(secondExpression, programMemory, &result, &error, settings);
     }
+    // TODO: add bailout message
 
     if (memory1)
         memory1->swap(startMemory);
@@ -7580,7 +7586,7 @@ static bool productParams(const Settings& settings, const std::unordered_map<Key
         args.back()[p.first] = p.second.front();
     }
     bool bail = false;
-    int max = settings.performanceValueFlowMaxSubFunctionArgs;
+    int max = settings.vfOptions.maxSubFunctionArgs;
     for (const auto& p:vars) {
         if (args.size() > max) {
             bail = true;
@@ -7611,6 +7617,7 @@ static bool productParams(const Settings& settings, const std::unordered_map<Key
     if (args.size() > max) {
         bail = true;
         args.resize(max);
+        // TODO: add bailout message
     }
 
     for (const auto& arg:args) {
@@ -9474,7 +9481,7 @@ struct ValueFlowPassRunner {
     bool run(std::initializer_list<ValuePtr<ValueFlowPass>> passes) const
     {
         std::size_t values = 0;
-        std::size_t n = state.settings.valueFlowMaxIterations;
+        std::size_t n = state.settings.vfOptions.maxIterations;
         while (n > 0 && values != getTotalValues()) {
             values = getTotalValues();
             if (std::any_of(passes.begin(), passes.end(), [&](const ValuePtr<ValueFlowPass>& pass) {
@@ -9501,8 +9508,10 @@ struct ValueFlowPassRunner {
     bool run(const ValuePtr<ValueFlowPass>& pass) const
     {
         auto start = Clock::now();
-        if (start > stop)
+        if (start > stop) {
+            // TODO: add bailout message
             return true;
+        }
         if (!state.tokenlist.isCPP() && pass->cpp())
             return false;
         if (timerResults) {
@@ -9524,7 +9533,7 @@ struct ValueFlowPassRunner {
 
     void setSkippedFunctions()
     {
-        if (state.settings.performanceValueFlowMaxIfCount > 0) {
+        if (state.settings.vfOptions.maxIfCount > 0) {
             for (const Scope* functionScope : state.symboldatabase.functionScopes) {
                 int countIfScopes = 0;
                 std::vector<const Scope*> scopes{functionScope};
@@ -9537,7 +9546,7 @@ struct ValueFlowPassRunner {
                             ++countIfScopes;
                     }
                 }
-                if (countIfScopes > state.settings.performanceValueFlowMaxIfCount) {
+                if (countIfScopes > state.settings.vfOptions.maxIfCount) {
                     state.skippedFunctions.emplace(functionScope);
 
                     if (state.settings.severity.isEnabled(Severity::information)) {
@@ -9550,7 +9559,7 @@ struct ValueFlowPassRunner {
                                                   Severity::information,
                                                   "Limiting ValueFlow analysis in function '" + functionName + "' since it is too complex. "
                                                   "Please specify --check-level=exhaustive to perform full analysis.",
-                                                  "checkLevelNormal",
+                                                  "checkLevelNormal", // TODO: use more specific ID
                                                   Certainty::normal);
                         state.errorLogger.reportErr(errmsg);
                     }
@@ -9561,8 +9570,8 @@ struct ValueFlowPassRunner {
 
     void setStopTime()
     {
-        if (state.settings.performanceValueFlowMaxTime >= 0)
-            stop = Clock::now() + std::chrono::seconds{state.settings.performanceValueFlowMaxTime};
+        if (state.settings.vfOptions.maxTime >= 0)
+            stop = Clock::now() + std::chrono::seconds{state.settings.vfOptions.maxTime};
     }
 
     ValueFlowState state;

test/testcmdlineparser.cpp

@@ -1906,14 +1906,14 @@ class TestCmdlineParser : public TestFixture {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=0", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
-        ASSERT_EQUALS(0, settings->valueFlowMaxIterations);
+        ASSERT_EQUALS(0, settings->vfOptions.maxIterations);
     }
 
     void valueFlowMaxIterations2() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=11", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
-        ASSERT_EQUALS(11, settings->valueFlowMaxIterations);
+        ASSERT_EQUALS(11, settings->vfOptions.maxIterations);
     }
 
     void valueFlowMaxIterationsInvalid() {
@@ -2006,7 +2006,7 @@ class TestCmdlineParser : public TestFixture {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--performance-valueflow-max-time=12", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
-        ASSERT_EQUALS(12, settings->performanceValueFlowMaxTime);
+        ASSERT_EQUALS(12, settings->vfOptions.maxTime);
     }
 
     void performanceValueflowMaxTimeInvalid() {
@@ -2020,7 +2020,7 @@ class TestCmdlineParser : public TestFixture {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--performance-valueflow-max-if-count=12", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
-        ASSERT_EQUALS(12, settings->performanceValueFlowMaxIfCount);
+        ASSERT_EQUALS(12, settings->vfOptions.maxIfCount);
     }
 
     void performanceValueFlowMaxIfCountInvalid() {
@@ -2569,31 +2569,38 @@ class TestCmdlineParser : public TestFixture {
     }
 #endif
 
+    // the CLI default to --check-level=normal
     void checkLevelDefault() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(2, argv));
         ASSERT_EQUALS_ENUM(Settings::CheckLevel::normal, settings->checkLevel);
-        ASSERT_EQUALS(100, settings->performanceValueFlowMaxIfCount);
-        ASSERT_EQUALS(8, settings->performanceValueFlowMaxSubFunctionArgs);
+        ASSERT_EQUALS(100, settings->vfOptions.maxIfCount);
+        ASSERT_EQUALS(8, settings->vfOptions.maxSubFunctionArgs);
+        ASSERT_EQUALS(false, settings->vfOptions.doConditionExpressionAnalysis);
+        ASSERT_EQUALS(4, settings->vfOptions.maxForwardBranches);
     }
 
     void checkLevelNormal() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--check-level=normal", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
         ASSERT_EQUALS_ENUM(Settings::CheckLevel::normal, settings->checkLevel);
-        ASSERT_EQUALS(100, settings->performanceValueFlowMaxIfCount);
-        ASSERT_EQUALS(8, settings->performanceValueFlowMaxSubFunctionArgs);
+        ASSERT_EQUALS(100, settings->vfOptions.maxIfCount);
+        ASSERT_EQUALS(8, settings->vfOptions.maxSubFunctionArgs);
+        ASSERT_EQUALS(false, settings->vfOptions.doConditionExpressionAnalysis);
+        ASSERT_EQUALS(4, settings->vfOptions.maxForwardBranches);
     }
 
     void checkLevelExhaustive() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--check-level=exhaustive", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
         ASSERT_EQUALS_ENUM(Settings::CheckLevel::exhaustive, settings->checkLevel);
-        ASSERT_EQUALS(-1, settings->performanceValueFlowMaxIfCount);
-        ASSERT_EQUALS(256, settings->performanceValueFlowMaxSubFunctionArgs);
+        ASSERT_EQUALS(-1, settings->vfOptions.maxIfCount);
+        ASSERT_EQUALS(256, settings->vfOptions.maxSubFunctionArgs);
+        ASSERT_EQUALS(true, settings->vfOptions.doConditionExpressionAnalysis);
+        ASSERT_EQUALS(-1, settings->vfOptions.maxForwardBranches);
     }
 
     void checkLevelUnknown() {

test/testsettings.cpp

@@ -272,8 +272,10 @@ class TestSettings : public TestFixture {
     {
         Settings s;
         ASSERT_EQUALS_ENUM(s.checkLevel, Settings::CheckLevel::exhaustive);
-        ASSERT_EQUALS(s.performanceValueFlowMaxIfCount, -1);
-        ASSERT_EQUALS(s.performanceValueFlowMaxSubFunctionArgs, 256);
+        ASSERT_EQUALS(s.vfOptions.maxIfCount, -1);
+        ASSERT_EQUALS(s.vfOptions.maxSubFunctionArgs, 256);
+        ASSERT_EQUALS(true, s.vfOptions.doConditionExpressionAnalysis);
+        ASSERT_EQUALS(-1, s.vfOptions.maxForwardBranches);
     }
 };
 

test/testvalueflow.cpp

@@ -8537,7 +8537,7 @@ class TestValueFlow : public TestFixture {
 
     void performanceIfCount() {
         /*const*/ Settings s(settings);
-        s.performanceValueFlowMaxIfCount = 1;
+        s.vfOptions.maxIfCount = 1;
 
         const char *code;