Fix a few things with -Winfinite-recursion. NFC

Now that -Winfinite-recursion no longer uses recursive calls to before path
analysis, several bits of the code can be improved.  The main changes:

1) Early return when finding a path to the exit block without a recursive call
2) Moving the states vector into checkForRecursiveFunctionCall instead of
   passing it in by reference
3) Change checkForRecursiveFunctionCall to return a bool when the warning
   should be emitted.
4) Use the State vector instead of storing it in the Stack vector.

llvm-svn: 245666

1 files changed, 48 insertions, 42 deletions

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 9e69ecc5834..f2430262210 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -163,18 +163,11 @@ public:
 // Check for infinite self-recursion in functions
 //===----------------------------------------------------------------------===//
 
-// All blocks are in one of three states.  States are ordered so that blocks
-// can only move to higher states.
-enum RecursiveState {
-  FoundNoPath,
-  FoundPath,
-  FoundPathWithNoRecursiveCall
-};
-
+// Returns true if the function is called anywhere within the CFGBlock.
+// For member functions, the additional condition of being call from the
+// this pointer is required.
 static bool hasRecursiveCallInPath(const FunctionDecl *FD, CFGBlock &Block) {
-  // Since the current state is FoundPathWithNoRecursiveCall, the successors
-  // will be either FoundPathWithNoRecursiveCall or FoundPath.  To determine
-  // which, process all the Stmt's in this block to find any recursive calls.
+  // Process all the Stmt's in this block to find any calls to FD.
   for (const auto &B : Block) {
     if (B.getKind() != CFGElement::Statement)
       continue;
@@ -203,44 +196,64 @@ static bool hasRecursiveCallInPath(const FunctionDecl *FD, CFGBlock &Block) {
   return false;
 }
 
-static void checkForFunctionCall(Sema &S, const FunctionDecl *FD,
-                                 CFGBlock &Block, unsigned ExitID,
-                                 llvm::SmallVectorImpl<RecursiveState> &States,
-                                 RecursiveState State) {
-  SmallVector<std::pair<CFGBlock *, RecursiveState>, 16> Stack;
-  Stack.emplace_back(&Block, State);
+// All blocks are in one of three states.  States are ordered so that blocks
+// can only move to higher states.
+enum RecursiveState {
+  FoundNoPath,
+  FoundPath,
+  FoundPathWithNoRecursiveCall
+};
+
+// Returns true if there exists a path to the exit block and every path
+// to the exit block passes through a call to FD.
+static bool checkForRecursiveFunctionCall(const FunctionDecl *FD, CFG *cfg) {
 
-  while (!Stack.empty()) {
-    CFGBlock &CurBlock = *Stack.back().first;
-    RecursiveState CurState = Stack.back().second;
-    Stack.pop_back();
+  const unsigned ExitID = cfg->getExit().getBlockID();
 
-    unsigned ID = CurBlock.getBlockID();
+  // Mark all nodes as FoundNoPath, then set the status of the entry block.
+  SmallVector<RecursiveState, 16> States(cfg->getNumBlockIDs(), FoundNoPath);
+  States[cfg->getEntry().getBlockID()] = FoundPathWithNoRecursiveCall;
 
-    // A block's state can only move to a higher state.
-    if (States[ID] >= CurState)
-      continue;
+  // Make the processing stack and seed it with the entry block.
+  SmallVector<CFGBlock *, 16> Stack;
+  Stack.push_back(&cfg->getEntry());
+
+  while (!Stack.empty()) {
+    CFGBlock *CurBlock = Stack.back();
+    Stack.pop_back();
 
-    States[ID] = CurState;
+    unsigned ID = CurBlock->getBlockID();
+    RecursiveState CurState = States[ID];
 
     if (CurState == FoundPathWithNoRecursiveCall) {
       // Found a path to the exit node without a recursive call.
       if (ExitID == ID)
-        continue;
+        return false;
 
-      if (hasRecursiveCallInPath(FD, CurBlock))
+      // Only change state if the block has a recursive call.
+      if (hasRecursiveCallInPath(FD, *CurBlock))
         CurState = FoundPath;
     }
 
-    for (auto I = CurBlock.succ_begin(), E = CurBlock.succ_end(); I != E; ++I)
-      if (*I)
-        Stack.emplace_back(*I, CurState);
+    // Loop over successor blocks and add them to the Stack if their state
+    // changes.
+    for (auto I = CurBlock->succ_begin(), E = CurBlock->succ_end(); I != E; ++I)
+      if (*I) {
+        unsigned next_ID = (*I)->getBlockID();
+        if (States[next_ID] < CurState) {
+          States[next_ID] = CurState;
+          Stack.push_back(*I);
+        }
+      }
   }
+
+  // Return true if the exit node is reachable, and only reachable through
+  // a recursive call.
+  return States[ExitID] == FoundPath;
 }
 
 static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD,
-                                   const Stmt *Body,
-                                   AnalysisDeclContext &AC) {
+                                   const Stmt *Body, AnalysisDeclContext &AC) {
   FD = FD->getCanonicalDecl();
 
   // Only run on non-templated functions and non-templated members of
@@ -256,15 +269,8 @@ static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD,
   if (cfg->getExit().pred_empty())
     return;
 
-  // Mark all nodes as FoundNoPath, then begin processing the entry block.
-  llvm::SmallVector<RecursiveState, 16> states(cfg->getNumBlockIDs(),
-                                               FoundNoPath);
-  checkForFunctionCall(S, FD, cfg->getEntry(), cfg->getExit().getBlockID(),
-                       states, FoundPathWithNoRecursiveCall);
-
-  // Check that the exit block is reachable.  This prevents triggering the
-  // warning on functions that do not terminate.
-  if (states[cfg->getExit().getBlockID()] == FoundPath)
+  // Emit diagnostic if a recursive function call is detected for all paths.
+  if (checkForRecursiveFunctionCall(FD, cfg))
     S.Diag(Body->getLocStart(), diag::warn_infinite_recursive_function);
 }