Skip to main content
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlena Laskavaia2011-04-24 22:13:33 -0400
committerAlena Laskavaia2011-04-24 22:13:33 -0400
commit2c65df3e22121b1f5764bbb1e9bd0c425163f775 (patch)
tree59cc4e9c905f0934941a03921cad6e8480a99460 /codan/org.eclipse.cdt.codan.checkers
parentb8b9084ce272fddef85a52d50cc26fcebb598223 (diff)
downloadorg.eclipse.cdt-2c65df3e22121b1f5764bbb1e9bd0c425163f775.tar.gz
org.eclipse.cdt-2c65df3e22121b1f5764bbb1e9bd0c425163f775.tar.xz
org.eclipse.cdt-2c65df3e22121b1f5764bbb1e9bd0c425163f775.zip
Bug 342906 - No warning if only one of an if/else pair has a return statement
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/ReturnChecker.java53
1 files changed, 44 insertions, 9 deletions
diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java
index 0a2fb6a844..afe30b1188 100644
--- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java
@@ -21,17 +21,23 @@ import org.eclipse.cdt.codan.core.model.cfg.IControlFlowGraph;
import org.eclipse.cdt.codan.core.model.cfg.IExitNode;
import org.eclipse.cdt.core.dom.ast.ASTVisitor;
import org.eclipse.cdt.core.dom.ast.DOMException;
+import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement;
import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
+import org.eclipse.cdt.core.dom.ast.IASTDoStatement;
import org.eclipse.cdt.core.dom.ast.IASTExpression;
+import org.eclipse.cdt.core.dom.ast.IASTForStatement;
import org.eclipse.cdt.core.dom.ast.IASTFunctionDeclarator;
import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
+import org.eclipse.cdt.core.dom.ast.IASTIfStatement;
import org.eclipse.cdt.core.dom.ast.IASTNamedTypeSpecifier;
import org.eclipse.cdt.core.dom.ast.IASTReturnStatement;
import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier;
import org.eclipse.cdt.core.dom.ast.IASTStatement;
+import org.eclipse.cdt.core.dom.ast.IASTSwitchStatement;
import org.eclipse.cdt.core.dom.ast.IASTTypeId;
+import org.eclipse.cdt.core.dom.ast.IASTWhileStatement;
import org.eclipse.cdt.core.dom.ast.IBasicType;
import org.eclipse.cdt.core.dom.ast.IBinding;
import org.eclipse.cdt.core.dom.ast.IType;
@@ -83,15 +89,18 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
}
public int visit(IASTStatement stmt) {
if (stmt instanceof IASTReturnStatement) {
- hasret = true;
IASTReturnStatement ret = (IASTReturnStatement) stmt;
+ boolean hasValue = ret.getReturnValue() != null;
+ if (hasret==false && hasValue) {
+ hasret=true;
+ }
if (!isVoid(func) && !isConstructorDestructor()) {
if (checkImplicitReturn(RET_NO_VALUE_ID) || isExplicitReturn(func)) {
- if (ret.getReturnValue() == null)
+ if (!hasValue)
reportProblem(RET_NO_VALUE_ID, ret);
}
} else {
- if (ret.getReturnValue() != null) {
+ if (hasValue) {
IType type = ret.getReturnValue().getExpressionType();
if (isVoid(type))
return PROCESS_SKIP;
@@ -130,16 +139,42 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
return; // if it is template get out of here
ReturnStmpVisitor visitor = new ReturnStmpVisitor(func);
func.accept(visitor);
- if (!visitor.hasret) {
- // no return at all
- if (!isVoid(func) && (checkImplicitReturn(RET_NORET_ID) || isExplicitReturn(func))) {
- if (endsWithNoExitNode(func))
- reportProblem(RET_NORET_ID, func.getDeclSpecifier());
+ boolean nonVoid = !isVoid(func);
+ if (nonVoid) {
+ if (!visitor.hasret) {
+ // no return at all
+ if (checkImplicitReturn(RET_NORET_ID) || isExplicitReturn(func)) {
+ if (endsWithNoExitNode(func))
+ reportProblem(RET_NORET_ID, func.getDeclSpecifier());
+ }
+ } else {
+ // there a return but maybe it is only on one branch
+ IASTStatement body = func.getBody();
+ if (body instanceof IASTCompoundStatement) {
+ IASTStatement[] statements = ((IASTCompoundStatement) body).getStatements();
+ if (statements.length > 0) {
+ IASTStatement last = statements[statements.length - 1];
+ // now check if last statement if complex (for optimization reasons, building CFG is expensive)
+ if (isCompoundStatement(last)) {
+ if (endsWithNoExitNode(func))
+ reportProblem(RET_NORET_ID, func.getDeclSpecifier());
+ }
+ }
+ }
}
}
}
/**
+ * @param last
+ * @return
+ */
+ private boolean isCompoundStatement(IASTStatement last) {
+ return last instanceof IASTIfStatement || last instanceof IASTWhileStatement || last instanceof IASTDoStatement
+ || last instanceof IASTForStatement || last instanceof IASTSwitchStatement;
+ }
+
+ /**
* @param if - problem id
* @return true if need to check inside functions with implicit return
*/
@@ -160,7 +195,7 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
IExitNode node = exitNodeIterator.next();
if (((ICfgData) node).getData() == null) {
// if it real exit node such as return, exit or throw data
- // will be an ast node, it is null it is fake node added by the
+ // will be an ast node, if it is null it is a fake node added by the
// graph builder
noexitop = true;
break;

Back to the top