diff --git a/lib/analyzer.h b/lib/analyzer.h index 8d0f5be7d..b519cdde8 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -128,15 +128,22 @@ struct Analyzer { None = 0, Quiet = (1 << 0), Absolute = (1 << 1), + ContainerEmpty = (1 << 2), }; }; + enum class Evaluate { Integral, ContainerEmpty }; + /// Analyze a token virtual Action analyze(const Token* tok, Direction d) const = 0; /// Update the state of the value virtual void update(Token* tok, Action a, Direction d) = 0; /// Try to evaluate the value of a token(most likely a condition) - virtual std::vector evaluate(const Token* tok, const Token* ctx = nullptr) const = 0; + virtual std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const = 0; + std::vector evaluate(const Token* tok, const Token* ctx = nullptr) const + { + return evaluate(Evaluate::Integral, tok, ctx); + } /// Lower any values to possible virtual bool lowerToPossible() = 0; /// Lower any values to inconclusive diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 91122f504..1f200bc85 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -565,7 +565,13 @@ struct ForwardTraversal { Token* conTok = condTok->astOperand2(); if (conTok && updateRecursive(conTok) == Progress::Break) return Break(); - if (updateLoop(end, endBlock, condTok) == Progress::Break) + bool isEmpty = false; + std::vector result = analyzer->evaluate(Analyzer::Evaluate::ContainerEmpty, conTok); + if (result.empty()) + analyzer->assume(conTok, false, Analyzer::Assume::ContainerEmpty); + else + isEmpty = result.front() != 0; + if (!isEmpty && updateLoop(end, endBlock, condTok) == Progress::Break) return Break(); } else { Token* stepTok = getStepTok(tok); diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index bd60e50fb..e5990c5c3 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -74,6 +74,15 @@ bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint* r return false; } +void ProgramMemory::setContainerSizeValue(nonneg int exprid, MathLib::bigint value, bool isEqual) +{ + ValueFlow::Value v(value); + v.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + if (!isEqual) + v.valueKind = ValueFlow::Value::ValueKind::Impossible; + values[exprid] = v; +} + void ProgramMemory::setUnknown(nonneg int exprid) { values[exprid].valueType = ValueFlow::Value::ValueType::UNINIT; @@ -142,6 +151,36 @@ bool conditionIsTrue(const Token *condition, const ProgramMemory &programMemory) return !error && result == 1; } +static const Token* getContainerFromEmpty(const Token* tok) +{ + if (!Token::Match(tok->tokAt(-2), ". %name% (")) + return nullptr; + const Token* containerTok = tok->tokAt(-2)->astOperand1(); + if (!astIsContainer(containerTok)) + return nullptr; + if (containerTok->valueType()->container && + containerTok->valueType()->container->getYield(tok->strAt(-1)) == Library::Container::Yield::EMPTY) + return containerTok; + if (Token::simpleMatch(tok->tokAt(-1), "empty ( )")) + return containerTok; + return nullptr; +} + +static const Token* getContainerFromSize(const Token* tok) +{ + if (!Token::Match(tok->tokAt(-2), ". %name% (")) + return nullptr; + const Token* containerTok = tok->tokAt(-2)->astOperand1(); + if (!astIsContainer(containerTok)) + return nullptr; + if (containerTok->valueType()->container && + containerTok->valueType()->container->getYield(tok->strAt(-1)) == Library::Container::Yield::SIZE) + return containerTok; + if (Token::Match(tok->tokAt(-1), "size|length ( )")) + return containerTok; + return nullptr; +} + void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings* settings, bool then) { if (Token::Match(tok, "==|>=|<=|<|>|!=")) { @@ -169,7 +208,12 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke return; if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true)) return; - pm.setIntValue(vartok->exprId(), then ? truevalue.intvalue : falsevalue.intvalue); + bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then); + if (!impossible) + pm.setIntValue(vartok->exprId(), then ? truevalue.intvalue : falsevalue.intvalue); + const Token* containerTok = getContainerFromSize(vartok); + if (containerTok) + pm.setContainerSizeValue(containerTok->exprId(), then ? truevalue.intvalue : falsevalue.intvalue, !impossible); } else if (Token::simpleMatch(tok, "!")) { programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then); } else if (then && Token::simpleMatch(tok, "&&")) { @@ -184,6 +228,9 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true)) return; pm.setIntValue(tok->exprId(), then); + const Token* containerTok = getContainerFromEmpty(tok); + if (containerTok) + pm.setContainerSizeValue(containerTok->exprId(), 0, then); } } @@ -331,10 +378,13 @@ void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& va replace(pm, tok); } -void ProgramMemoryState::assume(const Token* tok, bool b) +void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty) { ProgramMemory pm = state; - programMemoryParseCondition(pm, tok, nullptr, nullptr, b); + if (isEmpty) + pm.setContainerSizeValue(tok->exprId(), 0, b); + else + programMemoryParseCondition(pm, tok, nullptr, nullptr, b); const Token* origin = tok; const Token* top = tok->astTop(); if (top && Token::Match(top->previous(), "for|while (")) diff --git a/lib/programmemory.h b/lib/programmemory.h index 7ae865967..ba8a748d7 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -21,6 +21,7 @@ struct ProgramMemory { bool getContainerSizeValue(nonneg int exprid, MathLib::bigint* result) const; bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint* result) const; + void setContainerSizeValue(nonneg int exprid, MathLib::bigint value, bool isEqual = true); void setUnknown(nonneg int exprid); @@ -49,7 +50,7 @@ struct ProgramMemoryState { void addState(const Token* tok, const ProgramMemory::Map& vars); - void assume(const Token* tok, bool b); + void assume(const Token* tok, bool b, bool isEmpty = false); void removeModifiedVars(const Token* tok); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 832db5f20..22c494c8e 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1455,7 +1455,7 @@ void SymbolDatabase::createSymbolDatabaseExprIds() continue; if (tok1->exprId() == tok2->exprId()) continue; - if (!isSameExpression(isCPP(), true, tok1, tok2, mSettings->library, true, false)) + if (!isSameExpression(isCPP(), true, tok1, tok2, mSettings->library, false, false)) continue; nonneg int cid = std::min(tok1->exprId(), tok2->exprId()); tok1->exprId(cid); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 988923287..84f7a923a 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2000,7 +2000,7 @@ struct ValueFlowAnalyzer : Analyzer { // Check if its assigned to the same value if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) && astIsIntegral(tok->astParent()->astOperand2(), false)) { - std::vector result = evaluate(tok->astParent()->astOperand2()); + std::vector result = evaluate(Evaluate::Integral, tok->astParent()->astOperand2()); if (!result.empty() && value->equalTo(result.front())) return Action::Idempotent; } @@ -2177,32 +2177,48 @@ struct ValueFlowAnalyzer : Analyzer { return Action::None; } - virtual std::vector evaluate(const Token* tok, const Token* ctx = nullptr) const OVERRIDE { - if (tok->hasKnownIntValue()) - return {static_cast(tok->values().front().intvalue)}; - std::vector result; - ProgramMemory pm = pms.get(tok, ctx, getProgramState()); - if (Token::Match(tok, "&&|%oror%")) { - if (conditionIsTrue(tok, pm)) - result.push_back(1); - if (conditionIsFalse(tok, pm)) - result.push_back(0); - } else { - MathLib::bigint out = 0; - bool error = false; - execute(tok, &pm, &out, &error); - if (!error) - result.push_back(out); - } + virtual std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const OVERRIDE + { + if (e == Evaluate::Integral) { + if (tok->hasKnownIntValue()) + return {static_cast(tok->values().front().intvalue)}; + std::vector result; + ProgramMemory pm = pms.get(tok, ctx, getProgramState()); + if (Token::Match(tok, "&&|%oror%")) { + if (conditionIsTrue(tok, pm)) + result.push_back(1); + if (conditionIsFalse(tok, pm)) + result.push_back(0); + } else { + MathLib::bigint out = 0; + bool error = false; + execute(tok, &pm, &out, &error); + if (!error) + result.push_back(out); + } - return result; + return result; + } else if (e == Evaluate::ContainerEmpty) { + const ValueFlow::Value* value = ValueFlow::findValue(tok->values(), nullptr, [](const ValueFlow::Value& v) { + return v.isKnown() && v.isContainerSizeValue(); + }); + if (value) + return {value->intvalue == 0}; + ProgramMemory pm = pms.get(tok, ctx, getProgramState()); + MathLib::bigint out = 0; + if (pm.getContainerEmptyValue(tok->exprId(), &out)) + return {static_cast(out)}; + return {}; + } else { + return {}; + } } virtual void assume(const Token* tok, bool state, unsigned int flags) OVERRIDE { // Update program state pms.removeModifiedVars(tok); pms.addState(tok, getProgramState()); - pms.assume(tok, state); + pms.assume(tok, state, flags & Assume::ContainerEmpty); bool isCondBlock = false; const Token* parent = tok->astParent(); @@ -2221,8 +2237,13 @@ struct ValueFlowAnalyzer : Analyzer { } if (!(flags & Assume::Quiet)) { - std::string s = state ? "true" : "false"; - addErrorPath(tok, "Assuming condition is " + s); + if (flags & Assume::ContainerEmpty) { + std::string s = state ? "empty" : "not empty"; + addErrorPath(tok, "Assuming container is " + s); + } else { + std::string s = state ? "true" : "false"; + addErrorPath(tok, "Assuming condition is " + s); + } } if (!(flags & Assume::Absolute)) makeConditional(); diff --git a/test/teststl.cpp b/test/teststl.cpp index 74c9f467c..5b27b6764 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -538,6 +538,30 @@ private: " return Name;\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + checkNormal("bool f(bool b) {\n" + " std::vector v;\n" + " if (b)\n" + " v.push_back(0);\n" + " for(auto i:v)\n" + " if (v[i] > 0)\n" + " return true;\n" + " return false;\n" + "}\n"); + ASSERT_EQUALS("test.cpp:6:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + + checkNormal("std::vector range(int n);\n" + "bool f(bool b) {\n" + " std::vector v;\n" + " if (b)\n" + " v.push_back(1);\n" + " assert(range(v.size()).size() == v.size());\n" + " for(auto i:range(v.size()))\n" + " if (v[i] > 0)\n" + " return true;\n" + " return false;\n" + "}\n"); + ASSERT_EQUALS("test.cpp:8:style:Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); } void outOfBoundsIndexExpression() {