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 0a2fb6a8448..afe30b1188f 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,15 +139,41 @@ 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; diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java index abf5e920124..27551954fe2 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java @@ -186,4 +186,28 @@ public class ReturnCheckerTest extends CheckerTestCase { loadCodeAndRunCpp(getAboveComment()); checkErrorLine(1); } + +// int f() +// { +// if (g()) +// h(); +// else +// return 0; +// } + public void testBranches_Bug342906() { + loadCodeAndRun(getAboveComment()); + checkErrorLine(1); + } + +// int f() +// { +// switch (g()) { +// case 1: h(); break; +// case 2: +// return 0; +// } + public void testSwitch() { + loadCodeAndRun(getAboveComment()); + checkErrorLine(1); + } } \ No newline at end of file