Skip to content

Commit 2c03f19

Browse files
chrchr-githubchrchr-github
authored
Fix #12196 FP knownConditionTrueFalse when object is modified through another object / partial fix for #12795 FP knownConditionTrueFalse (#8485)
1 parent 61d83ea commit 2c03f19

3 files changed

Lines changed: 66 additions & 11 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3099,8 +3099,7 @@ static const Token* findExpressionChangedImpl(const Token* expr,
30993099
}
31003100
bool global = false;
31013101
if (tok->variable()) {
3102-
global = !tok->variable()->isLocal() && !tok->variable()->isArgument() &&
3103-
!(tok->variable()->isMember() && !tok->variable()->isStatic());
3102+
global = !tok->variable()->isLocal() && !tok->variable()->isArgument();
31043103
} else if (tok->isIncompleteVar() && !tok->isIncompleteConstant()) {
31053104
global = true;
31063105
}

‎test/testcondition.cpp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5112,6 +5112,37 @@ class TestCondition : public TestFixture {
51125112
" if (i < 0) {}\n"
51135113
"}\n");
51145114
ASSERT_EQUALS("", errout_str());
5115+
5116+
check("struct A { int x; int y; };"
5117+
"void use(int);\n"
5118+
"void test(A a) {\n"
5119+
" int i = a.x;\n"
5120+
" int j = a.x;\n"
5121+
" use(j);\n"
5122+
" if (i == j) {}\n"
5123+
" if (i == a.x) {}\n"
5124+
" if (j == a.x) {}\n"
5125+
"}");
5126+
ASSERT_EQUALS("[test.cpp:6:11]: (style) Condition 'i==j' is always true [knownConditionTrueFalse]\n"
5127+
"[test.cpp:7:11]: (style) Condition 'i==a.x' is always true [knownConditionTrueFalse]\n"
5128+
"[test.cpp:8:11]: (style) Condition 'j==a.x' is always true [knownConditionTrueFalse]\n",
5129+
errout_str());
5130+
5131+
check("struct S { int i; };\n" // #12795
5132+
"struct T {\n"
5133+
" std::map<std::string, S*> m;\n"
5134+
" S* get(const std::string& s) { return m[s]; }\n"
5135+
" void modify() { for (const auto& e : m) e.second->i = 0; }\n"
5136+
"};\n"
5137+
"void f(T& t) {\n"
5138+
" const S* p = t.get(\"abc\");\n"
5139+
" const int o = p->i;\n"
5140+
" t.modify();\n"
5141+
" if (p->i == o) {}\n"
5142+
"}\n");
5143+
TODO_ASSERT_EQUALS("",
5144+
"[test.cpp:11:14]: (style) Condition 'p->i==o' is always true [knownConditionTrueFalse]\n",
5145+
errout_str());
51155146
}
51165147

51175148
void alwaysTrueInfer() {

‎test/testother.cpp‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ class TestOther : public TestFixture {
200200
TEST_CASE(duplicateExpression18);
201201
TEST_CASE(duplicateExpression19);
202202
TEST_CASE(duplicateExpression20);
203+
TEST_CASE(duplicateExpression21);
203204
TEST_CASE(duplicateExpressionLoop);
204205
TEST_CASE(duplicateValueTernary);
205206
TEST_CASE(duplicateValueTernarySizeof); // #13773
@@ -8186,6 +8187,35 @@ class TestOther : public TestFixture {
81868187
ASSERT_EQUALS("", errout_str());
81878188
}
81888189

8190+
void duplicateExpression21() {
8191+
check("struct S { int i; };\n" // #12795
8192+
"struct T {\n"
8193+
" std::map<std::string, S*> m;\n"
8194+
" S* get(const std::string& s) { return m[s]; }\n"
8195+
" void modify() { for (const auto& e : m) e.second->i = 0; }\n"
8196+
"};\n"
8197+
"void f(T& t) {\n"
8198+
" const S* p = t.get(\"abc\");\n"
8199+
" const int o = p->i;\n"
8200+
" t.modify();\n"
8201+
" if (p->i == o) {}\n"
8202+
"}\n");
8203+
ASSERT_EQUALS("", errout_str());
8204+
8205+
check("struct S { int i; };\n"
8206+
" struct T {\n"
8207+
" std::vector<S*> m;\n"
8208+
" void modify() { for (auto e : m) e->i = 0; }\n"
8209+
"};\n"
8210+
"void f(T& t) {\n"
8211+
" const S* p = t.m[0];\n"
8212+
" const int o = p->i;\n"
8213+
" t.modify();\n"
8214+
" if (p->i == o) {}\n"
8215+
"}\n");
8216+
ASSERT_EQUALS("", errout_str());
8217+
}
8218+
81898219
void duplicateExpressionLoop() {
81908220
check("void f() {\n"
81918221
" int a = 1;\n"
@@ -8897,8 +8927,7 @@ class TestOther : public TestFixture {
88978927
" if (i == j) {}\n"
88988928
"}");
88998929
ASSERT_EQUALS(
8900-
"[test.cpp:4:9] -> [test.cpp:3:9]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'. [duplicateAssignExpression]\n"
8901-
"[test.cpp:3:14] -> [test.cpp:4:14] -> [test.cpp:6:11]: (style) The comparison 'i == j' is always true because 'i' and 'j' represent the same value. [knownConditionTrueFalse]\n",
8930+
"[test.cpp:4:9] -> [test.cpp:3:9]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'. [duplicateAssignExpression]\n",
89028931
errout_str());
89038932

89048933
check("struct A { int x; int y; };"
@@ -8910,8 +8939,7 @@ class TestOther : public TestFixture {
89108939
" if (i == a.x) {}\n"
89118940
"}");
89128941
ASSERT_EQUALS(
8913-
"[test.cpp:4:9] -> [test.cpp:3:9]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'. [duplicateAssignExpression]\n"
8914-
"[test.cpp:3:14] -> [test.cpp:6:11]: (style) The comparison 'i == a.x' is always true because 'i' and 'a.x' represent the same value. [knownConditionTrueFalse]\n",
8942+
"[test.cpp:4:9] -> [test.cpp:3:9]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'. [duplicateAssignExpression]\n",
89158943
errout_str());
89168944

89178945
check("struct A { int x; int y; };"
@@ -8923,8 +8951,7 @@ class TestOther : public TestFixture {
89238951
" if (j == a.x) {}\n"
89248952
"}");
89258953
ASSERT_EQUALS(
8926-
"[test.cpp:4:9] -> [test.cpp:3:9]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'. [duplicateAssignExpression]\n"
8927-
"[test.cpp:4:14] -> [test.cpp:6:11]: (style) The comparison 'j == a.x' is always true because 'j' and 'a.x' represent the same value. [knownConditionTrueFalse]\n",
8954+
"[test.cpp:4:9] -> [test.cpp:3:9]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'. [duplicateAssignExpression]\n",
89288955
errout_str());
89298956

89308957
// Issue #8612
@@ -9992,9 +10019,7 @@ class TestOther : public TestFixture {
999210019
" u.g();\n"
999310020
" if (c == m->get()) {}\n"
999410021
"}\n");
9995-
TODO_ASSERT_EQUALS("",
9996-
"[test.cpp:16:33] -> [test.cpp:18:11]: (style) The comparison 'c == m->get()' is always true because 'c' and 'm->get()' represent the same value. [knownConditionTrueFalse]\n",
9997-
errout_str());
10022+
ASSERT_EQUALS("", errout_str());
999810023

999910024
check("struct S {\n" // #12925
1000010025
" const std::string & f() const { return str; }\n"

0 commit comments

Comments
 (0)