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

[281187] checker was doing the opposite - implemented properly

Browse source
This commit is contained in:
Alena Laskavaia 2010-03-26 15:02:42 +00:00
parent 5e8566888d
commit 47c9b5205a
2 changed files with 37 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
codan/org.eclipse.cdt.codan.checkers/plugin.xml
View file

@ -50,8 +50,10 @@
<problem <problem
category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems" category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
defaultSeverity="Warning" defaultSeverity="Warning"
description="Catching by reference is recommended by C++ experts, &quot;Throw by value, catch by reference&quot;. &#x0A; For one thing, this avoids copying and potentially slicing the exception."
id="org.eclipse.cdt.codan.internal.checkers.CatchUsesReference" id="org.eclipse.cdt.codan.internal.checkers.CatchUsesReference"
name="Catch uses reference to exception"> name="Catching by reference is recommended"
messagePattern="Catching by reference is recommended for non-basic types">
</problem> </problem>
</checker> </checker>
<checker <checker

38
codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CatchUsesReference.java
View file

@ -13,18 +13,25 @@ package org.eclipse.cdt.codan.internal.checkers;
import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker; import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.ASTVisitor;
import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
import org.eclipse.cdt.core.dom.ast.IASTDeclaration; import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
import org.eclipse.cdt.core.dom.ast.IASTDeclarator; import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
import org.eclipse.cdt.core.dom.ast.IASTPointerOperator; import org.eclipse.cdt.core.dom.ast.IASTPointerOperator;
import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration;
import org.eclipse.cdt.core.dom.ast.IASTStatement; import org.eclipse.cdt.core.dom.ast.IASTStatement;
import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
import org.eclipse.cdt.core.dom.ast.IBasicType;
import org.eclipse.cdt.core.dom.ast.IBinding;
import org.eclipse.cdt.core.dom.ast.IType;
import org.eclipse.cdt.core.dom.ast.ITypedef;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCatchHandler; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCatchHandler;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamedTypeSpecifier;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTReferenceOperator; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTReferenceOperator;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTryBlockStatement; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTryBlockStatement;
/** /**
* @author Alena * Catching by reference is recommended by C++ experts, for example Herb Sutter/Andrei Alexandresscu "C++ Coding Standards", Rule 73 "Throw by value, catch by reference".
* For one thing, this avoids copying and potentially slicing the exception.
* *
*/ */
public class CatchUsesReference extends AbstractIndexAstChecker { public class CatchUsesReference extends AbstractIndexAstChecker {
@ -39,14 +46,31 @@ public class CatchUsesReference extends AbstractIndexAstChecker {
OnCatch() { OnCatch() {
shouldVisitStatements = true; shouldVisitStatements = true;
} }
public int visit(IASTStatement stmt) { public int visit(IASTStatement stmt) {
if (stmt instanceof ICPPASTTryBlockStatement) { if (stmt instanceof ICPPASTTryBlockStatement) {
ICPPASTTryBlockStatement tblock = (ICPPASTTryBlockStatement) stmt; ICPPASTTryBlockStatement tblock = (ICPPASTTryBlockStatement) stmt;
ICPPASTCatchHandler[] catchHandlers = tblock.getCatchHandlers(); ICPPASTCatchHandler[] catchHandlers = tblock.getCatchHandlers();
for (int i = 0; i < catchHandlers.length; i++) { next: for (int i = 0; i < catchHandlers.length; i++) {
ICPPASTCatchHandler catchHandler = catchHandlers[i]; ICPPASTCatchHandler catchHandler = catchHandlers[i];
if (usesReference(catchHandler)) { IASTDeclaration decl = catchHandler.getDeclaration();
reportProblem(ER_ID, catchHandler.getDeclaration(), "Catch clause uses reference in declaration of exception"); if (decl instanceof IASTSimpleDeclaration) {
IASTSimpleDeclaration sdecl = (IASTSimpleDeclaration) decl;
IASTDeclSpecifier spec = sdecl.getDeclSpecifier();
if (!usesReference(catchHandler)) {
if (spec instanceof ICPPASTNamedTypeSpecifier) {
IBinding typeName = ((ICPPASTNamedTypeSpecifier) spec).getName().getBinding();
// unwind typedef chain
while (typeName instanceof ITypedef) {
IType t = ((ITypedef) typeName).getType();
if (t instanceof IBasicType) continue next;
if (t instanceof IBinding) typeName = (IBinding) t;
else break;
}
reportProblem(ER_ID, decl);
}
}
} }
} }
@ -54,6 +78,7 @@ public class CatchUsesReference extends AbstractIndexAstChecker {
} }
return PROCESS_CONTINUE; return PROCESS_CONTINUE;
} }
/** /**
* @param catchHandler * @param catchHandler
* @return * @return
@ -67,9 +92,7 @@ public class CatchUsesReference extends AbstractIndexAstChecker {
IASTPointerOperator[] pointerOperators = d.getPointerOperators(); IASTPointerOperator[] pointerOperators = d.getPointerOperators();
for (int j = 0; j < pointerOperators.length; j++) { for (int j = 0; j < pointerOperators.length; j++) {
IASTPointerOperator po = pointerOperators[j]; IASTPointerOperator po = pointerOperators[j];
if (po instanceof ICPPASTReferenceOperator) { if (po instanceof ICPPASTReferenceOperator) { return true; }
return true;
}
} }
} }
@ -78,5 +101,4 @@ public class CatchUsesReference extends AbstractIndexAstChecker {
} }
} }
} }