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}} + } + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits