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

Enforcing CppCoreGuideline C.35 on virtual class destructor (#2572) 

* Enforcing CppCoreGuideline C.35
A base class destructor should be either public and virtual, or protected and non-virtual

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-dtor-virtual

* Protected destructor of ciurtual class can be virtual
This commit is contained in:
Lionel Gimbert 2020-04-27 09:22:42 +02:00 committed by GitHub
parent 20b02bff30
commit ad6be7b122
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/checkclass.cpp
View File

@ -1643,7 +1643,7 @@ void CheckClass::virtualDestructor()
// * derived class has non-empty destructor (only c++03, in c++11 it's UB see paragraph 3 in [expr.delete]) // * derived class has non-empty destructor (only c++03, in c++11 it's UB see paragraph 3 in [expr.delete])
// * base class is deleted // * base class is deleted
// unless inconclusive in which case: // unless inconclusive in which case:
// * base class has virtual members but doesn't have virtual destructor // * A class with any virtual functions should have a destructor that is either public and virtual or protected
const bool printInconclusive = mSettings->inconclusive; const bool printInconclusive = mSettings->inconclusive;
std::list<const Function *> inconclusiveErrors; std::list<const Function *> inconclusiveErrors;
@ -1654,7 +1654,9 @@ void CheckClass::virtualDestructor()
if (scope->definedType->derivedFrom.empty()) { if (scope->definedType->derivedFrom.empty()) {
if (printInconclusive) { if (printInconclusive) {
const Function *destructor = scope->getDestructor(); const Function *destructor = scope->getDestructor();
if (destructor && !destructor->hasVirtualSpecifier()) { if (destructor) {
if(!((destructor->hasVirtualSpecifier() && destructor->access == AccessControl::Public) ||
(destructor->access == AccessControl::Protected))) {
for (const Function &func : scope->functionList) { for (const Function &func : scope->functionList) {
if (func.hasVirtualSpecifier()) { if (func.hasVirtualSpecifier()) {
inconclusiveErrors.push_back(destructor); inconclusiveErrors.push_back(destructor);
@ -1663,6 +1665,7 @@ void CheckClass::virtualDestructor()
} }
} }
} }
}
continue; continue;
} }

9
test/testclass.cpp
View File

@ -2699,6 +2699,15 @@ private:
" delete base;\n" " delete base;\n"
"}\n", true); "}\n", true);
ASSERT_EQUALS("[test.cpp:3]: (error) Class 'Base' which is inherited by class 'Derived' does not have a virtual destructor.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (error) Class 'Base' which is inherited by class 'Derived' does not have a virtual destructor.\n", errout.str());
// class Base destructor is not virtual but protected -> no error
checkVirtualDestructor("class Base {\n"
"public:\n"
" virtual void foo(){}\n"
"protected:\n"
" ~Base(){}\n"
"};\n", true);
ASSERT_EQUALS("", errout.str());
} }
void checkNoMemset(const char code[]) { void checkNoMemset(const char code[]) {