From 19e3e3d0c75f292c7c777cbc096889be0c30eb3a Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 30 Apr 2025 21:55:51 +0200 Subject: [PATCH 1/2] Fix #13816 FN uninitMemberVar with union (regression) --- lib/checkclass.cpp | 16 ++++++++-------- test/testconstructors.cpp | 7 ++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 21a1b43d567..d49db3dc3b2 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -174,13 +174,13 @@ void CheckClass::constructors() // #3196 => bailout if there are nested unions // TODO: handle union variables better - { - const bool bailout = std::any_of(scope->nestedList.cbegin(), scope->nestedList.cend(), [](const Scope* nestedScope) { - return nestedScope->type == ScopeType::eUnion; - }); - if (bailout) - continue; - } + // { + // const bool bailout = std::any_of(scope->nestedList.cbegin(), scope->nestedList.cend(), [](const Scope* nestedScope) { + // return nestedScope->type == ScopeType::eUnion; + // }); + // if (bailout) + // continue; + // } std::vector usageList = createUsageList(scope); @@ -311,7 +311,7 @@ void CheckClass::constructors() if (!precedes(scope->bodyStart, func.tokenDef)) continue; const Scope *varType = var.typeScope(); - if (!varType || varType->type != ScopeType::eUnion) { + { const bool derived = scope != var.scope(); if (func.type == FunctionType::eConstructor && func.nestedIn && (func.nestedIn->numConstructors - func.nestedIn->numCopyOrMoveConstructors) > 1 && diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index f587afb7d62..b788834fd1e 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -1302,7 +1302,7 @@ class TestConstructors : public TestFixture { " {\n" " }\n" "};"); - TODO_ASSERT_EQUALS("[test.cpp:9]: (warning) Member variable 'Fred::U' is not initialized in the constructor.\n", "", errout_str()); + ASSERT_EQUALS("[test.cpp:9]: (warning) Member variable 'Fred::U' is not initialized in the constructor.\n", errout_str()); } @@ -3428,9 +3428,6 @@ class TestConstructors : public TestFixture { } void uninitVarUnion2() { - // If the "data_type" is 0 it means union member "data" is invalid. - // So it's ok to not initialize "data". - // related forum: http://sourceforge.net/apps/phpbb/cppcheck/viewtopic.php?f=3&p=1806 check("union Data { int id; int *ptr; };\n" "\n" "class Fred {\n" @@ -3441,7 +3438,7 @@ class TestConstructors : public TestFixture { " Fred() : data_type(0)\n" " { }\n" "};"); - ASSERT_EQUALS("", errout_str()); + ASSERT_EQUALS("[test.cpp:8]: (warning) Member variable 'Fred::data' is not initialized in the constructor.\n", errout_str()); } void uninitMissingFuncDef() { From 8a140bc646ca86babba02610dd6d5b8aaed759cd Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 30 Apr 2025 21:58:28 +0200 Subject: [PATCH 2/2] Format --- lib/checkclass.cpp | 41 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index d49db3dc3b2..8b432a0b180 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -172,17 +172,6 @@ void CheckClass::constructors() if (!printWarnings) continue; - // #3196 => bailout if there are nested unions - // TODO: handle union variables better - // { - // const bool bailout = std::any_of(scope->nestedList.cbegin(), scope->nestedList.cend(), [](const Scope* nestedScope) { - // return nestedScope->type == ScopeType::eUnion; - // }); - // if (bailout) - // continue; - // } - - std::vector usageList = createUsageList(scope); for (const Function &func : scope->functionList) { @@ -310,22 +299,20 @@ void CheckClass::constructors() // If constructor is not in scope then we maybe using a constructor from a different template specialization if (!precedes(scope->bodyStart, func.tokenDef)) continue; - const Scope *varType = var.typeScope(); - { - const bool derived = scope != var.scope(); - if (func.type == FunctionType::eConstructor && - func.nestedIn && (func.nestedIn->numConstructors - func.nestedIn->numCopyOrMoveConstructors) > 1 && - func.argCount() == 0 && func.functionScope && - func.arg && func.arg->link()->next() == func.functionScope->bodyStart && - func.functionScope->bodyStart->link() == func.functionScope->bodyStart->next()) { - // don't warn about user defined default constructor when there are other constructors - if (printInconclusive) - uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, true); - } else if (missingCopy) - missingMemberCopyError(func.token, func.type, var.scope()->className, var.name()); - else - uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, false); - } + + const bool derived = scope != var.scope(); + if (func.type == FunctionType::eConstructor && + func.nestedIn && (func.nestedIn->numConstructors - func.nestedIn->numCopyOrMoveConstructors) > 1 && + func.argCount() == 0 && func.functionScope && + func.arg && func.arg->link()->next() == func.functionScope->bodyStart && + func.functionScope->bodyStart->link() == func.functionScope->bodyStart->next()) { + // don't warn about user defined default constructor when there are other constructors + if (printInconclusive) + uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, true); + } else if (missingCopy) + missingMemberCopyError(func.token, func.type, var.scope()->className, var.name()); + else + uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, false); } } }