mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-08-17 05:05:43 +02:00
Code Issues Wiki Activity

Bug 315528 - [fp] Non-virtual destructor diagnostics doesn't take superclass into account. Patch by Patrick Hofer.

Browse source
This commit is contained in:
Sergey Prigogin 2011-05-28 04:57:43 +00:00
parent 46695ac68b
commit f651a8baa6
2 changed files with 44 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

43
codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/NonVirtualDestructor.java
View file

@ -36,7 +36,7 @@ public class NonVirtualDestructor extends AbstractIndexAstChecker {
public static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.NonVirtualDestructorProblem"; //$NON-NLS-1$ public static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.NonVirtualDestructorProblem"; //$NON-NLS-1$
public void processAst(IASTTranslationUnit ast) { public void processAst(IASTTranslationUnit ast) {
// Traverse the ast using the visitor pattern. // Traverse the AST using the visitor pattern.
ast.accept(new OnEachClass()); ast.accept(new OnEachClass());
} }
@ -102,25 +102,28 @@ public class NonVirtualDestructor extends AbstractIndexAstChecker {
// class has base virtual method and own non-virtual destructor // class has base virtual method and own non-virtual destructor
ICPPMethod[] declaredMethods = classType.getDeclaredMethods(); ICPPMethod[] declaredMethods = classType.getDeclaredMethods();
boolean hasOwnVirtualMethod = false; boolean hasOwnVirtualMethod = false;
boolean hasOwnNonVirDestructor = false; // implicit destructor boolean hasOwnNonVirtualDestructor = false; // implicit destructor
boolean hasDestructor = false; boolean hasDestructor = false;
boolean hasVirtualMethod = false; boolean hasVirtualMethod = false;
for (ICPPMethod method : declaredMethods) { for (ICPPMethod method : declaredMethods) {
if (method.isVirtual() && !method.isDestructor()) { if (method.isPureVirtual()) {
// Class has at least one pure virtual method, it is abstract and cannot be instantiated.
return false;
} else if (method.isVirtual() && !method.isDestructor()) {
hasOwnVirtualMethod = true; hasOwnVirtualMethod = true;
virtualMethod = method; virtualMethod = method;
} }
if (method.isDestructor()) { if (method.isDestructor()) {
hasDestructor = true; hasDestructor = true;
if (!method.isVirtual()) { if (!method.isVirtual()) {
hasOwnNonVirDestructor = true; hasOwnNonVirtualDestructor = true;
destructor = method; destructor = method;
} }
} }
} }
boolean hasVirtualDestructor = false; boolean hasVirtualDestructor = false;
// Class has own virtual method and own non-virtual destructor. // Class has own virtual method and own non-virtual destructor.
if (hasOwnVirtualMethod && hasOwnNonVirDestructor) { if (hasOwnVirtualMethod && hasOwnNonVirtualDestructor) {
if (classType.getFriends().length == 0) { if (classType.getFriends().length == 0) {
if (destructor instanceof ICPPMethod) { if (destructor instanceof ICPPMethod) {
// Check visibility of dtor. No error if it is protected or private. // Check visibility of dtor. No error if it is protected or private.
@ -134,15 +137,13 @@ public class NonVirtualDestructor extends AbstractIndexAstChecker {
// Destructor can be called from a class declared as friend. // Destructor can be called from a class declared as friend.
return true; return true;
} }
// Class does have virtual methods and no destructors, compiler will generate implicit public destructor // Class has virtual methods and implicit public destructor.
if (hasOwnVirtualMethod && !hasDestructor) { if (hasOwnVirtualMethod && !hasDestructor && classType.getBases().length == 0) {
if (classType.getBases().length == 0) { return true;
return true;
}
} }
// Class does not have virtual methods but has virtual destructor // Class does not have virtual methods but has virtual destructor
// - not an error // - not an error
if (!hasOwnVirtualMethod && hasDestructor && !hasOwnNonVirDestructor) { if (!hasOwnVirtualMethod && hasDestructor && !hasOwnNonVirtualDestructor) {
return false; return false;
} }
// Check inherited methods // Check inherited methods
@ -157,9 +158,8 @@ public class NonVirtualDestructor extends AbstractIndexAstChecker {
hasDestructor = true; hasDestructor = true;
if (method.isVirtual()) { if (method.isVirtual()) {
hasVirtualDestructor = true; hasVirtualDestructor = true;
} else { } else if (destructor == null) {
if (destructor == null) destructor = method;
destructor = method;
} }
} }
} }
@ -172,10 +172,9 @@ public class NonVirtualDestructor extends AbstractIndexAstChecker {
} }
return noVirtualDtorInBase; return noVirtualDtorInBase;
} }
} else if (hasVirtualMethod) { } else if (hasVirtualMethod) {
// Class has base virtual method and own non-virtual destructor. // Class has base virtual method and own non-virtual destructor.
if (hasOwnNonVirDestructor) { if (hasOwnNonVirtualDestructor) {
return true; return true;
} }
boolean noVirtualDtorInBase = !hasVirtualDtorInBaseClass(classType); boolean noVirtualDtorInBase = !hasVirtualDtorInBaseClass(classType);
@ -196,6 +195,9 @@ public class NonVirtualDestructor extends AbstractIndexAstChecker {
ICPPClassType testedBaseClass = (ICPPClassType) base.getBaseClass(); ICPPClassType testedBaseClass = (ICPPClassType) base.getBaseClass();
ICPPMethod[] declaredBaseMethods = testedBaseClass.getDeclaredMethods(); ICPPMethod[] declaredBaseMethods = testedBaseClass.getDeclaredMethods();
for (ICPPMethod method : declaredBaseMethods) { for (ICPPMethod method : declaredBaseMethods) {
if (method.isPureVirtual()) {
return false;
}
if (method.isDestructor() && method.isVirtual()) { if (method.isDestructor() && method.isVirtual()) {
return true; return true;
} }
@ -206,15 +208,8 @@ public class NonVirtualDestructor extends AbstractIndexAstChecker {
return false; return false;
} }
/**
* @param decl
* @return
*/
private boolean isClassDecl(IASTDeclSpecifier decl) { private boolean isClassDecl(IASTDeclSpecifier decl) {
if (decl instanceof ICPPASTCompositeTypeSpecifier) { return decl instanceof ICPPASTCompositeTypeSpecifier;
return true;
}
return false;
} }
} }
} }

32
codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/NonVirtualDestructorCheckerTest.java
View file

@ -48,7 +48,7 @@ public class NonVirtualDestructorCheckerTest extends CheckerTestCase {
} }
// struct A { // struct A {
// virtual void f() = 0; // virtual void f() { };
// ~A(); // warn! public non-virtual dtor. // ~A(); // warn! public non-virtual dtor.
// }; // };
public void testPublicVirtualDtorInClass() { public void testPublicVirtualDtorInClass() {
@ -57,7 +57,7 @@ public class NonVirtualDestructorCheckerTest extends CheckerTestCase {
} }
// struct A { // struct A {
// virtual void f() = 0; // virtual void f() { };
// // warn! implicit public non-virtual dtor. // // warn! implicit public non-virtual dtor.
// }; // };
public void testImplicitPublicNonVirtualDtorInClass() { public void testImplicitPublicNonVirtualDtorInClass() {
@ -68,7 +68,7 @@ public class NonVirtualDestructorCheckerTest extends CheckerTestCase {
// struct F { }; // struct F { };
// //
// struct A { // struct A {
// virtual void f() = 0; // virtual void f() { };
// private: // private:
// friend class F; // friend class F;
// ~A(); // warn! can be called from class F. // ~A(); // warn! can be called from class F.
@ -79,7 +79,7 @@ public class NonVirtualDestructorCheckerTest extends CheckerTestCase {
} }
// struct A { // struct A {
// virtual void f() = 0; // virtual void f() { };
// virtual ~A(); // virtual ~A();
// }; // };
// //
@ -92,7 +92,7 @@ public class NonVirtualDestructorCheckerTest extends CheckerTestCase {
} }
// struct A { // struct A {
// virtual void f() = 0; // virtual void f() { };
// virtual ~A(); // ok. // virtual ~A(); // ok.
// }; // };
// //
@ -109,7 +109,7 @@ public class NonVirtualDestructorCheckerTest extends CheckerTestCase {
} }
// struct A { // struct A {
// virtual void f() = 0; // virtual void f() { };
// ~A(); // warn! public non-virtual dtor. // ~A(); // warn! public non-virtual dtor.
// // this affects B, D and E further down in the hierarchy as well // // this affects B, D and E further down in the hierarchy as well
// }; // };
@ -122,7 +122,25 @@ public class NonVirtualDestructorCheckerTest extends CheckerTestCase {
// //
// struct E : public D { // struct E : public D {
// }; // };
public void _testNonVirtualDtorInBaseClass2() { public void testNonVirtualDtorInBaseClass2() {
loadCodeAndRun(getAboveComment());
checkErrorLines(3, 7, 11, 13); checkErrorLines(3, 7, 11, 13);
} }
// class A { // OK. Do _not_ warn here.
// // A is an abstract class because it has one pure virtual method.
// // A cannot be instantiated.
// virtual void f1() { };
// virtual void f2() = 0;
// };
//
// class B : public A {
// virtual void f1() { };
// virtual void f2() { };
// virtual ~B();
// };
public void testAbstractBaseClass() {
loadCodeAndRun(getAboveComment());
checkNoErrors();
}
} }