Add warning for falling off the end of a function that should return a
value. This is on by default, and controlled by -Wreturn-type (-Wmost
-Wall). I believe there should be very few false positives, though
the most interesting case would be:
int() { bar(); }
when bar does:
bar() { while (1) ; }
Here, we assume functions return, unless they are marked with the
noreturn attribute. I can envision a fixit note for functions that
never return normally that don't have a noreturn attribute to add a
noreturn attribute.
If anyone spots other false positives, let me know!
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@76821 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index d9c1224..3d795f6 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -16,10 +16,12 @@
#include "clang/AST/APValue.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
+#include "clang/Analysis/CFG.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtCXX.h"
+#include "clang/AST/StmtObjc.h"
#include "clang/Parse/DeclSpec.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/SourceManager.h"
@@ -30,6 +32,7 @@
#include "llvm/ADT/STLExtras.h"
#include <algorithm>
#include <functional>
+#include <queue>
using namespace clang;
/// getDeclName - Return a pretty name for the specified decl if possible, or
@@ -1011,6 +1014,114 @@
New->setPreviousDeclaration(Old);
}
+Sema::ControlFlowKind Sema::CheckFallThrough(Stmt *Root) {
+ llvm::OwningPtr<CFG> cfg (CFG::buildCFG(Root, &Context));
+
+ // FIXME: They should never return 0, fix that, delete this code.
+ if (cfg == 0)
+ return NeverFallThrough;
+ // The CFG leaves in dead things, run a simple mark and sweep on it
+ // to weed out the trivially dead things.
+ std::queue<CFGBlock*> workq;
+ llvm::OwningArrayPtr<char> live(new char[cfg->getNumBlockIDs()]);
+ // Prep work queue
+ workq.push(&cfg->getEntry());
+ // Solve
+ while (!workq.empty()) {
+ CFGBlock *item = workq.front();
+ workq.pop();
+ live[item->getBlockID()] = 1;
+ CFGBlock::succ_iterator i;
+ for (i=item->succ_begin(); i != item->succ_end(); ++i) {
+ if ((*i) && ! live[(*i)->getBlockID()]) {
+ live[(*i)->getBlockID()] = 1;
+ workq.push(*i);
+ }
+ }
+ }
+
+ CFGBlock::succ_iterator i;
+ bool HasLiveReturn = false;
+ bool HasFakeEdge = false;
+ bool HasPlainEdge = false;
+ for (i=cfg->getExit().pred_begin(); i != cfg->getExit().pred_end(); ++i) {
+ if (!live[(*i)->getBlockID()])
+ continue;
+ if ((*i)->size() == 0) {
+ // A labeled empty statement, or the entry block...
+ HasPlainEdge = true;
+ continue;
+ }
+ Stmt *S = (**i)[(*i)->size()-1];
+ if (isa<ReturnStmt>(S)) {
+ HasLiveReturn = true;
+ continue;
+ }
+ if (isa<ObjCAtThrowStmt>(S)) {
+ HasFakeEdge = true;
+ continue;
+ }
+ if (isa<CXXThrowExpr>(S)) {
+ HasFakeEdge = true;
+ continue;
+ }
+ bool NoReturnEdge = false;
+ if (CallExpr *C = dyn_cast<CallExpr>(S))
+ {
+ Expr *CEE = C->getCallee()->IgnoreParenCasts();
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(CEE)) {
+ if (FunctionDecl *FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
+ if (FD->hasAttr<NoReturnAttr>()) {
+ NoReturnEdge = true;
+ HasFakeEdge = true;
+ }
+ }
+ }
+ }
+ if (NoReturnEdge == false)
+ HasPlainEdge = true;
+ }
+ if (HasPlainEdge) {
+ if (HasFakeEdge|HasLiveReturn)
+ return MaybeFallThrough;
+ // This says never for calls to functions that are not marked noreturn, that
+ // don't return. For people that don't like this, we encourage marking the
+ // functions as noreturn.
+ return AlwaysFallThrough;
+ }
+ return NeverFallThrough;
+}
+
+/// CheckFallThroughForFunctionDef - Check that we don't fall off the end of a
+/// function that should return a value.
+///
+/// \returns Never iff we can never alter control flow (we always fall off the
+/// end of the statement, Conditional iff we might or might not alter
+/// control-flow and Always iff we always alter control flow (we never fall off
+/// the end of the statement).
+void Sema::CheckFallThroughForFunctionDef(Decl *D, Stmt *Body) {
+ // FIXME: Would be nice if we had a better way to control cascding errors, but
+ // for now, avoid them.
+ if (getDiagnostics().hasErrorOccurred())
+ return;
+ if (Diags.getDiagnosticLevel(diag::warn_maybe_falloff_nonvoid_function)
+ == Diagnostic::Ignored)
+ return;
+ // FIXME: Funtion try block
+ if (CompoundStmt *Compound = dyn_cast<CompoundStmt>(Body)) {
+ switch (CheckFallThrough(Body)) {
+ case MaybeFallThrough:
+ Diag(Compound->getRBracLoc(), diag::warn_maybe_falloff_nonvoid_function);
+ break;
+ case AlwaysFallThrough:
+ Diag(Compound->getRBracLoc(), diag::warn_falloff_nonvoid_function);
+ break;
+ case NeverFallThrough:
+ break;
+ }
+ }
+}
+
/// CheckParmsForFunctionDef - Check that the parameters of the given
/// function are appropriate for the definition of a function. This
/// takes care of any checks that cannot be performed on the
@@ -3246,6 +3357,10 @@
Stmt *Body = BodyArg.takeAs<Stmt>();
if (FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(dcl)) {
FD->setBody(Body);
+ if (!FD->getResultType()->isVoidType()
+ // C and C++ allow for main to automagically return 0.
+ && !FD->isMain())
+ CheckFallThroughForFunctionDef(FD, Body);
if (!FD->isInvalidDecl())
DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
@@ -3260,6 +3375,8 @@
} else if (ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(dcl)) {
assert(MD == getCurMethodDecl() && "Method parsing confused");
MD->setBody(Body);
+ if (!MD->getResultType()->isVoidType())
+ CheckFallThroughForFunctionDef(MD, Body);
MD->setEndLoc(Body->getLocEnd());
if (!MD->isInvalidDecl())