Skip to main content
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSergey Prigogin2011-07-17 20:20:16 -0400
committerSergey Prigogin2011-07-17 22:07:11 -0400
commit21e056c85659a76f88f4a26de0e0704c9ed1756a (patch)
tree10aca629091b87bfcf2b5e5b0354c9d85695c9f1 /codan/org.eclipse.cdt.codan.checkers
parente6b07afaf6e6ef100cbc5d09fa8442a0718c3c96 (diff)
downloadorg.eclipse.cdt-21e056c85659a76f88f4a26de0e0704c9ed1756a.tar.gz
org.eclipse.cdt-21e056c85659a76f88f4a26de0e0704c9ed1756a.tar.xz
org.eclipse.cdt-21e056c85659a76f88f4a26de0e0704c9ed1756a.zip
Bug 315528 - [fp] Non-virtual destructor diagnostics doesn't take
superclass into account. Fix and additional tests including the ones contributed by Tomasz Wesolowski.
Diffstat (limited to 'codan/org.eclipse.cdt.codan.checkers')
-rw-r--r--codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/NonVirtualDestructor.java216
1 files changed, 60 insertions, 156 deletions
diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/NonVirtualDestructor.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/NonVirtualDestructor.java
index 076e2e1878..f4e365cd03 100644
--- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/NonVirtualDestructor.java
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/NonVirtualDestructor.java
@@ -11,10 +11,8 @@
*******************************************************************************/
package org.eclipse.cdt.codan.internal.checkers;
-import org.eclipse.cdt.codan.checkers.CodanCheckersActivator;
import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
import org.eclipse.cdt.core.dom.ast.ASTVisitor;
-import org.eclipse.cdt.core.dom.ast.DOMException;
import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
import org.eclipse.cdt.core.dom.ast.IASTName;
import org.eclipse.cdt.core.dom.ast.IASTNode;
@@ -33,183 +31,89 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPInternalBinding;
* @author Alena Laskavaia
*/
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 PROBLEM_ID = "org.eclipse.cdt.codan.internal.checkers.NonVirtualDestructorProblem"; //$NON-NLS-1$
public void processAst(IASTTranslationUnit ast) {
// Traverse the AST using the visitor pattern.
ast.accept(new OnEachClass());
}
- class OnEachClass extends ASTVisitor {
- private IASTName className;
- private IBinding virtualMethod;
- private IBinding destructor;
- private IASTDeclSpecifier warningLocation;
-
- OnEachClass() {
- shouldVisitDeclSpecifiers = true;
+ private static ICPPMethod getDestructor(ICPPClassType classType) {
+ for (ICPPMethod method : classType.getDeclaredMethods()) {
+ if (method.isDestructor()) {
+ return method;
+ }
}
+ return null;
+ }
- public int visit(IASTDeclSpecifier decl) {
- if (isClassDecl(decl)) {
- try {
- boolean err = hasErrorCondition(decl);
- if (err) {
- String clazz = className.toString();
- String method = virtualMethod.getName();
- IASTNode node = decl;
- if (destructor != null) {
- if (destructor instanceof ICPPInternalBinding) {
- IASTNode[] decls = ((ICPPInternalBinding) destructor).getDeclarations();
- if (warningLocation == null) {
- if (decls != null && decls.length > 0)
- node = decls[0];
- } else {
- node = warningLocation;
- }
- }
- }
- reportProblem(ER_ID, node, clazz, method);
- }
- } catch (DOMException e) {
- // Ignore, not an error.
- } catch (Exception e) {
- CodanCheckersActivator.log(e);
+ private static boolean hasVirtualDestructor(ICPPClassType classType) {
+ ICPPMethod destructor = getDestructor(classType);
+ if (destructor != null && destructor.isVirtual()) {
+ return true;
+ }
+ ICPPBase[] bases = classType.getBases();
+ for (ICPPBase base : bases) {
+ IBinding baseClass = base.getBaseClass();
+ if (baseClass instanceof ICPPClassType) {
+ if (hasVirtualDestructor((ICPPClassType) baseClass)) {
+ return true;
}
- return PROCESS_SKIP;
}
- return PROCESS_CONTINUE;
}
+ return false;
+ }
- /**
- * @param decl
- * @throws DOMException
- */
- private boolean hasErrorCondition(IASTDeclSpecifier decl) throws DOMException {
- ICPPASTCompositeTypeSpecifier spec = (ICPPASTCompositeTypeSpecifier) decl;
- className = spec.getName();
- IBinding binding = className.resolveBinding();
- if ((binding instanceof ICPPClassType) == false) {
- return false;
- }
- ICPPClassType classType = (ICPPClassType) binding;
- virtualMethod = null;
- destructor = null;
- warningLocation = null;
- // Check for the following conditions:
- // class has own virtual method and own non-virtual destructor
- // class has own virtual method and base non-virtual destructor
- // class has base virtual method and own non-virtual destructor
- ICPPMethod[] declaredMethods = classType.getDeclaredMethods();
- boolean hasOwnVirtualMethod = false;
- boolean hasOwnNonVirtualDestructor = false; // implicit destructor
- boolean hasDestructor = false;
- boolean hasVirtualMethod = false;
- for (ICPPMethod method : declaredMethods) {
- 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;
- virtualMethod = method;
+ private class OnEachClass extends ASTVisitor {
+ OnEachClass() {
+ shouldVisitDeclSpecifiers = true;
+ }
+
+ public int visit(IASTDeclSpecifier decl) {
+ if (decl instanceof ICPPASTCompositeTypeSpecifier) {
+ ICPPASTCompositeTypeSpecifier spec = (ICPPASTCompositeTypeSpecifier) decl;
+ IASTName className = spec.getName();
+ IBinding binding = className.resolveBinding();
+ if (!(binding instanceof ICPPClassType)) {
+ return PROCESS_SKIP;
}
- if (method.isDestructor()) {
- hasDestructor = true;
- if (!method.isVirtual()) {
- hasOwnNonVirtualDestructor = true;
- destructor = method;
- }
+ ICPPClassType classType = (ICPPClassType) binding;
+ if (hasVirtualDestructor(classType)) {
+ return PROCESS_SKIP;
}
- }
- boolean hasVirtualDestructor = false;
- // Class has own virtual method and own non-virtual destructor.
- if (hasOwnVirtualMethod && hasOwnNonVirtualDestructor) {
- if (classType.getFriends().length == 0) {
- if (destructor instanceof ICPPMethod) {
- // Check visibility of dtor. No error if it is protected or private.
- if (((ICPPMethod) destructor).getVisibility() != ICPPASTVisibilityLabel.v_public) {
- return false;
- }
- }
- // Check if one of its base classes has a virtual destructor.
- return !hasVirtualDtorInBaseClass(classType);
- }
- // Destructor can be called from a class declared as friend.
- return true;
- }
- // Class has virtual methods and implicit public destructor.
- if (hasOwnVirtualMethod && !hasDestructor && classType.getBases().length == 0) {
- return true;
- }
- // Class does not have virtual methods but has virtual destructor
- // - not an error
- if (!hasOwnVirtualMethod && hasDestructor && !hasOwnNonVirtualDestructor) {
- return false;
- }
- // Check inherited methods
- ICPPMethod[] allDeclaredMethods = classType.getAllDeclaredMethods();
- for (ICPPMethod method : allDeclaredMethods) {
- if (method.isVirtual() && !method.isDestructor()) {
- hasVirtualMethod = true;
- if (virtualMethod == null)
+ ICPPMethod virtualMethod = null;
+ for (ICPPMethod method : classType.getAllDeclaredMethods()) {
+ if (method.isPureVirtual()) {
+ // Class has at least one pure virtual method, it is abstract
+ // and cannot be instantiated.
+ return PROCESS_SKIP;
+ } else if (method.isVirtual() && !method.isDestructor()) {
virtualMethod = method;
- }
- if (method.isDestructor()) {
- hasDestructor = true;
- if (method.isVirtual()) {
- hasVirtualDestructor = true;
- } else if (destructor == null) {
- destructor = method;
}
}
- }
- if (hasOwnVirtualMethod) {
- // Class has own virtual method and base non-virtual destructor.
- if (hasDestructor && !hasVirtualDestructor) {
- boolean noVirtualDtorInBase = !hasVirtualDtorInBaseClass(classType);
- if (noVirtualDtorInBase) {
- warningLocation = decl;
- }
- return noVirtualDtorInBase;
- }
- } else if (hasVirtualMethod) {
- // Class has base virtual method and own non-virtual destructor.
- if (hasOwnNonVirtualDestructor) {
- return true;
+ if (virtualMethod == null) {
+ return PROCESS_SKIP;
}
- boolean noVirtualDtorInBase = !hasVirtualDtorInBaseClass(classType);
- if (noVirtualDtorInBase) {
- warningLocation = decl;
+ ICPPMethod destructor = getDestructor(classType);
+ if (destructor != null &&
+ destructor.getVisibility() != ICPPASTVisibilityLabel.v_public &&
+ classType.getFriends().length == 0) {
+ // No error if the destructor is protected or private and there are no friends.
+ return PROCESS_SKIP;
}
- return noVirtualDtorInBase;
- }
- return false;
- }
- private boolean hasVirtualDtorInBaseClass(ICPPClassType classType) {
- ICPPBase[] bases = classType.getBases();
- for (ICPPBase base : bases) {
- if (!(base.getBaseClass() instanceof ICPPClassType)) {
- continue;
- }
- ICPPClassType testedBaseClass = (ICPPClassType) base.getBaseClass();
- ICPPMethod[] declaredBaseMethods = testedBaseClass.getDeclaredMethods();
- for (ICPPMethod method : declaredBaseMethods) {
- if (method.isPureVirtual()) {
- return false;
- }
- if (method.isDestructor() && method.isVirtual()) {
- return true;
+ IASTNode node = decl;
+ if (destructor instanceof ICPPInternalBinding) {
+ IASTNode[] decls = ((ICPPInternalBinding) destructor).getDeclarations();
+ if (decls != null && decls.length > 0) {
+ node = decls[0];
}
}
- if (hasVirtualDtorInBaseClass(testedBaseClass))
- return true;
+ reportProblem(PROBLEM_ID, node, className.getSimpleID().toString(),
+ virtualMethod.getName());
+ return PROCESS_SKIP;
}
- return false;
- }
-
- private boolean isClassDecl(IASTDeclSpecifier decl) {
- return decl instanceof ICPPASTCompositeTypeSpecifier;
+ return PROCESS_CONTINUE;
}
}
}

Back to the top