Fix #11084: ValueType for pointer typedef, don't warn for typedef'd pointers, improve error message (#4123)

* Fix ValueType for pointer typedef

* Add test

* Don't warn for typedef'd pointers, improve error message

* Fix tests

* Add TODO

* Fix test

* Set isSimplifiedTypedef() for more tokens, add test

* Add test
This commit is contained in:
chrchr-github 2022-05-24 10:09:48 +02:00 committed by GitHub
parent c30333425b
commit 2ceaf308de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 110 additions and 46 deletions

View File

@ -1600,6 +1600,8 @@ void CheckOther::checkConstPointer()
const int indirect = p->isArray() ? p->dimensions().size() : 1;
if (isVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, mSettings, mTokenizer->isCPP()))
continue;
if (p->isArgument() && p->typeStartToken() && p->typeStartToken()->isSimplifiedTypedef() && !(Token::simpleMatch(p->typeEndToken(), "*") && !p->typeEndToken()->isSimplifiedTypedef()))
continue;
constVariableError(p, nullptr);
}
}
@ -1615,10 +1617,11 @@ void CheckOther::constVariableError(const Variable *var, const Function *functio
const std::string vartype(var->isArgument() ? "Parameter" : "Variable");
const std::string varname(var->name());
const std::string ptrRefArray = var->isArray() ? "const array" : (var->isPointer() ? "pointer to const" : "reference to const");
ErrorPath errorPath;
std::string id = "const" + vartype;
std::string message = "$symbol:" + varname + "\n" + vartype + " '$symbol' can be declared with const";
std::string message = "$symbol:" + varname + "\n" + vartype + " '$symbol' can be declared as " + ptrRefArray;
errorPath.push_back(ErrorPathItem(var ? var->nameToken() : nullptr, message));
if (var && var->isArgument() && function && function->functionPointerUsage) {
errorPath.push_front(ErrorPathItem(function->functionPointerUsage, "You might need to cast the function pointer here"));

View File

@ -602,6 +602,13 @@ public:
setFlag(fIncompleteVar, b);
}
bool isSimplifiedTypedef() const {
return getFlag(fIsSimplifiedTypedef);
}
void isSimplifiedTypedef(bool b) {
setFlag(fIsSimplifiedTypedef, b);
}
bool isIncompleteConstant() const {
return getFlag(fIsIncompleteConstant);
}
@ -1285,6 +1292,7 @@ private:
fIsRemovedVoidParameter = (1ULL << 35), // A void function parameter has been removed
fIsIncompleteConstant = (1ULL << 36),
fIsRestrict = (1ULL << 37), // Is this a restrict pointer type
fIsSimplifiedTypedef = (1ULL << 38),
};
Token::Type mTokType;

View File

@ -1378,6 +1378,9 @@ void Tokenizer::simplifyTypedef()
typeEnd = typeStart;
// start substituting at the typedef name by replacing it with the type
Token* replStart = tok2; // track first replaced token
for (Token* tok3 = typeStart; tok3->str() != ";"; tok3 = tok3->next())
tok3->isSimplifiedTypedef(true);
tok2->str(typeStart->str());
// restore qualification if it was removed
@ -1386,7 +1389,7 @@ void Tokenizer::simplifyTypedef()
tok2 = tok2->previous();
if (globalScope) {
tok2->insertToken("::");
replStart = tok2->insertToken("::");
tok2 = tok2->next();
}
@ -1415,7 +1418,7 @@ void Tokenizer::simplifyTypedef()
startIdx = spaceIdx + 1;
}
tok2->previous()->insertToken(removed1.substr(startIdx));
tok2->previous()->insertToken("::");
replStart = tok2->previous()->insertToken("::");
break;
}
idx = removed1.rfind(" ::");
@ -1425,12 +1428,21 @@ void Tokenizer::simplifyTypedef()
removed1.resize(idx);
}
}
replStart->isSimplifiedTypedef(true);
Token* constTok = Token::simpleMatch(tok2->previous(), "const") ? tok2->previous() : nullptr;
// add remainder of type
tok2 = TokenList::copyTokens(tok2, typeStart->next(), typeEnd);
if (!pointers.empty()) {
for (const std::string &p : pointers) {
tok2->insertToken(p);
tok2->isSimplifiedTypedef(true);
tok2 = tok2->next();
}
if (constTok) {
constTok->deleteThis();
tok2->insertToken("const");
tok2->isSimplifiedTypedef(true);
tok2 = tok2->next();
}
}

View File

@ -1858,7 +1858,7 @@ private:
check("void f(std::string str) {\n"
" std::string& s2 = str;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 's2' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 's2' can be declared as reference to const\n", errout.str());
check("void f(std::string str) {\n"
" const std::string& s2 = str;\n"
@ -2013,12 +2013,12 @@ private:
" int& i = x[0];\n"
" return i;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'i' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'i' can be declared as reference to const\n", errout.str());
check("int f(std::vector<int>& x) {\n"
" return x[0];\n"
"}");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'x' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
check("int f(std::vector<int> x) {\n"
" const int& i = x[0];\n"
@ -2059,7 +2059,7 @@ private:
check("const int& f(std::vector<int>& x) {\n"
" return x[0];\n"
"}");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'x' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'x' can be declared as reference to const\n", errout.str());
check("int f(std::vector<int>& x) {\n"
" x[0]++;\n"
@ -2176,17 +2176,17 @@ private:
" for(auto x:v)\n"
" x = 1;\n"
"}");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared as reference to const\n", errout.str());
check("void f(std::vector<int>& v) {\n"
" for(auto& x:v) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared as reference to const\n", errout.str());
check("void f(std::vector<int>& v) {\n"
" for(const auto& x:v) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared as reference to const\n", errout.str());
check("void f(int& i) {\n"
" int& j = i;\n"
@ -2290,14 +2290,14 @@ private:
" a x;\n"
" x(i);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Parameter 'i' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (style) Parameter 'i' can be declared as reference to const\n", errout.str());
//cast or assignment to a non-const reference should prevent the warning
check("struct T { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2311,7 +2311,7 @@ private:
" x.dostuff();\n"
" const U& y = x\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2332,7 +2332,7 @@ private:
" const U& y = static_cast<const U&>(x)\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2345,7 +2345,7 @@ private:
" x.dostuff();\n"
" const U& y = dynamic_cast<const U&>(x)\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2353,13 +2353,13 @@ private:
" const U& y = dynamic_cast<U const &>(x)\n"
"}"
);
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
" const U& y = dynamic_cast<U & const>(x)\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2372,7 +2372,7 @@ private:
" x.dostuff();\n"
" const U& y = dynamic_cast<typename const U&>(x)\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2443,7 +2443,7 @@ private:
" x.dostuff();\n"
" const U& y = (const U&)(x)\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2456,7 +2456,7 @@ private:
" x.dostuff();\n"
" const U& y = (typename const U&)(x)\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", 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"
@ -2495,7 +2495,7 @@ private:
"void f(int& i) {\n"
" a()(i);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Parameter 'i' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (style) Parameter 'i' can be declared as reference to const\n", errout.str());
// #9767
check("void fct1(MyClass& object) {\n"
@ -2744,7 +2744,7 @@ private:
" : c(i)\n"
"{\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared with const\n", "", errout.str());
TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared as reference to const\n", "", errout.str());
check("class C\n"
"{\n"
@ -2765,7 +2765,7 @@ private:
" : c(i)\n"
"{\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared with const\n", "", errout.str());
TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared as reference to const\n", "", errout.str());
check("class C\n"
"{\n"
@ -2786,7 +2786,7 @@ private:
" : c(0, i)\n"
"{\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared with const\n", "", errout.str());
TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared as reference to const\n", "", errout.str());
check("void f(std::map<int, std::vector<int>> &map) {\n" // #10266
" for (auto &[slave, panels] : map)\n"
@ -2839,7 +2839,7 @@ private:
void constParameterCallback() {
check("int callback(std::vector<int>& x) { return x[0]; }\n"
"void f() { dostuff(callback); }");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (style) Parameter 'x' can be declared with const. However it seems that 'callback' is a callback function, if 'x' is declared with const you might also need to cast function pointer(s).\n", errout.str());
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (style) Parameter 'x' can be declared as reference to const. However it seems that 'callback' is a callback function, if 'x' is declared with const you might also need to cast function pointer(s).\n", errout.str());
// #9906
check("class EventEngine : public IEventEngine {\n"
@ -2857,51 +2857,51 @@ private:
"void EventEngine::signalEvent(ev::sig& signal, int revents) {\n"
" switch (signal.signum) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:10] -> [test.cpp:13]: (style) Parameter 'signal' can be declared with const. However it seems that 'signalEvent' is a callback function, if 'signal' is declared with const you might also need to cast function pointer(s).\n", errout.str());
ASSERT_EQUALS("[test.cpp:10] -> [test.cpp:13]: (style) Parameter 'signal' can be declared as reference to const. However it seems that 'signalEvent' is a callback function, if 'signal' is declared with const you might also need to cast function pointer(s).\n", errout.str());
}
void constPointer() {
check("void foo(int *p) { return *p; }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { x = *p; }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { int &ref = *p; ref = 12; }");
ASSERT_EQUALS("", errout.str());
check("void foo(int *p) { x = *p + 10; }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { return p[10]; }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { int &ref = p[0]; ref = 12; }");
ASSERT_EQUALS("", errout.str());
check("void foo(int *p) { x[*p] = 12; }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { if (p) {} }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { if (p || x) {} }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { if (p == 0) {} }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { if (!p) {} }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { if (*p > 123) {} }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { return *p + 1; }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(int *p) { return *p > 1; }");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared as pointer to const\n", errout.str());
check("void foo(const int* c) { if (c == 0) {}; }");
ASSERT_EQUALS("", errout.str());
@ -3002,14 +3002,14 @@ private:
" static int i[1] = {};\n"
" return i[0];\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'i' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'i' can be declared as const array\n", errout.str());
check("int f() {\n"
" static int i[] = { 0 };\n"
" int j = i[0] + 1;\n"
" return j;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'i' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'i' can be declared as const array\n", errout.str());
// #10471
check("void f(std::array<int, 1> const& i) {\n"
@ -3032,7 +3032,7 @@ private:
" for (const auto* p : v)\n"
" if (p == nullptr) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared as reference to const\n", errout.str());
check("void f(std::vector<const int*>& v) {\n"
" for (const auto& p : v)\n"
@ -3040,7 +3040,7 @@ private:
" for (const auto* p : v)\n"
" if (p == nullptr) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared as reference to const\n", errout.str());
check("void f(const std::vector<const int*>& v) {\n"
" for (const auto& p : v)\n"
@ -3078,9 +3078,33 @@ private:
" 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",
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'a' can be declared as const array\n"
"[test.cpp:4]: (style) Variable 'b' can be declared as const array\n",
errout.str());
check("typedef void* HWND;\n"
"void f(const HWND h) {\n"
" if (h == nullptr) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("using HWND = void*;\n"
"void f(const HWND h) {\n"
" if (h == nullptr) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("typedef int A;\n"
"void f(A* x) {\n"
" if (x == nullptr) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as pointer to const\n", errout.str());
check("using A = int;\n"
"void f(A* x) {\n"
" if (x == nullptr) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as pointer to const\n", errout.str());
}
void switchRedundantAssignmentTest() {
@ -5254,7 +5278,7 @@ private:
// #5535: Reference named like its type
check("void foo() { UMSConfig& UMSConfig = GetUMSConfiguration(); }");
ASSERT_EQUALS("[test.cpp:1]: (style) Variable 'UMSConfig' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Variable 'UMSConfig' can be declared as reference to const\n", errout.str());
// #3868 - false positive (same expression on both sides of |)
check("void f(int x) {\n"
@ -9280,7 +9304,7 @@ private:
" int local_argc = 0;\n"
" local_argv[local_argc++] = argv[0];\n"
"}\n", "test.c");
ASSERT_EQUALS("[test.c:1]: (style) Parameter 'argv' can be declared with const\n", errout.str());
ASSERT_EQUALS("[test.c:1]: (style) Parameter 'argv' can be declared as const array\n", errout.str());
check("void f() {\n"
" int x = 0;\n"

View File

@ -7744,6 +7744,7 @@ private:
ASSERT_EQUALS("const signed int", typeOf("; const auto x = 3;", "x"));
ASSERT_EQUALS("const signed int", typeOf("; constexpr auto x = 3;", "x"));
ASSERT_EQUALS("const signed int", typeOf("; const constexpr auto x = 3;", "x"));
ASSERT_EQUALS("void * const", typeOf("typedef void* HWND; const HWND x = 0;", "x"));
// Variable declaration
ASSERT_EQUALS("char *", typeOf("; char abc[] = \"abc\";", "["));

View File

@ -151,6 +151,7 @@ private:
TEST_CASE(localvaralias17); // ticket #8911
TEST_CASE(localvaralias18); // ticket #9234 - iterator
TEST_CASE(localvaralias19); // ticket #9828
TEST_CASE(localvaralias20); // ticket #10966
TEST_CASE(localvarasm);
TEST_CASE(localvarstatic);
TEST_CASE(localvarextern);
@ -4674,6 +4675,21 @@ private:
ASSERT_EQUALS("", errout.str());
}
void localvaralias20() { // #10966
functionVariableUsage("struct A {};\n"
"A g();\n"
"void f() {\n"
" const auto& a = g();\n"
" const auto& b = a;\n"
" const auto&& c = g();\n"
" auto&& d = c;\n"
"}\n");
TODO_ASSERT_EQUALS("[test.cpp:5]: (style) Variable 'b' is assigned a value that is never used.\n"
"[test.cpp:7]: (style) Variable 'd' is assigned a value that is never used.\n",
"[test.cpp:7]: (style) Variable 'd' is assigned a value that is never used.\n",
errout.str());
}
void localvarasm() {
functionVariableUsage("void foo(int &b)\n"