Refactorization: Moved code from header to source

- from utils.h to new utils.cpp
- from token.h to token.cpp
- from valueflow.h to valueflow.cpp
- from errorlogger.h to errorlogger.cpp

lib/astutils.cpp

@@ -27,6 +27,7 @@
 #include "token.h"
 #include "valueflow.h"
 
+#include <algorithm>
 #include <iterator>
 #include <list>
 #include <stack>

lib/cppcheck.vcxproj

@@ -96,6 +96,7 @@
     <ClCompile Include="token.cpp" />
     <ClCompile Include="tokenize.cpp" />
     <ClCompile Include="tokenlist.cpp" />
+    <ClCompile Include="utils.cpp" />
     <ClCompile Include="valueflow.cpp" />
     <ClCompile Include="forwardanalyzer.cpp" />
   </ItemGroup>
@@ -558,4 +559,4 @@ xcopy "$(SolutionDir)platforms" "$(OutDir)platforms" /E /I /D /Y</Command>
   </ItemDefinitionGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets" />
-</Project>
+</Project>

lib/cppcheck.vcxproj.filters

@@ -170,6 +170,12 @@
     <ClCompile Include="clangimport.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="forwardanalyzer.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
+    <ClCompile Include="utils.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="checkbufferoverrun.h">
@@ -338,4 +344,4 @@
   <ItemGroup>
     <Natvis Include="cppcheck.natvis" />
   </ItemGroup>
-</Project>
+</Project>

lib/errorlogger.cpp

@@ -58,6 +58,51 @@ InternalError::InternalError(const Token *tok, const std::string &errorMsg, Type
     }
 }
 
+std::string Severity::toString(Severity::SeverityType severity)
+{
+    switch (severity) {
+    case none:
+        return "";
+    case error:
+        return "error";
+    case warning:
+        return "warning";
+    case style:
+        return "style";
+    case performance:
+        return "performance";
+    case portability:
+        return "portability";
+    case information:
+        return "information";
+    case debug:
+        return "debug";
+    }
+    throw InternalError(nullptr, "Unknown severity");
+}
+Severity::SeverityType Severity::fromString(const std::string& severity)
+{
+    if (severity.empty())
+        return none;
+    if (severity == "none")
+        return none;
+    if (severity == "error")
+        return error;
+    if (severity == "warning")
+        return warning;
+    if (severity == "style")
+        return style;
+    if (severity == "performance")
+        return performance;
+    if (severity == "portability")
+        return portability;
+    if (severity == "information")
+        return information;
+    if (severity == "debug")
+        return debug;
+    return none;
+}
+
 ErrorLogger::ErrorMessage::ErrorMessage()
     : incomplete(false), severity(Severity::none), cwe(0U), inconclusive(false)
 {

lib/errorlogger.h

@@ -126,48 +126,8 @@ public:
         debug
     };
 
-    static std::string toString(SeverityType severity) {
+    static std::string toString(SeverityType severity);
-        switch (severity) {
+    static SeverityType fromString(const std::string& severity);
-        case none:
-            return "";
-        case error:
-            return "error";
-        case warning:
-            return "warning";
-        case style:
-            return "style";
-        case performance:
-            return "performance";
-        case portability:
-            return "portability";
-        case information:
-            return "information";
-        case debug:
-            return "debug";
-        }
-        throw InternalError(nullptr, "Unknown severity");
-    }
-    static SeverityType fromString(const std::string &severity) {
-        if (severity.empty())
-            return none;
-        if (severity == "none")
-            return none;
-        if (severity == "error")
-            return error;
-        if (severity == "warning")
-            return warning;
-        if (severity == "style")
-            return style;
-        if (severity == "performance")
-            return performance;
-        if (severity == "portability")
-            return portability;
-        if (severity == "information")
-            return information;
-        if (severity == "debug")
-            return debug;
-        return none;
-    }
 };
 
 

lib/forwardanalyzer.cpp

@@ -5,6 +5,7 @@
 #include "token.h"
 #include "valueptr.h"
 
+#include <algorithm>
 #include <functional>
 
 struct ForwardTraversal {

lib/pathanalysis.cpp

@@ -3,6 +3,7 @@
 #include "symboldatabase.h"
 #include "token.h"
 #include "valueflow.h"
+#include <algorithm>
 
 const Scope* PathAnalysis::findOuterScope(const Scope * scope)
 {

lib/token.cpp

@@ -25,6 +25,7 @@
 #include "symboldatabase.h"
 #include "utils.h"
 
+#include <algorithm>
 #include <cassert>
 #include <cctype>
 #include <cstring>
@@ -2127,6 +2128,83 @@ std::shared_ptr<ScopeInfo2> Token::scopeInfo() const
     return mImpl->mScopeInfo;
 }
 
+bool Token::hasKnownIntValue() const
+{
+    if (!mImpl->mValues)
+        return false;
+    return std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) {
+        return value.isKnown() && value.isIntValue();
+    });
+}
+
+bool Token::hasKnownValue() const
+{
+    return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown));
+}
+
+bool Token::isImpossibleIntValue(const MathLib::bigint val) const
+{
+    if (!mImpl->mValues)
+        return false;
+    for (const auto& v : *mImpl->mValues) {
+        if (v.isIntValue() && v.isImpossible() && v.intvalue == val)
+            return true;
+        if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Lower && val > v.intvalue)
+            return true;
+        if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Upper && val < v.intvalue)
+            return true;
+    }
+    return false;
+}
+
+const ValueFlow::Value* Token::getValue(const MathLib::bigint val) const
+{
+    if (!mImpl->mValues)
+        return nullptr;
+    const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) {
+        return value.isIntValue() && !value.isImpossible() && value.intvalue == val;
+    });
+    return it == mImpl->mValues->end() ? nullptr : &*it;
+}
+
+const ValueFlow::Value* Token::getMaxValue(bool condition) const
+{
+    if (!mImpl->mValues)
+        return nullptr;
+    const ValueFlow::Value* ret = nullptr;
+    for (const ValueFlow::Value& value : *mImpl->mValues) {
+        if (!value.isIntValue())
+            continue;
+        if (value.isImpossible())
+            continue;
+        if ((!ret || value.intvalue > ret->intvalue) &&
+            ((value.condition != nullptr) == condition))
+            ret = &value;
+    }
+    return ret;
+}
+
+const ValueFlow::Value* Token::getMovedValue() const
+{
+    if (!mImpl->mValues)
+        return nullptr;
+    const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) {
+        return value.isMovedValue() && !value.isImpossible() &&
+               value.moveKind != ValueFlow::Value::MoveKind::NonMovedVariable;
+    });
+    return it == mImpl->mValues->end() ? nullptr : &*it;
+}
+
+const ValueFlow::Value* Token::getContainerSizeValue(const MathLib::bigint val) const
+{
+    if (!mImpl->mValues)
+        return nullptr;
+    const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) {
+        return value.isContainerSizeValue() && !value.isImpossible() && value.intvalue == val;
+    });
+    return it == mImpl->mValues->end() ? nullptr : &*it;
+}
+
 TokenImpl::~TokenImpl()
 {
     delete mOriginalName;

lib/token.h

@@ -27,7 +27,6 @@
 #include "templatesimplifier.h"
 #include "utils.h"
 
-#include <algorithm>
 #include <cstddef>
 #include <functional>
 #include <list>
@@ -1016,84 +1015,27 @@ public:
             *mImpl->mOriginalName = name;
     }
 
-    bool hasKnownIntValue() const {
+    bool hasKnownIntValue() const;
-        if (!mImpl->mValues)
+    bool hasKnownValue() const;
-            return false;
-        return std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value &value) {
-            return value.isKnown() && value.isIntValue();
-        });
-    }
-
-    bool hasKnownValue() const {
-        return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown));
-    }
 
     MathLib::bigint getKnownIntValue() const {
         return mImpl->mValues->front().intvalue;
     }
 
-    bool isImpossibleIntValue(const MathLib::bigint val) const {
+    bool isImpossibleIntValue(const MathLib::bigint val) const;
-        if (!mImpl->mValues)
-            return false;
-        for (const auto &v: *mImpl->mValues) {
-            if (v.isIntValue() && v.isImpossible() && v.intvalue == val)
-                return true;
-            if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Lower && val > v.intvalue)
-                return true;
-            if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Upper && val < v.intvalue)
-                return true;
-        }
-        return false;
-    }
 
-    const ValueFlow::Value * getValue(const MathLib::bigint val) const {
+    const ValueFlow::Value* getValue(const MathLib::bigint val) const;
-        if (!mImpl->mValues)
-            return nullptr;
-        const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) {
-            return value.isIntValue() && !value.isImpossible() && value.intvalue == val;
-        });
-        return it == mImpl->mValues->end() ? nullptr : &*it;
-    }
 
-    const ValueFlow::Value * getMaxValue(bool condition) const {
+    const ValueFlow::Value* getMaxValue(bool condition) const;
-        if (!mImpl->mValues)
-            return nullptr;
-        const ValueFlow::Value *ret = nullptr;
-        for (const ValueFlow::Value &value : *mImpl->mValues) {
-            if (!value.isIntValue())
-                continue;
-            if (value.isImpossible())
-                continue;
-            if ((!ret || value.intvalue > ret->intvalue) &&
-                ((value.condition != nullptr) == condition))
-                ret = &value;
-        }
-        return ret;
-    }
 
-    const ValueFlow::Value * getMovedValue() const {
+    const ValueFlow::Value* getMovedValue() const;
-        if (!mImpl->mValues)
-            return nullptr;
-        const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) {
-            return value.isMovedValue() && !value.isImpossible() &&
-                   value.moveKind != ValueFlow::Value::MoveKind::NonMovedVariable;
-        });
-        return it == mImpl->mValues->end() ? nullptr : &*it;
-    }
 
     const ValueFlow::Value * getValueLE(const MathLib::bigint val, const Settings *settings) const;
     const ValueFlow::Value * getValueGE(const MathLib::bigint val, const Settings *settings) const;
 
     const ValueFlow::Value * getInvalidValue(const Token *ftok, nonneg int argnr, const Settings *settings) const;
 
-    const ValueFlow::Value * getContainerSizeValue(const MathLib::bigint val) const {
+    const ValueFlow::Value* getContainerSizeValue(const MathLib::bigint val) const;
-        if (!mImpl->mValues)
-            return nullptr;
-        const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) {
-            return value.isContainerSizeValue() && !value.isImpossible() && value.intvalue == val;
-        });
-        return it == mImpl->mValues->end() ? nullptr : &*it;
-    }
 
     const Token *getValueTokenMaxStrLength() const;
     const Token *getValueTokenMinStrSize(const Settings *settings) const;

lib/utils.cpp

@@ -0,0 +1,113 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2020 Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "utils.h"
+
+#include <utility>
+#include <stack>
+
+
+int caseInsensitiveStringCompare(const std::string &lhs, const std::string &rhs)
+{
+    if (lhs.size() != rhs.size())
+        return (lhs.size() < rhs.size()) ? -1 : 1;
+    for (unsigned int i = 0; i < lhs.size(); ++i) {
+        const int c1 = std::toupper(lhs[i]);
+        const int c2 = std::toupper(rhs[i]);
+        if (c1 != c2)
+            return (c1 < c2) ? -1 : 1;
+    }
+    return 0;
+}
+
+bool isValidGlobPattern(const std::string& pattern)
+{
+    for (std::string::const_iterator i = pattern.begin(); i != pattern.end(); ++i) {
+        if (*i == '*' || *i == '?') {
+            std::string::const_iterator j = i + 1;
+            if (j != pattern.end() && (*j == '*' || *j == '?')) {
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
+bool matchglob(const std::string& pattern, const std::string& name)
+{
+    const char* p = pattern.c_str();
+    const char* n = name.c_str();
+    std::stack<std::pair<const char*, const char*> > backtrack;
+
+    for (;;) {
+        bool matching = true;
+        while (*p != '\0' && matching) {
+            switch (*p) {
+            case '*':
+                // Step forward until we match the next character after *
+                while (*n != '\0' && *n != p[1]) {
+                    n++;
+                }
+                if (*n != '\0') {
+                    // If this isn't the last possibility, save it for later
+                    backtrack.push(std::make_pair(p, n));
+                }
+                break;
+            case '?':
+                // Any character matches unless we're at the end of the name
+                if (*n != '\0') {
+                    n++;
+                } else {
+                    matching = false;
+                }
+                break;
+            default:
+                // Non-wildcard characters match literally
+                if (*n == *p) {
+                    n++;
+                } else if (*n == '\\' && *p == '/') {
+                    n++;
+                } else if (*n == '/' && *p == '\\') {
+                    n++;
+                } else {
+                    matching = false;
+                }
+                break;
+            }
+            p++;
+        }
+
+        // If we haven't failed matching and we've reached the end of the name, then success
+        if (matching && *n == '\0') {
+            return true;
+        }
+
+        // If there are no other paths to try, then fail
+        if (backtrack.empty()) {
+            return false;
+        }
+
+        // Restore pointers from backtrack stack
+        p = backtrack.top().first;
+        n = backtrack.top().second;
+        backtrack.pop();
+
+        // Advance name pointer by one because the current position didn't work
+        n++;
+    }
+}

lib/utils.h

@@ -21,11 +21,9 @@
 #define utilsH
 //---------------------------------------------------------------------------
 
-#include <algorithm>
 #include <cctype>
 #include <cstddef>
 #include <string>
-#include <stack>
 #include <vector>
 
 inline bool endsWith(const std::string &str, char c)
@@ -98,95 +96,11 @@ inline static const char *getOrdinalText(int i)
     return "th";
 }
 
-inline static int caseInsensitiveStringCompare(const std::string &lhs, const std::string &rhs)
+CPPCHECKLIB int caseInsensitiveStringCompare(const std::string& lhs, const std::string& rhs);
-{
-    if (lhs.size() != rhs.size())
-        return (lhs.size() < rhs.size()) ? -1 : 1;
-    for (unsigned int i = 0; i < lhs.size(); ++i) {
-        const int c1 = std::toupper(lhs[i]);
-        const int c2 = std::toupper(rhs[i]);
-        if (c1 != c2)
-            return (c1 < c2) ? -1 : 1;
-    }
-    return 0;
-}
 
-inline static bool isValidGlobPattern(const std::string& pattern)
+CPPCHECKLIB bool isValidGlobPattern(const std::string& pattern);
-{
-    for (std::string::const_iterator i = pattern.begin(); i != pattern.end(); ++i) {
-        if (*i == '*' || *i == '?') {
-            std::string::const_iterator j = i + 1;
-            if (j != pattern.end() && (*j == '*' || *j == '?')) {
-                return false;
-            }
-        }
-    }
-    return true;
-}
 
-inline static bool matchglob(const std::string& pattern, const std::string& name)
+CPPCHECKLIB bool matchglob(const std::string& pattern, const std::string& name);
-{
-    const char* p = pattern.c_str();
-    const char* n = name.c_str();
-    std::stack<std::pair<const char*, const char*> > backtrack;
-
-    for (;;) {
-        bool matching = true;
-        while (*p != '\0' && matching) {
-            switch (*p) {
-            case '*':
-                // Step forward until we match the next character after *
-                while (*n != '\0' && *n != p[1]) {
-                    n++;
-                }
-                if (*n != '\0') {
-                    // If this isn't the last possibility, save it for later
-                    backtrack.push(std::make_pair(p, n));
-                }
-                break;
-            case '?':
-                // Any character matches unless we're at the end of the name
-                if (*n != '\0') {
-                    n++;
-                } else {
-                    matching = false;
-                }
-                break;
-            default:
-                // Non-wildcard characters match literally
-                if (*n == *p) {
-                    n++;
-                } else if (*n == '\\' && *p == '/') {
-                    n++;
-                } else if (*n == '/' && *p == '\\') {
-                    n++;
-                } else {
-                    matching = false;
-                }
-                break;
-            }
-            p++;
-        }
-
-        // If we haven't failed matching and we've reached the end of the name, then success
-        if (matching && *n == '\0') {
-            return true;
-        }
-
-        // If there are no other paths to try, then fail
-        if (backtrack.empty()) {
-            return false;
-        }
-
-        // Restore pointers from backtrack stack
-        p = backtrack.top().first;
-        n = backtrack.top().second;
-        backtrack.pop();
-
-        // Advance name pointer by one because the current position didn't work
-        n++;
-    }
-}
 
 #define UNUSED(x) (void)(x)
 

lib/valueflow.cpp

@@ -6061,6 +6061,20 @@ std::string ValueFlow::Value::infoString() const
     throw InternalError(nullptr, "Invalid ValueFlow Value type");
 }
 
+const char* ValueFlow::Value::toString(MoveKind moveKind)
+{
+    switch (moveKind) {
+    case MoveKind::NonMovedVariable:
+        return "NonMovedVariable";
+    case MoveKind::MovedVariable:
+        return "MovedVariable";
+    case MoveKind::ForwardedVariable:
+        return "ForwardedVariable";
+    }
+    return "";
+}
+
+
 const ValueFlow::Value *ValueFlow::valueFlowConstantFoldAST(Token *expr, const Settings *settings)
 {
     if (expr && expr->values().empty()) {
@@ -6143,4 +6157,4 @@ std::string ValueFlow::eitherTheConditionIsRedundant(const Token *condition)
         return "Either the switch case '" + expr + "' is redundant";
     }
     return "Either the condition '" + condition->expressionString() + "' is redundant";
-}
+}

lib/valueflow.h

@@ -256,17 +256,7 @@ namespace ValueFlow {
 
         enum class LifetimeScope { Local, Argument } lifetimeScope;
 
-        static const char * toString(MoveKind moveKind) {
+        static const char* toString(MoveKind moveKind);
-            switch (moveKind) {
-            case MoveKind::NonMovedVariable:
-                return "NonMovedVariable";
-            case MoveKind::MovedVariable:
-                return "MovedVariable";
-            case MoveKind::ForwardedVariable:
-                return "ForwardedVariable";
-            }
-            return "";
-        }
 
         /** How known is this value */
         enum class ValueKind {

test/testvalueflow.cpp

@@ -25,6 +25,7 @@
 #include "valueflow.h"
 
 #include <simplecpp.h>
+#include <algorithm>
 #include <cmath>
 #include <list>
 #include <map>