Fix some -Wexceptions false positives.

Reimplement the "noexcept function actually throws" warning to properly handle
nested try-blocks. In passing, change 'throw;' handling to treat any enclosing
try block as being sufficient to suppress the warning rather than requiring a
'catch (...)'; the warning is intended to be conservatively-correct.

llvm-svn: 325545

2 files changed, 106 insertions, 73 deletions

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index e67f94c6383..aaed8f98273 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -281,87 +281,62 @@ static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD,
 //===----------------------------------------------------------------------===//
 // Check for throw in a non-throwing function.
 //===----------------------------------------------------------------------===//
-enum ThrowState {
-  FoundNoPathForThrow,
-  FoundPathForThrow,
-  FoundPathWithNoThrowOutFunction,
-};
 
-static bool isThrowCaughtByHandlers(Sema &S,
-                                    const CXXThrowExpr *CE,
-                                    const CXXTryStmt *TryStmt) {
-  for (unsigned H = 0, E = TryStmt->getNumHandlers(); H < E; ++H) {
-    QualType Caught = TryStmt->getHandler(H)->getCaughtType();
-    if (Caught.isNull() || // catch (...) catches everything
-        (CE->getSubExpr() && // throw; is only caught by ...
-         S.handlerCanCatch(Caught, CE->getSubExpr()->getType())))
-      return true;
-  }
-  return false;
-}
+/// Determine whether an exception thrown by E, unwinding from ThrowBlock,
+/// can reach ExitBlock.
+static bool throwEscapes(Sema &S, const CXXThrowExpr *E, CFGBlock &ThrowBlock,
+                         CFG *Body) {
+  SmallVector<CFGBlock *, 16> Stack;
+  llvm::BitVector Queued(Body->getNumBlockIDs());
 
-static bool doesThrowEscapePath(Sema &S, CFGBlock Block,
-                                SourceLocation &OpLoc) {
-  for (const auto &B : Block) {
-    if (B.getKind() != CFGElement::Statement)
-      continue;
-    const auto *CE = dyn_cast<CXXThrowExpr>(B.getAs<CFGStmt>()->getStmt());
-    if (!CE)
-      continue;
+  Stack.push_back(&ThrowBlock);
+  Queued[ThrowBlock.getBlockID()] = true;
 
-    OpLoc = CE->getThrowLoc();
-    for (const auto &I : Block.succs()) {
-      if (!I.isReachable())
+  while (!Stack.empty()) {
+    CFGBlock &UnwindBlock = *Stack.back();
+    Stack.pop_back();
+
+    for (auto &Succ : UnwindBlock.succs()) {
+      if (!Succ.isReachable() || Queued[Succ->getBlockID()])
         continue;
-      if (const auto *Terminator =
-              dyn_cast_or_null<CXXTryStmt>(I->getTerminator()))
-        if (isThrowCaughtByHandlers(S, CE, Terminator))
-          return false;
+
+      if (Succ->getBlockID() == Body->getExit().getBlockID())
+        return true;
+
+      if (auto *Catch =
+              dyn_cast_or_null<CXXCatchStmt>(Succ->getLabel())) {
+        QualType Caught = Catch->getCaughtType();
+        if (Caught.isNull() || // catch (...) catches everything
+            !E->getSubExpr() || // throw; is considered cuaght by any handler
+            S.handlerCanCatch(Caught, E->getSubExpr()->getType()))
+          // Exception doesn't escape via this path.
+          break;
+      } else {
+        Stack.push_back(Succ);
+        Queued[Succ->getBlockID()] = true;
+      }
     }
-    return true;
   }
+
   return false;
 }
 
-static bool hasThrowOutNonThrowingFunc(Sema &S, SourceLocation &OpLoc,
-                                       CFG *BodyCFG) {
-  unsigned ExitID = BodyCFG->getExit().getBlockID();
-
-  SmallVector<ThrowState, 16> States(BodyCFG->getNumBlockIDs(),
-                                     FoundNoPathForThrow);
-  States[BodyCFG->getEntry().getBlockID()] = FoundPathWithNoThrowOutFunction;
-
-  SmallVector<CFGBlock *, 16> Stack;
-  Stack.push_back(&BodyCFG->getEntry());
-  while (!Stack.empty()) {
-    CFGBlock *CurBlock = Stack.pop_back_val();
-
-    unsigned ID = CurBlock->getBlockID();
-    ThrowState CurState = States[ID];
-    if (CurState == FoundPathWithNoThrowOutFunction) {
-      if (ExitID == ID)
+static void visitReachableThrows(
+    CFG *BodyCFG,
+    llvm::function_ref<void(const CXXThrowExpr *, CFGBlock &)> Visit) {
+  llvm::BitVector Reachable(BodyCFG->getNumBlockIDs());
+  clang::reachable_code::ScanReachableFromBlock(&BodyCFG->getEntry(), Reachable);
+  for (CFGBlock *B : *BodyCFG) {
+    if (!Reachable[B->getBlockID()])
+      continue;
+    for (CFGElement &E : *B) {
+      Optional<CFGStmt> S = E.getAs<CFGStmt>();
+      if (!S)
         continue;
-
-      if (doesThrowEscapePath(S, *CurBlock, OpLoc))
-        CurState = FoundPathForThrow;
+      if (auto *Throw = dyn_cast<CXXThrowExpr>(S->getStmt()))
+        Visit(Throw, *B);
     }
-
-    // Loop over successor blocks and add them to the Stack if their state
-    // changes.
-    for (const auto &I : CurBlock->succs())
-      if (I.isReachable()) {
-        unsigned NextID = I->getBlockID();
-        if (NextID == ExitID && CurState == FoundPathForThrow) {
-          States[NextID] = CurState;
-        } else if (States[NextID] < CurState) {
-          States[NextID] = CurState;
-          Stack.push_back(I);
-        }
-      }
   }
-  // Return true if the exit node is reachable, and only reachable through
-  // a throw expression.
-  return States[ExitID] == FoundPathForThrow;
 }
 
 static void EmitDiagForCXXThrowInNonThrowingFunc(Sema &S, SourceLocation OpLoc,
@@ -392,8 +367,10 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD,
   if (BodyCFG->getExit().pred_empty())
     return;
   SourceLocation OpLoc;
-  if (hasThrowOutNonThrowingFunc(S, OpLoc, BodyCFG))
-    EmitDiagForCXXThrowInNonThrowingFunc(S, OpLoc, FD);
+  visitReachableThrows(BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) {
+    if (throwEscapes(S, Throw, Block, BodyCFG))
+      EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD);
+  });
 }
 
 static bool isNoexcept(const FunctionDecl *FD) {
diff --git a/clang/test/SemaCXX/warn-throw-out-noexcept-func.cpp b/clang/test/SemaCXX/warn-throw-out-noexcept-func.cpp
index 4ecc3a94ebd..646cea446c2 100644
--- a/clang/test/SemaCXX/warn-throw-out-noexcept-func.cpp
+++ b/clang/test/SemaCXX/warn-throw-out-noexcept-func.cpp
@@ -248,9 +248,11 @@ void o_ShouldNotDiag() noexcept {
   }
 }
 
-void p_ShouldDiag() noexcept { //expected-note {{function declared non-throwing here}}
+void p_ShouldNotDiag() noexcept {
+  // Don't warn here: it's possible that the user arranges to only call this
+  // when the active exception is of type 'int'.
   try {
-    throw; //expected-warning {{has a non-throwing exception specification but}}
+    throw;
   } catch (int){
   }
 }
@@ -406,3 +408,57 @@ namespace HandlerSpecialCases {
     try { throw nullptr; } catch (int) {} // expected-warning {{still throw}}
   }
 }
+
+namespace NestedTry {
+  void f() noexcept {
+    try {
+      try {
+        throw 0;
+      } catch (float) {}
+    } catch (int) {}
+  }
+
+  struct A { [[noreturn]] ~A(); };
+
+  void g() noexcept { // expected-note {{here}}
+    try {
+      try {
+        throw 0; // expected-warning {{still throw}}
+      } catch (float) {}
+    } catch (const char*) {}
+  }
+
+  void h() noexcept { // expected-note {{here}}
+    try {
+      try {
+        throw 0;
+      } catch (float) {}
+    } catch (int) {
+      throw; // expected-warning {{still throw}}
+    }
+  }
+
+  // FIXME: Ideally, this should still warn; we can track which types are
+  // potentially thrown by the rethrow.
+  void i() noexcept {
+    try {
+      try {
+        throw 0;
+      } catch (int) {
+        throw;
+      }
+    } catch (float) {}
+  }
+
+  // FIXME: Ideally, this should not warn: the second catch block is
+  // unreachable.
+  void j() noexcept { // expected-note {{here}}
+    try {
+      try {
+        throw 0;
+      } catch (int) {}
+    } catch (float) {
+      throw; // expected-warning {{still throw}}
+    }
+  }
+}