Skip to content

Commit

Permalink
Fix #12203 false negative: constParameterReference when taking address (
Browse files Browse the repository at this point in the history 
danmar#5682)
  • Loading branch information
@chrchr-github
chrchr-github authored Nov 22, 2023
1 parent 727d086 commit 83b5cb5
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 36 deletions.
20 changes: 1 addition & 19 deletions lib/checkother.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1429,25 +1429,7 @@ void CheckOther::checkConstVariable()
}
}
if (tok->isUnaryOp("&") && Token::Match(tok, "& %varid%", var->declarationId())) {
const Token* opTok = tok->astParent();
int argn = -1;
if (opTok && opTok->isUnaryOp("!"))
continue;
if (opTok && (opTok->isComparisonOp() || opTok->isAssignmentOp() || opTok->isCalculation())) {
if (opTok->isComparisonOp() || opTok->isCalculation()) {
if (opTok->astOperand1() != tok)
opTok = opTok->astOperand1();
else
opTok = opTok->astOperand2();
}
if (opTok && opTok->valueType() && var->valueType() && opTok->valueType()->isConst(var->valueType()->pointer))
continue;
} else if (const Token* ftok = getTokenArgumentFunction(tok, argn)) {
bool inconclusive{};
if (var->valueType() && !isVariableChangedByFunctionCall(ftok, var->valueType()->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive)
continue;
}
usedInAssignment = true;
usedInAssignment = isExpressionChangedAt(tok->next(), tok, 0, false, mSettings, true);
break;
}
if (astIsRangeBasedForDecl(tok) && Token::Match(tok->astParent()->astOperand2(), "%varid%", var->declarationId())) {
Expand Down
4 changes: 2 additions & 2 deletions lib/token.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2109,10 +2109,10 @@ static void mergeAdjacent(std::list<ValueFlow::Value>& values)

static void removeOverlaps(std::list<ValueFlow::Value>& values)
{
for (ValueFlow::Value& x : values) {
for (const ValueFlow::Value& x : values) {
if (x.isNonValue())
continue;
values.remove_if([&](ValueFlow::Value& y) {
values.remove_if([&](const ValueFlow::Value& y) {
if (y.isNonValue())
return false;
if (&x == &y)
Expand Down
27 changes: 12 additions & 15 deletions test/testother.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2794,16 +2794,14 @@ class TestOther : public TestFixture {
"void a(T& x) {\n"
" x.dostuff();\n"
" const U * y = dynamic_cast<const U *>(&x);\n"
" y->mutate();\n" // to avoid warnings that y can be const
"}");
TODO_ASSERT_EQUALS("can be const", errout.str(), ""); //Currently taking the address is treated as a non-const operation when it should depend on what we do with it
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U const * y = dynamic_cast<U const *>(&x);\n"
" y->mutate();\n" // to avoid warnings that y can be const
"}");
TODO_ASSERT_EQUALS("can be const", errout.str(), ""); //Currently taking the address is treated as a non-const operation when it should depend on what we do with it
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
Expand All @@ -2814,17 +2812,9 @@ class TestOther : public TestFixture {
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" const U const * const * const * const y = dynamic_cast<const U const * const * const * const>(&x);\n"
" y->mutate();\n" // to avoid warnings that y can be const
" U const * const * * const y = dynamic_cast<U const * const * * const>(&x);\n"
"}");
TODO_ASSERT_EQUALS("can be const", errout.str(), ""); //Currently taking the address is treated as a non-const operation when it should depend on what we do with it
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" const U const * const * * const y = dynamic_cast<const U const * const * * const>(&x);\n"
" y->mutate();\n" // to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
Expand Down Expand Up @@ -3374,6 +3364,13 @@ class TestOther : public TestFixture {
" return is >> s.x;\n"
"}\n");
ASSERT_EQUALS("", errout.str());

check("bool f(std::string& s1, std::string& s2) {\n" // #12203
" return &s1 == &s2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 's1' can be declared as reference to const\n"
"[test.cpp:1]: (style) Parameter 's2' can be declared as reference to const\n",
errout.str());
}

void constParameterCallback() {
Expand Down Expand Up @@ -3763,7 +3760,7 @@ class TestOther : public TestFixture {
check("void f(int& i) {\n"
" new (&i) int();\n"
"}\n");
ASSERT_EQUALS("", errout.str()); // don't crash
TODO_ASSERT_EQUALS("", "[test.cpp:1]: (style) Parameter 'i' can be declared as reference to const\n", errout.str()); // don't crash

check("void f(int& i) {\n"
" int& r = i;\n"
Expand Down

0 comments on commit 83b5cb5

Please sign in to comment.