Skip to content

Commit

Permalink
Fix #12214 FN constParameterReference / #12216 FP constParameterRefer…
Browse files Browse the repository at this point in the history
…ence (#5691)
  • Loading branch information
chrchr-github authored Nov 25, 2023
1 parent c1f6132 commit 33981fe
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 31 deletions.
4 changes: 4 additions & 0 deletions lib/astutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2290,6 +2290,8 @@ static T* getTokenArgumentFunctionImpl(T* tok, int& argn)
tok = tok->astOperand1();
while (tok && (tok->isUnaryOp("*") || tok->str() == "["))
tok = tok->astOperand1();
if (Token::Match(tok, ". * %name%")) // bailout for pointer to member
return tok->tokAt(2);
while (Token::simpleMatch(tok, "."))
tok = tok->astOperand2();
while (Token::simpleMatch(tok, "::")) {
Expand Down Expand Up @@ -2630,6 +2632,8 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
if (!ftok->function() || !ftok->function()->isConst())
return true;
}
if (Token::Match(tok2->astParent(), ". * %name%")) // bailout
return true;

if (Token::simpleMatch(tok2, "[") && astIsContainer(tok) && vt && vt->container && vt->container->stdAssociativeLike)
return true;
Expand Down
21 changes: 2 additions & 19 deletions lib/checkother.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,8 @@ void CheckOther::checkConstVariable()
continue;
if (var->isVolatile())
continue;
if (var->nameToken()->isExpandedMacro())
continue;
if (isStructuredBindingVariable(var)) // TODO: check all bound variables
continue;
if (isVariableChanged(var, mSettings, mTokenizer->isCPP()))
Expand Down Expand Up @@ -1461,25 +1463,6 @@ void CheckOther::checkConstVariable()
if (usedInAssignment)
continue;
}
// Skip if we ever cast this variable to a pointer/reference to a non-const type
{
bool castToNonConst = false;
for (const Token* tok = var->nameToken(); tok != scope->bodyEnd && tok != nullptr; tok = tok->next()) {
if (tok->isCast()) {
if (!tok->valueType()) {
castToNonConst = true; // safe guess
break;
}
const bool isConst = tok->valueType()->isConst(tok->valueType()->pointer);
if (!isConst) {
castToNonConst = true;
break;
}
}
}
if (castToNonConst)
continue;
}

constVariableError(var, hasFunction ? function : nullptr);
}
Expand Down
9 changes: 3 additions & 6 deletions lib/tokenize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ namespace {
bool mUsed = false;

public:
TypedefSimplifier(Token* typedefToken, int &num) : mTypedefToken(typedefToken) {
explicit TypedefSimplifier(Token* typedefToken) : mTypedefToken(typedefToken) {
Token* start = typedefToken->next();
if (Token::simpleMatch(start, "typename"))
start = start->next();
Expand All @@ -641,7 +641,6 @@ namespace {
mRangeTypeQualifiers = rangeQualifiers;
Token* typeName = rangeBefore.second->previous();
if (typeName->isKeyword()) {
(void)num;
// TODO typeName->insertToken("T:" + std::to_string(num++));
typeName->insertToken(nameToken->str());
}
Expand Down Expand Up @@ -1048,16 +1047,14 @@ void Tokenizer::simplifyTypedef()
std::map<std::string, int> numberOfTypedefs;
for (Token* tok = list.front(); tok; tok = tok->next()) {
if (tok->str() == "typedef") {
int dummy = 0;
TypedefSimplifier ts(tok, dummy);
TypedefSimplifier ts(tok);
if (!ts.fail())
numberOfTypedefs[ts.name()]++;
continue;
}
}

int indentlevel = 0;
int typeNum = 1;
std::map<std::string, TypedefSimplifier> typedefs;
for (Token* tok = list.front(); tok; tok = tok->next()) {
if (!tok->isName()) {
Expand All @@ -1069,7 +1066,7 @@ void Tokenizer::simplifyTypedef()
}

if (indentlevel == 0 && tok->str() == "typedef") {
TypedefSimplifier ts(tok, typeNum);
TypedefSimplifier ts(tok);
if (!ts.fail() && numberOfTypedefs[ts.name()] == 1) {
if (mSettings->severity.isEnabled(Severity::portability) && ts.isInvalidConstFunctionType(typedefs))
reportError(tok->next(), Severity::portability, "invalidConstFunctionType",
Expand Down
4 changes: 2 additions & 2 deletions lib/valueflow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5238,7 +5238,7 @@ static const Scope* getLoopScope(const Token* tok)
}

//
static void valueFlowConditionExpressions(TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings &settings)
static void valueFlowConditionExpressions(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings &settings)
{
for (const Scope * scope : symboldatabase.functionScopes) {
if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) {
Expand Down Expand Up @@ -7482,7 +7482,7 @@ static void valueFlowInjectParameter(const TokenList& tokenlist,
settings);
}

static void valueFlowSwitchVariable(TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
static void valueFlowSwitchVariable(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
{
for (const Scope &scope : symboldatabase.scopeList) {
if (scope.type != Scope::ScopeType::eSwitch)
Expand Down
2 changes: 1 addition & 1 deletion test/cfg/std.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4206,7 +4206,7 @@ void uninivar_istream_read(std::istream &f)
f.read(buffer, size);
}

void uninitvar_string_compare(std::string &teststr, std::wstring &testwstr)
void uninitvar_string_compare(const std::string &teststr, const std::wstring &testwstr)
{
const char *pStrUninit;
// cppcheck-suppress uninitvar
Expand Down
2 changes: 1 addition & 1 deletion test/cfg/wxwidgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
#include <wx/textctrl.h>
#include <wx/propgrid/property.h>

void uninitvar_wxRegEx_GetMatch(wxRegEx &obj, size_t *start, size_t *len, size_t index)
void uninitvar_wxRegEx_GetMatch(const wxRegEx &obj, size_t *start, size_t *len, size_t index)
{
size_t s,l;
size_t *sPtr,*lPtr;
Expand Down
18 changes: 16 additions & 2 deletions test/testother.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2873,7 +2873,7 @@ class TestOther : public TestFixture {
" x.f();\n"
" foo( static_cast<U2>(0) );\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());

check("class a {\n"
" void foo(const int& i) const;\n"
Expand Down Expand Up @@ -3375,6 +3375,18 @@ class TestOther : public TestFixture {
"[test.cpp:1]: (style) Parameter 's2' can be declared as reference to const\n",
errout.str());

check("void f(int& r) {\n" // #12214
" (void)(true);\n"
" if (r) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'r' can be declared as reference to const\n", errout.str());

check("struct S { void f(int&); };\n" // #12216
"void g(S& s, int& r, void (S::* p2m)(int&)) {\n"
" (s.*p2m)(r);\n"
"}\n");
ASSERT_EQUALS("", errout.str());

check("struct S {\n"
" void f(int& r) { p = &r; }\n"
" int* p;\n"
Expand Down Expand Up @@ -10861,7 +10873,9 @@ class TestOther : public TestFixture {
" for (auto &j : g(std::move(l))) { (void)j; }\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (warning) Access of moved variable 'l'.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (style) Variable 'j' can be declared as reference to const\n"
"[test.cpp:4]: (warning) Access of moved variable 'l'.\n",
errout.str());
}

void moveCallback()
Expand Down

0 comments on commit 33981fe

Please sign in to comment.