Refactor: Remove variable analyzer (danmar#3339)

pfultz2 · web-flow · commit 59a1c1a9d8a4 · 2021-07-18T07:46:31.000+02:00
diff --git a/Makefile b/Makefile
@@ -545,7 +545,7 @@ $(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.
 $(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/programmemory.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/programmemory.o $(libcppdir)/programmemory.cpp
 
-$(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/library.h lib/mathlib.h lib/reverseanalyzer.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h
+$(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/reverseanalyzer.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/reverseanalyzer.o $(libcppdir)/reverseanalyzer.cpp
 
 $(libcppdir)/settings.o: lib/settings.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/summaries.h lib/suppressions.h lib/timer.h lib/utils.h lib/valueflow.h
diff --git a/cfg/std.cfg b/cfg/std.cfg
@@ -7717,6 +7717,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
   <!-- template< class T >           typename std::remove_reference<T>::type&& move( T&& t ) noexcept; // (since C++11) -->
   <!-- template< class T > constexpr typename std::remove_reference<T>::type&& move( T&& t ) noexcept; // (until C++14) -->
   <function name="std::move">
+    <pure/>
     <noreturn>false</noreturn>
     <use-retval/>
     <arg nr="1">
diff --git a/cmake/compileroptions.cmake b/cmake/compileroptions.cmake
@@ -26,7 +26,6 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang"
     add_compile_options(-Wall)
     add_compile_options(-Wextra)
     add_compile_options(-Wcast-qual)                # Cast for removing type qualifiers
-    add_compile_options(-Wno-deprecated-declarations)
     add_compile_options(-Wfloat-equal)              # Floating values used in equality comparisons
     add_compile_options(-Wmissing-declarations)     # If a global function is defined without a previous declaration
     add_compile_options(-Wmissing-format-attribute) #
diff --git a/lib/analyzer.h b/lib/analyzer.h
@@ -119,6 +119,13 @@ struct Analyzer {
         {}
         Action action;
         Terminate terminate;
+
+        void update(Result rhs)
+        {
+            if (terminate == Terminate::None)
+                terminate = rhs.terminate;
+            action |= rhs.action;
+        }
     };
 
     enum class Direction { Forward, Reverse };
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -347,8 +347,10 @@ static bool hasToken(const Token * startTok, const Token * stopTok, const Token
 template <class T, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*>)>
 static T* previousBeforeAstLeftmostLeafGeneric(T* tok)
 {
+    if (!tok)
+        return nullptr;
     T* leftmostLeaf = tok;
-    while (leftmostLeaf && leftmostLeaf->astOperand1())
+    while (leftmostLeaf->astOperand1())
         leftmostLeaf = leftmostLeaf->astOperand1();
     return leftmostLeaf->previous();
 }
@@ -1392,14 +1394,96 @@ bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * cons
     return false;
 }
 
+static bool functionModifiesArguments(const Function* f)
+{
+    return std::any_of(f->argumentList.begin(), f->argumentList.end(), [](const Variable& var) {
+        if (var.isReference() || var.isPointer())
+            return !var.isConst();
+        return true;
+    });
+}
+
+bool isConstFunctionCall(const Token* ftok, const Library& library)
+{
+    if (!Token::Match(ftok, "%name% ("))
+        return false;
+    if (const Function* f = ftok->function()) {
+        if (f->isAttributePure() || f->isAttributeConst())
+            return true;
+        if (Function::returnsVoid(f))
+            return false;
+        // Any modified arguments
+        if (functionModifiesArguments(f))
+            return false;
+        // Member function call
+        if (Token::simpleMatch(ftok->previous(), ".")) {
+            if (f->isConst())
+                return true;
+            // Check for const overloaded function that just return the const version
+            if (!Function::returnsConst(f)) {
+                std::vector<const Function*> fs = f->getOverloadedFunctions();
+                if (std::any_of(fs.begin(), fs.end(), [&](const Function* g) {
+                        if (f == g)
+                            return false;
+                        if (f->argumentList.size() != g->argumentList.size())
+                            return false;
+                        if (functionModifiesArguments(g))
+                            return false;
+                        if (g->isConst() && Function::returnsConst(g))
+                            return true;
+                        return false;
+                    }))
+                    return true;
+            }
+            return false;
+        } else if (f->argumentList.empty()) {
+            // TODO: Check for constexpr
+            return false;
+        }
+    } else if (const Library::Function* f = library.getFunction(ftok)) {
+        if (f->ispure)
+            return true;
+        for (auto&& p : f->argumentChecks) {
+            const Library::ArgumentChecks& ac = p.second;
+            if (ac.direction != Library::ArgumentChecks::Direction::DIR_IN)
+                return false;
+        }
+        if (Token::simpleMatch(ftok->previous(), ".")) {
+            if (!f->isconst)
+                return false;
+        } else if (f->argumentChecks.empty()) {
+            return false;
+        }
+    } else {
+        bool memberFunction = Token::Match(ftok->previous(), ". %name% (");
+        bool constMember = !memberFunction;
+        if (Token::Match(ftok->tokAt(-2), "%var% . %name% (")) {
+            const Variable* var = ftok->tokAt(-2)->variable();
+            if (var)
+                constMember = var->isConst();
+        }
+        // TODO: Only check const on lvalues
+        std::vector<const Token*> args = getArguments(ftok);
+        if (memberFunction && args.empty())
+            return false;
+        return constMember && std::all_of(args.begin(), args.end(), [](const Token* tok) {
+                   const Variable* var = tok->variable();
+                   if (var)
+                       return var->isConst();
+                   return false;
+               });
+    }
+    return true;
+}
+
 bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp)
 {
     if (!tok)
         return true;
+    if (tok->variable() && tok->variable()->isVolatile())
+        return false;
     if (tok->isName() && tok->next()->str() == "(") {
-        if (!tok->function() && !Token::Match(tok->previous(), ".|::") && !library.isFunctionConst(tok->str(), pure))
-            return false;
-        else if (tok->function() && !tok->function()->isConst())
+        if (!isConstFunctionCall(tok, library))
             return false;
     }
     if (tok->tokType() == Token::eIncDecOp)
@@ -1880,6 +1964,9 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
     // Member function call
     if (tok->variable() && Token::Match(tok2->astParent(), ". %name%") && isFunctionCall(tok2->astParent()->next()) && tok2->astParent()->astOperand1() == tok2) {
         const Variable * var = tok->variable();
+        // Member function cannot change what `this` points to
+        if (indirect == 0 && astIsPointer(tok))
+            return false;
         bool isConst = var && var->isConst();
         if (!isConst) {
             const ValueType * valueType = var->valueType();
@@ -2092,24 +2179,28 @@ bool isVariablesChanged(const Token* start,
     return false;
 }
 
+bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp)
+{
+    if (Token::Match(tok->previous(), "%name% (")) {
+        if (tok->previous()->function()) {
+            return (!tok->previous()->function()->isConst());
+        } else if (!tok->previous()->isKeyword()) {
+            return true;
+        }
+    }
+    if (isVariableChanged(tok, indirect, settings, cpp))
+        return true;
+    return false;
+}
+
 bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp)
 {
     if (!precedes(start, end))
         return false;
     for (const Token* tok = start; tok != end; tok = tok->next()) {
         if (!exprDependsOnThis(tok))
             continue;
-        if (Token::Match(tok->previous(), "%name% (")) {
-            if (tok->previous()->function()) {
-                if (!tok->previous()->function()->isConst())
-                    return true;
-                else
-                    continue;
-            } else if (!tok->previous()->isKeyword()) {
-                return true;
-            }
-        }
-        if (isVariableChanged(tok, indirect, settings, cpp))
+        if (isThisChanged(tok, indirect, settings, cpp))
             return true;
     }
     return false;
diff --git a/lib/astutils.h b/lib/astutils.h
@@ -171,6 +171,8 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
 
 bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr);
 
+bool isConstFunctionCall(const Token* ftok, const Library& library);
+
 bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp);
 
 bool isWithoutSideEffects(bool cpp, const Token* tok);
@@ -226,6 +228,7 @@ bool isVariablesChanged(const Token* start,
                         const Settings* settings,
                         bool cpp);
 
+bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp);
 bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp);
 
 const Token* findVariableChanged(const Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
@@ -441,15 +441,15 @@ void CheckCondition::duplicateCondition()
         if (scope.type != Scope::eIf)
             continue;
 
-        const Token *cond1 = scope.classDef->next()->astOperand2();
+        const Token* tok2 = scope.classDef->next();
+        if (!tok2)
+            continue;
+        const Token* cond1 = tok2->astOperand2();
         if (!cond1)
             continue;
         if (cond1->hasKnownIntValue())
             continue;
 
-        const Token *tok2 = scope.classDef->next();
-        if (!tok2)
-            continue;
         tok2 = tok2->link();
         if (!Token::simpleMatch(tok2, ") {"))
             continue;
diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp
@@ -3,6 +3,7 @@
 #include "astutils.h"
 #include "errortypes.h"
 #include "forwardanalyzer.h"
+#include "settings.h"
 #include "symboldatabase.h"
 #include "token.h"
 #include "valueptr.h"
@@ -171,7 +172,8 @@ struct ReverseTraversal {
                                                     settings);
                         }
                         // Assignment to
-                    } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownValue()) {
+                    } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownValue() &&
+                               isConstExpression(assignTok->astOperand2(), settings->library, true, true)) {
                         const std::string info = "Assignment to '" + assignTok->expressionString() + "'";
                         ValuePtr<Analyzer> a = analyzer->reanalyze(assignTok->astOperand2(), info);
                         if (a) {
diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp
@@ -3943,6 +3943,29 @@ bool Function::isImplicitlyVirtual(bool defaultVal) const
     return defaultVal; //If we can't see all the bases classes then we can't say conclusively
 }
 
+std::vector<const Function*> Function::getOverloadedFunctions() const
+{
+    std::vector<const Function*> result;
+    const Scope* scope = nestedIn;
+
+    while (scope) {
+        const bool isMemberFunction = scope->isClassOrStruct() && !isStatic();
+        for (std::multimap<std::string, const Function*>::const_iterator it = scope->functionMap.find(tokenDef->str());
+             it != scope->functionMap.end() && it->first == tokenDef->str();
+             ++it) {
+            const Function* func = it->second;
+            if (isMemberFunction == func->isStatic())
+                continue;
+            result.push_back(func);
+        }
+        if (isMemberFunction)
+            break;
+        scope = scope->nestedIn;
+    }
+
+    return result;
+}
+
 const Function *Function::getOverriddenFunction(bool *foundAllBaseClasses) const
 {
     if (foundAllBaseClasses)
diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h
@@ -769,6 +769,8 @@ class CPPCHECKLIB Function {
     /** @brief check if this function is virtual in the base classes */
     bool isImplicitlyVirtual(bool defaultVal = false) const;
 
+    std::vector<const Function*> getOverloadedFunctions() const;
+
     /** @brief get function in base class that is overridden */
     const Function *getOverriddenFunction(bool *foundAllBaseClasses = nullptr) const;
 
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
