Skip to content

Commit 1cf526d

Browse files
authored
Fix 13046: false negative: knownConditionTrueFalse with this (danmar#7432)
1 parent 04c885d commit 1cf526d

File tree

2 files changed

+48
-15
lines changed

2 files changed

+48
-15
lines changed

lib/valueflow.cpp

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5858,29 +5858,41 @@ static const ValueFlow::Value* getKnownValueFromToken(const Token* tok)
58585858
return std::addressof(*it);
58595859
}
58605860

5861-
static const ValueFlow::Value* getKnownValueFromTokens(const std::vector<const Token*>& toks)
5861+
static std::vector<ValueFlow::Value> getCommonValuesFromTokens(const std::vector<const Token*>& toks)
58625862
{
58635863
if (toks.empty())
5864-
return nullptr;
5865-
const ValueFlow::Value* result = getKnownValueFromToken(toks.front());
5866-
if (!result)
5867-
return nullptr;
5868-
if (!std::all_of(std::next(toks.begin()), toks.end(), [&](const Token* tok) {
5869-
return std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) {
5870-
return v.equalValue(*result) && v.valueKind == result->valueKind;
5864+
return {};
5865+
std::vector<ValueFlow::Value> result;
5866+
std::copy_if(toks.front()->values().begin(),
5867+
toks.front()->values().end(),
5868+
std::back_inserter(result),
5869+
[&](const ValueFlow::Value& v) {
5870+
if (!v.isKnown() && !v.isImpossible())
5871+
return false;
5872+
return (v.isIntValue() || v.isContainerSizeValue() || v.isFloatValue());
5873+
});
5874+
std::for_each(toks.begin() + 1, toks.end(), [&](const Token* tok) {
5875+
auto it = std::remove_if(result.begin(), result.end(), [&](const ValueFlow::Value& v) {
5876+
return std::none_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v2) {
5877+
return v.equalValue(v2) && v.valueKind == v2.valueKind;
5878+
});
58715879
});
5872-
}))
5873-
return nullptr;
5880+
result.erase(it, result.end());
5881+
});
58745882
return result;
58755883
}
58765884

5877-
static void setFunctionReturnValue(const Function* f, Token* tok, ValueFlow::Value v, const Settings& settings)
5885+
static void setFunctionReturnValue(const Function* f,
5886+
Token* tok,
5887+
ValueFlow::Value v,
5888+
const Settings& settings,
5889+
bool known = true)
58785890
{
58795891
if (f->hasVirtualSpecifier()) {
58805892
if (v.isImpossible())
58815893
return;
58825894
v.setPossible();
5883-
} else if (!v.isImpossible()) {
5895+
} else if (!v.isImpossible() && known) {
58845896
v.setKnown();
58855897
}
58865898
v.errorPath.emplace_back(tok, "Calling function '" + f->name() + "' returns " + v.toString());
@@ -5911,11 +5923,17 @@ static void valueFlowFunctionReturn(TokenList& tokenlist, ErrorLogger& errorLogg
59115923
if (returns.empty())
59125924
continue;
59135925

5914-
if (const ValueFlow::Value* v = getKnownValueFromTokens(returns)) {
5915-
setFunctionReturnValue(function, tok, *v, settings);
5916-
continue;
5926+
bool hasKnownValue = false;
5927+
5928+
for (const ValueFlow::Value& v : getCommonValuesFromTokens(returns)) {
5929+
setFunctionReturnValue(function, tok, v, settings, false);
5930+
if (v.isKnown())
5931+
hasKnownValue = true;
59175932
}
59185933

5934+
if (hasKnownValue)
5935+
continue;
5936+
59195937
// Arguments..
59205938
std::vector<const Token*> arguments = getArguments(tok);
59215939

test/testcondition.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4760,6 +4760,21 @@ class TestCondition : public TestFixture {
47604760
"}");
47614761
ASSERT_EQUALS("", errout_str());
47624762

4763+
check("struct S {\n"
4764+
" const S* get2() const {\n"
4765+
" if (mS)\n"
4766+
" return mS;\n"
4767+
" return this;\n"
4768+
" }\n"
4769+
" S* mS = nullptr;\n"
4770+
"};\n"
4771+
"void f2() {\n"
4772+
" const S s;\n"
4773+
" const S* sp2 = s.get2();\n"
4774+
" if (sp2) {}\n"
4775+
"}\n");
4776+
ASSERT_EQUALS("[test.cpp:12]: (style) Condition 'sp2' is always true\n", errout_str());
4777+
47634778
check("struct S {\n"
47644779
" void f(int i);\n"
47654780
" bool g() const { return !m.empty(); }\n"

0 commit comments

Comments
 (0)