walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

fix false positives in constParameter (#2758)

This commit is contained in:
shaneasd 2020-09-04 00:44:44 +08:00 committed by GitHub
parent 68ec7dad41
commit 08ea6435ac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 369 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
lib/checkother.cpp
View File

@ -1391,6 +1391,42 @@ void CheckOther::checkConstVariable()
// Skip if address is taken
if (Token::findmatch(var->nameToken(), "& %varid%", scope->bodyEnd, var->declarationId()))
continue;
// Skip if another non-const variable is initialized with this variable
{
//Is it the right side of an initialization of a non-const reference
bool usedInAssignment = false;
for (const Token* tok = var->nameToken(); tok != scope->bodyEnd && tok != nullptr; tok = tok->next())
{
if (!Token::Match(tok, "& %var% = %varid%", var->declarationId()))
continue;
const Variable* refvar = tok->next()->variable();
if (refvar && !refvar->isConst() && refvar->nameToken() == tok->next()) {
usedInAssignment = true;
break;
}
}
if (usedInAssignment)
continue;
}
// Skip if we ever cast this variable to a pointer/reference to a non-const type
{
bool castToNonConst = [&]
{
for (const Token* tok = var->nameToken(); tok != scope->bodyEnd && tok != nullptr; tok = tok->next())
{
if (tok->isCast())
{
bool isConst = 0 != (tok->valueType()->constness & (1 << tok->valueType()->pointer));
if (!isConst)
return true;
}
}
return false;
}();
if (castToNonConst)
continue;
}
constVariableError(var);
}
}

28
lib/symboldatabase.cpp
View File

@ -5382,9 +5382,11 @@ void SymbolDatabase::setValueType(Token *tok, const ValueType &valuetype)
}
if (parent->isAssignmentOp()) {
if (vt1)
setValueType(parent, *vt1);
else if (mIsCpp && ((Token::Match(parent->tokAt(-3), "%var% ; %var% =") && parent->strAt(-3) == parent->strAt(-1)) ||
if (vt1) {
auto vt = *vt1;
vt.reference = Reference::None;
setValueType(parent, vt);
} else if (mIsCpp && ((Token::Match(parent->tokAt(-3), "%var% ; %var% =") && parent->strAt(-3) == parent->strAt(-1)) ||
Token::Match(parent->tokAt(-1), "%var% ="))) {
Token *var1Tok = parent->strAt(-2) == ";" ? parent->tokAt(-3) : parent->tokAt(-1);
Token *autoTok = nullptr;
@ -5469,6 +5471,7 @@ void SymbolDatabase::setValueType(Token *tok, const ValueType &valuetype)
}
if (parent->str() == "&" && !parent->astOperand2()) {
ValueType vt(valuetype);
vt.reference = Reference::None; //Given int& x; the type of &x is int* not int&*
vt.pointer += 1U;
setValueType(parent, vt);
return;
@ -5705,7 +5708,7 @@ static const Token * parsedecl(const Token *type, ValueType * const valuetype, V
} else
valuetype->type = ValueType::Type::RECORD;
bool par = false;
while (Token::Match(type, "%name%|*|&|::|(") && !Token::Match(type, "typename|template") && type->varId() == 0 &&
while (Token::Match(type, "%name%|*|&|&&|::|(") && !Token::Match(type, "typename|template") && type->varId() == 0 &&
!type->variable() && !type->function()) {
if (type->str() == "(") {
if (Token::Match(type->link(), ") const| {"))
@ -5818,6 +5821,10 @@ static const Token * parsedecl(const Token *type, ValueType * const valuetype, V
return nullptr;
else if (type->str() == "*")
valuetype->pointer++;
else if (type->str() == "&")
valuetype->reference = Reference::LValue;
else if (type->str() == "&&")
valuetype->reference = Reference::RValue;
else if (type->isStandardType())
valuetype->fromLibraryType(type->str(), settings);
else if (Token::Match(type->previous(), "!!:: %name% !!::"))
@ -5835,7 +5842,7 @@ static const Token * parsedecl(const Token *type, ValueType * const valuetype, V
valuetype->sign = ValueType::Sign::SIGNED;
}
return (type && (valuetype->type != ValueType::Type::UNKNOWN_TYPE || valuetype->pointer > 0)) ? type : nullptr;
return (type && (valuetype->type != ValueType::Type::UNKNOWN_TYPE || valuetype->pointer > 0 || valuetype->reference != Reference::None)) ? type : nullptr;
}
static const Scope *getClassScope(const Token *tok)
@ -6351,6 +6358,13 @@ std::string ValueType::dump() const
if (constness > 0)
ret << " valueType-constness=\"" << constness << '\"';
if (reference == Reference::None)
ret << " valueType-reference=\"None\"";
else if (reference == Reference::LValue)
ret << " valueType-reference=\"LValue\"";
else if (reference == Reference::RValue)
ret << " valueType-reference=\"RValue\"";
if (typeScope)
ret << " valueType-typeScope=\"" << typeScope << '\"';
@ -6452,6 +6466,10 @@ std::string ValueType::str() const
if (constness & (2 << p))
ret += " const";
}
if (reference == Reference::LValue)
ret += " &";
else if (reference == Reference::RValue)
ret += " &&";
return ret.empty() ? ret : ret.substr(1);
}

7
lib/symboldatabase.h
View File

@ -1144,6 +1144,12 @@ private:
void findFunctionInBase(const std::string & name, nonneg int args, std::vector<const Function *> & matches) const;
};
enum class Reference
{
None,
LValue,
RValue
};
/** Value type */
class CPPCHECKLIB ValueType {
@ -1153,6 +1159,7 @@ public:
nonneg int bits; ///< bitfield bitcount
nonneg int pointer; ///< 0=>not pointer, 1=>*, 2=>**, 3=>***, etc
nonneg int constness; ///< bit 0=data, bit 1=*, bit 2=**
Reference reference = Reference::None;///< Is the outermost indirection of this type a reference or rvalue reference or not? pointer=2, Reference=LValue would be a T**&
const Scope *typeScope; ///< if the type definition is seen this point out the type scope
const ::Type *smartPointerType; ///< Smart pointer type
const Token* smartPointerTypeToken; ///< Smart pointer type token

2
test/testcondition.cpp
View File

@ -2326,7 +2326,7 @@ private:
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("template<class T> void f1(const T &s) { if(s.size() > 42) if(s.empty()) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
ASSERT_EQUALS("", errout.str()); //We don't know the type of T so we don't know the relationship between size() and empty(). e.g. s might be a 50 tonne truck with nothing in it.
check("void f2(const std::wstring &s) { if(s.empty()) if(s.size() > 42) {}} ");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());

181
test/testother.cpp
View File

@ -2052,6 +2052,187 @@ private:
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Parameter 'i' can be declared with 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());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" const T& z = x;\n" //Make sure we find all assignments
" T& y = x\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" const U& y = x\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared with const\n", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U& y = x\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" my<fancy>::type& y = x\n" //we don't know if y is const or not
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" 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());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U& y = static_cast<U&>(x)\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" 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());
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());
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());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U& y = dynamic_cast<U&>(x)\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" 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());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U& y = dynamic_cast<typename U&>(x)\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U* y = dynamic_cast<U*>(&x)\n"
" y->mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"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
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
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
"}");
ASSERT_EQUALS("", errout.str());
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
"}");
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());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" my::fancy<typename type const *> const * * const y = dynamic_cast<my::fancy<typename type const *> const * * const>(&x)\n"
" y->mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" my::fancy<typename type const *> const * const * const y = dynamic_cast<my::fancy<typename type const *> const * const * const>(&x)\n"
" y->mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" 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());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U& y = (U&)(x)\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" 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());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U& y = (typename U&)(x)\n"
" y.mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("struct T : public U { void dostuff() const {}};\n"
"void a(T& x) {\n"
" x.dostuff();\n"
" U* y = (U*)(&x)\n"
" y->mutate();\n" //to avoid warnings that y can be const
"}");
ASSERT_EQUALS("", errout.str());
check("class a {\n"
" void foo(const int& i) const;\n"
" void operator()(int& i) const;\n"

122
test/testsymboldatabase.cpp
View File

@ -160,6 +160,7 @@ private:
TEST_CASE(VariableValueType3);
TEST_CASE(VariableValueType4); // smart pointer type
TEST_CASE(VariableValueType5); // smart pointer type
TEST_CASE(VariableValueTypeReferences);
TEST_CASE(findVariableType1);
TEST_CASE(findVariableType2);
@ -966,6 +967,122 @@ private:
ASSERT(p->valueType()->pointer == 1);
}
void VariableValueTypeReferences()
{
{
GET_SYMBOL_DB("void foo(int x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 0);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::None);
}
{
GET_SYMBOL_DB("void foo(int* x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 1);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::None);
}
{
GET_SYMBOL_DB("void foo(int& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 0);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::LValue);
}
{
GET_SYMBOL_DB("void foo(int&& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 0);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::RValue);
}
{
GET_SYMBOL_DB("void foo(int*& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 1);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::LValue);
}
{
GET_SYMBOL_DB("void foo(int*&& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 1);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::RValue);
}
{
GET_SYMBOL_DB("void foo(int**& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 2);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::LValue);
}
{
GET_SYMBOL_DB("void foo(int**&& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 2);
ASSERT(p->valueType()->constness == 0);
ASSERT(p->valueType()->reference == Reference::RValue);
}
{
GET_SYMBOL_DB("void foo(const int& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 0);
ASSERT(p->valueType()->constness == 1);
ASSERT(p->valueType()->reference == Reference::LValue);
}
{
GET_SYMBOL_DB("void foo(const int&& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 0);
ASSERT(p->valueType()->constness == 1);
ASSERT(p->valueType()->reference == Reference::RValue);
}
{
GET_SYMBOL_DB("void foo(const int*& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 1);
ASSERT(p->valueType()->constness == 1);
ASSERT(p->valueType()->reference == Reference::LValue);
}
{
GET_SYMBOL_DB("void foo(const int*&& x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 1);
ASSERT(p->valueType()->constness == 1);
ASSERT(p->valueType()->reference == Reference::RValue);
}
{
GET_SYMBOL_DB("void foo(int* const & x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 1);
ASSERT(p->valueType()->constness == 2);
ASSERT(p->valueType()->reference == Reference::LValue);
}
{
GET_SYMBOL_DB("void foo(int* const && x) {}\n");
const Variable* const p = db->getVariableFromVarId(1);
ASSERT(p->valueType());
ASSERT(p->valueType()->pointer == 1);
ASSERT(p->valueType()->constness == 2);
ASSERT(p->valueType()->reference == Reference::RValue);
}
}
void findVariableType1() {
GET_SYMBOL_DB("class A {\n"
"public:\n"
@ -6697,6 +6814,9 @@ private:
ASSERT_EQUALS("AB *", typeOf("struct AB {int a; int b;}; struct AB ab; x=&ab;", "&"));
ASSERT_EQUALS("A::BC *", typeOf("namespace A { struct BC { int b; int c; }; }; struct A::BC abc; x=&abc;", "&"));
ASSERT_EQUALS("A::BC *", typeOf("namespace A { struct BC { int b; int c; }; }; struct A::BC *abc; x=abc+1;", "+"));
ASSERT_EQUALS("signed int", typeOf("auto a(int& x, int& y) { return x + y; }", "+"));
ASSERT_EQUALS("signed int", typeOf("void a(int& x, int& y) { x = y; }", "=")); //Debatably this should be a signed int & but we'll stick with the current behavior for now
ASSERT_EQUALS("signed int", typeOf("auto a(int* y) { return *y; }", "*")); //Debatably this should be a signed int & but we'll stick with the current behavior for now
// Unary arithmetic/bit operators
ASSERT_EQUALS("signed int", typeOf("int x; a = -x;", "-"));
@ -6774,7 +6894,7 @@ private:
ASSERT_EQUALS("signed int", typeOf("struct AB { int a; int b; } *ab; x = ab[1].a;", "."));
// Overloaded operators
ASSERT_EQUALS("Fred", typeOf("class Fred { Fred& operator<(int); }; void f() { Fred fred; x=fred<123; }", "<"));
ASSERT_EQUALS("Fred &", typeOf("class Fred { Fred& operator<(int); }; void f() { Fred fred; x=fred<123; }", "<"));
// Static members
ASSERT_EQUALS("signed int", typeOf("struct AB { static int a; }; x = AB::a;", "::"));