diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index a72603c782b..ee27d861c9a 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -116,6 +116,90 @@ CheckClass::CheckClass(const Tokenizer *tokenizer, const Settings *settings, Err mSymbolDatabase(tokenizer?tokenizer->getSymbolDatabase():nullptr) {} +bool CheckClass::isInitialized(const CheckClass::Usage& usage, FunctionType funcType) const +{ + const Variable& var = *usage.var; + + if (usage.assign || usage.init || var.isStatic()) + return true; + + if (!var.nameToken() || var.nameToken()->isAnonymous()) + return true; + + if (var.valueType() && var.valueType()->pointer == 0 && var.type() && var.type()->needInitialization == Type::NeedInitialization::False && var.type()->derivedFrom.empty()) + return true; + + if (var.isConst() && funcType == FunctionType::eOperatorEqual) // We can't set const members in assignment operator + return true; + + // Check if this is a class constructor + if (!var.isPointer() && !var.isPointerArray() && var.isClass() && funcType == FunctionType::eConstructor) { + // Unknown type so assume it is initialized + if (!var.type()) { + if (var.isStlType() && var.valueType() && var.valueType()->containerTypeToken) { + if (var.valueType()->type == ValueType::Type::ITERATOR) + { + // needs initialization + } + else if (var.getTypeName() == "std::array") { + const Token* ctt = var.valueType()->containerTypeToken; + if (!ctt->isStandardType() && + (!ctt->type() || ctt->type()->needInitialization != Type::NeedInitialization::True) && + !mSettings->library.podtype(ctt->str())) // TODO: handle complex type expression + return true; + } + else + return true; + } + else + return true; + } + + // Known type that doesn't need initialization or + // known type that has member variables of an unknown type + else if (var.type()->needInitialization != Type::NeedInitialization::True) + return true; + } + + // Check if type can't be copied + if (!var.isPointer() && !var.isPointerArray() && var.typeScope()) { + if (funcType == FunctionType::eMoveConstructor) { + if (canNotMove(var.typeScope())) + return true; + } + else { + if (canNotCopy(var.typeScope())) + return true; + } + } + return false; +} + +void CheckClass::handleUnionMembers(std::vector& usageList) +{ + // Assign 1 union member => assign all union members + for (const Usage& usage : usageList) { + const Variable& var = *usage.var; + if (!usage.assign && !usage.init) + continue; + const Scope* varScope1 = var.nameToken()->scope(); + while (varScope1->type == ScopeType::eStruct) + varScope1 = varScope1->nestedIn; + if (varScope1->type == ScopeType::eUnion) { + for (Usage& usage2 : usageList) { + const Variable& var2 = *usage2.var; + if (usage2.assign || usage2.init || var2.isStatic()) + continue; + const Scope* varScope2 = var2.nameToken()->scope(); + while (varScope2->type == ScopeType::eStruct) + varScope2 = varScope2->nestedIn; + if (varScope1 == varScope2) + usage2.assign = true; + } + } + } +} + //--------------------------------------------------------------------------- // ClassCheck: Check that all class constructors are ok. //--------------------------------------------------------------------------- @@ -145,6 +229,7 @@ void CheckClass::constructors() }); // There are no constructors. + std::set diagVars; if (scope->numConstructors == 0 && printStyle && !usedInUnion) { // If there is a private variable, there should be a constructor.. int needInit = 0, haveInit = 0; @@ -163,8 +248,10 @@ void CheckClass::constructors() if (haveInit == 0) noConstructorError(scope->classDef, scope->className, scope->classDef->str() == "struct"); else - for (const Variable* uv : uninitVars) + for (const Variable* uv : uninitVars) { uninitVarError(uv->typeStartToken(), uv->scope()->className, uv->name()); + diagVars.emplace(uv); + } } } @@ -201,85 +288,15 @@ void CheckClass::constructors() std::list callstack; initializeVarList(func, callstack, scope, usageList); - // Assign 1 union member => assign all union members - for (const Usage &usage : usageList) { - const Variable& var = *usage.var; - if (!usage.assign && !usage.init) - continue; - const Scope* varScope1 = var.nameToken()->scope(); - while (varScope1->type == ScopeType::eStruct) - varScope1 = varScope1->nestedIn; - if (varScope1->type == ScopeType::eUnion) { - for (Usage &usage2 : usageList) { - const Variable& var2 = *usage2.var; - if (usage2.assign || usage2.init || var2.isStatic()) - continue; - const Scope* varScope2 = var2.nameToken()->scope(); - while (varScope2->type == ScopeType::eStruct) - varScope2 = varScope2->nestedIn; - if (varScope1 == varScope2) - usage2.assign = true; - } - } - } + handleUnionMembers(usageList); // Check if any variables are uninitialized for (const Usage &usage : usageList) { - const Variable& var = *usage.var; - - if (usage.assign || usage.init || var.isStatic()) - continue; - - if (!var.nameToken() || var.nameToken()->isAnonymous()) - continue; - - if (var.valueType() && var.valueType()->pointer == 0 && var.type() && var.type()->needInitialization == Type::NeedInitialization::False && var.type()->derivedFrom.empty()) + if (isInitialized(usage, func.type)) continue; - if (var.isConst() && func.isOperator()) // We can't set const members in assignment operator - continue; - - // Check if this is a class constructor - if (!var.isPointer() && !var.isPointerArray() && var.isClass() && func.type == FunctionType::eConstructor) { - // Unknown type so assume it is initialized - if (!var.type()) { - if (var.isStlType() && var.valueType() && var.valueType()->containerTypeToken) { - if (var.valueType()->type == ValueType::Type::ITERATOR) - { - // needs initialization - } - else if (var.getTypeName() == "std::array") { - const Token* ctt = var.valueType()->containerTypeToken; - if (!ctt->isStandardType() && - (!ctt->type() || ctt->type()->needInitialization != Type::NeedInitialization::True) && - !mSettings->library.podtype(ctt->str())) // TODO: handle complex type expression - continue; - } - else - continue; - } - else - continue; - } - - // Known type that doesn't need initialization or - // known type that has member variables of an unknown type - else if (var.type()->needInitialization != Type::NeedInitialization::True) - continue; - } - - // Check if type can't be copied - if (!var.isPointer() && !var.isPointerArray() && var.typeScope()) { - if (func.type == FunctionType::eMoveConstructor) { - if (canNotMove(var.typeScope())) - continue; - } else { - if (canNotCopy(var.typeScope())) - continue; - } - } - // Is there missing member copy in copy/move constructor or assignment operator? + const Variable& var = *usage.var; bool missingCopy = false; // Don't warn about unknown types in copy constructors since we @@ -326,6 +343,38 @@ void CheckClass::constructors() } } } + + if (scope->numConstructors == 0) { + + // Mark all variables not used + clearAllVar(usageList); + + // Variables with default initializers + bool hasAnyDefaultInit = false; + for (Usage& usage : usageList) { + const Variable& var = *usage.var; + + // check for C++11 initializer + if (var.hasDefault()) { + usage.init = true; + hasAnyDefaultInit = true; + } + } + if (!hasAnyDefaultInit) + continue; + + handleUnionMembers(usageList); + + // Check if any variables are uninitialized + for (const Usage& usage : usageList) { + if (isInitialized(usage, FunctionType::eConstructor)) + continue; + + const Variable& var = *usage.var; + if (diagVars.find(&var) == diagVars.end()) + uninitVarError(scope->bodyStart, false, FunctionType::eConstructor, var.scope()->className, var.name(), false, false); + } + } } } diff --git a/lib/checkclass.h b/lib/checkclass.h index 3b49861126a..0e2bb94eab6 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -254,6 +254,10 @@ class CPPCHECKLIB CheckClass : public Check { bool init{}; }; + static void handleUnionMembers(std::vector& usageList); + + bool isInitialized(const Usage& usage, FunctionType funcType) const; + static bool isBaseClassMutableMemberFunc(const Token *tok, const Scope *scope); /** diff --git a/lib/pathmatch.h b/lib/pathmatch.h index ab9d2a3c43d..825a6fae933 100644 --- a/lib/pathmatch.h +++ b/lib/pathmatch.h @@ -274,9 +274,9 @@ class PathMatch::PathIterator { /* Position struct */ struct Pos { /* String pointer */ - const char *p; + const char *p{}; /* Raw characters left */ - std::size_t l; + std::size_t l{}; /* Buffered character */ int c {EOF}; }; diff --git a/lib/tokenize.h b/lib/tokenize.h index 61d1003f316..af6de50c8eb 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -694,12 +694,12 @@ class CPPCHECKLIB Tokenizer { struct TypedefInfo { std::string name; std::string filename; - int lineNumber; - int column; + int lineNumber{}; + int column{}; int tagLine{-1}; int tagColumn{-1}; - bool used; - bool isFunctionPointer; + bool used{}; + bool isFunctionPointer{}; std::vector typedefInfoTokens; }; std::vector mTypedefInfo; diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index 54af2d0a1c3..3ab1e2c5d1c 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -92,6 +92,7 @@ class TestConstructors : public TestFixture { TEST_CASE(noConstructor13); // #9998 TEST_CASE(noConstructor14); // #10770 TEST_CASE(noConstructor15); // #5499 + TEST_CASE(noConstructor16); TEST_CASE(forwardDeclaration); // ticket #4290/#3190 @@ -758,6 +759,18 @@ class TestConstructors : public TestFixture { ASSERT_EQUALS("[test.cpp:3:5]: (warning) Member variable 'C::i2' is not initialized in the constructor. [uninitMemberVar]\n", errout_str()); } + void noConstructor16() { + check("struct S {\n" // #14546 + " int a = 0, b;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:1:10]: (warning) Member variable 'S::b' is not initialized in the constructor. [uninitMemberVar]\n", errout_str()); + + check("struct S {\n" + " int a, b;\n" + "};\n"); + ASSERT_EQUALS("", errout_str()); + } + // ticket #4290 "False Positive: style (noConstructor): The class 'foo' does not have a constructor." // ticket #3190 "SymbolDatabase: Parse of sub class constructor fails" void forwardDeclaration() {