diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a8b279a52..82fe0911f 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1540,45 +1540,47 @@ void CheckOther::checkConstPointer() if (!mSettings->severity.isEnabled(Severity::style)) return; - std::set pointers; - std::set nonConstPointers; + std::vector pointers, nonConstPointers; for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { - if (!tok->variable()) + const Variable* const var = tok->variable(); + if (!var) continue; - if (!tok->variable()->isLocal() && !tok->variable()->isArgument()) + if (!var->isLocal() && !var->isArgument()) continue; - const Token* const nameTok = tok->variable()->nameToken(); + const Token* const nameTok = var->nameToken(); // declarations of (static) pointers are (not) split up, array declarations are never split up - if (tok == nameTok && (!tok->variable()->isStatic() || Token::simpleMatch(nameTok->next(), "[")) && + if (tok == nameTok && (!var->isStatic() || Token::simpleMatch(nameTok->next(), "[")) && // range-based for loop !(Token::simpleMatch(nameTok->astParent(), ":") && Token::simpleMatch(nameTok->astParent()->astParent(), "("))) continue; - if (!tok->valueType()) + const ValueType* const vt = tok->valueType(); + if (!vt) continue; - if (tok->valueType()->pointer != 1 || (tok->valueType()->constness & 1) || tok->valueType()->reference != Reference::None) + if (vt->pointer != 1 && !(vt->pointer == 2 && var->isArray()) || (vt->constness & 1) || vt->reference != Reference::None) continue; - if (nonConstPointers.find(tok->variable()) != nonConstPointers.end()) + if (std::find(nonConstPointers.begin(), nonConstPointers.end(), var) != nonConstPointers.end()) continue; - pointers.insert(tok->variable()); - const Token *parent = tok->astParent(); + pointers.emplace_back(var); + const Token* const parent = tok->astParent(); bool deref = false; if (parent && parent->isUnaryOp("*")) deref = true; - else if (Token::simpleMatch(parent, "[") && parent->astOperand1() == tok && parent->astOperand1() != nameTok) + else if (Token::simpleMatch(parent, "[") && parent->astOperand1() == tok && tok != nameTok) deref = true; if (deref) { - if (Token::Match(parent->astParent(), "%cop%") && !parent->astParent()->isUnaryOp("&") && !parent->astParent()->isUnaryOp("*")) + const Token* const gparent = parent->astParent(); + if (Token::Match(gparent, "%cop%") && !gparent->isUnaryOp("&") && !gparent->isUnaryOp("*")) continue; - if (Token::simpleMatch(parent->astParent(), "return")) + if (Token::simpleMatch(gparent, "return")) continue; - else if (Token::Match(parent->astParent(), "%assign%") && parent == parent->astParent()->astOperand2()) { + else if (Token::Match(gparent, "%assign%") && parent == gparent->astOperand2()) { bool takingRef = false; - const Token *lhs = parent->astParent()->astOperand1(); + const Token *lhs = gparent->astOperand1(); if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs->variable()->nameToken() == lhs) takingRef = true; if (!takingRef) continue; - } else if (Token::simpleMatch(parent->astParent(), "[") && parent->astParent()->astOperand2() == parent) + } else if (Token::simpleMatch(gparent, "[") && gparent->astOperand2() == parent) continue; } else { if (Token::Match(parent, "%oror%|%comp%|&&|?|!|-")) @@ -1586,14 +1588,14 @@ void CheckOther::checkConstPointer() else if (Token::simpleMatch(parent, "(") && Token::Match(parent->astOperand1(), "if|while")) continue; } - nonConstPointers.insert(tok->variable()); + nonConstPointers.emplace_back(var); } for (const Variable *p: pointers) { if (p->isArgument()) { if (!p->scope() || !p->scope()->function || p->scope()->function->isImplicitlyVirtual(true) || p->scope()->function->hasVirtualSpecifier()) continue; } - if (nonConstPointers.find(p) == nonConstPointers.end()) { + if (std::find(nonConstPointers.begin(), nonConstPointers.end(), p) == nonConstPointers.end()) { const Token *start = (p->isArgument()) ? p->scope()->bodyStart : p->nameToken()->next(); const int indirect = p->isArray() ? p->dimensions().size() : 1; if (isVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, mSettings, mTokenizer->isCPP())) diff --git a/test/testother.cpp b/test/testother.cpp index bf31fcf28..af712faff 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -3059,6 +3059,19 @@ private: "};\n" "S s;\n"); ASSERT_EQUALS("", errout.str()); + + check("void f(int i) {\n" + " const char *tmp;\n" + " char* a[] = { \"a\", \"aa\" };\n" + " static char* b[] = { \"b\", \"bb\" };\n" + " tmp = a[i];\n" + " printf(\"%s\", tmp);\n" + " tmp = b[i];\n" + " printf(\"%s\", tmp);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'a' can be declared with const\n" + "[test.cpp:4]: (style) Variable 'b' can be declared with const\n", + errout.str()); } void switchRedundantAssignmentTest() { @@ -9213,7 +9226,7 @@ private: " int local_argc = 0;\n" " local_argv[local_argc++] = argv[0];\n" "}\n", "test.c"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.c:1]: (style) Parameter 'argv' can be declared with const\n", errout.str()); check("void f() {\n" " int x = 0;\n"