r325545 - Fix some -Wexceptions false positives.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 19 18:32:30 PST 2018
Author: rsmith
Date: Mon Feb 19 18:32:30 2018
New Revision: 325545
URL: http://llvm.org/viewvc/llvm-project?rev=325545&view=rev
Log:
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.
Modified:
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=325545&r1=325544&r2=325545&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Mon Feb 19 18:32:30 2018
@@ -281,87 +281,62 @@ static void checkRecursiveFunction(Sema
//===----------------------------------------------------------------------===//
// 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;
-}
-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;
+/// 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());
- OpLoc = CE->getThrowLoc();
- for (const auto &I : Block.succs()) {
- if (!I.isReachable())
+ Stack.push_back(&ThrowBlock);
+ Queued[ThrowBlock.getBlockID()] = true;
+
+ 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(
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) {
Modified: cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp?rev=325545&r1=325544&r2=325545&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp Mon Feb 19 18:32:30 2018
@@ -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}}
+ }
+ }
+}
More information about the cfe-commits
mailing list