jyu2 updated this revision to Diff 100796.
jyu2 added a comment.
Okay this CFG version of this change. In this change I am basic using same
algorithm with -Winfinite-recursion.
In addition to my original implementation, I add handler type checking which
basic using https://reviews.llvm.org/D19201 method.
There are couple things I am worry about this implementation:
1> compile time...
2> Correctness...
3> Stack overflow for large CFG...
https://reviews.llvm.org/D33333
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/AnalysisBasedWarnings.cpp
test/CXX/except/except.spec/p11.cpp
test/SemaCXX/warn-throw-out-noexcept-func.cpp
Index: lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -279,6 +279,158 @@
}
//===----------------------------------------------------------------------===//
+// Check for throw in a non-throwing function.
+//===----------------------------------------------------------------------===//
+enum ThrowState {
+ FoundNoPathForThrow,
+ FoundPathForThrow,
+ FoundPathWithNoThrowOutFunction,
+};
+
+static bool mayThrowBeCaughted(const CXXThrowExpr *Throw,
+ const CXXCatchStmt *Catch) {
+ bool MayCaught = false;
+ const auto *ThrowType = Throw->getSubExpr()->getType().getTypePtrOrNull();
+ const auto *CaughtType = Catch->getCaughtType().getTypePtrOrNull();
+
+ if (CaughtType == nullptr)
+ return true;
+ if (CaughtType == ThrowType)
+ return true;
+
+ const auto *CaughtAsRecordType = CaughtType->getPointeeCXXRecordDecl();
+ const auto *ThrowTypeAsRecordType = ThrowType->getAsCXXRecordDecl();
+ if (CaughtAsRecordType && ThrowTypeAsRecordType) {
+ if (CaughtAsRecordType == ThrowTypeAsRecordType)
+ MayCaught = true;
+ MayCaught = ThrowTypeAsRecordType->isDerivedFrom(CaughtAsRecordType);
+ }
+ return MayCaught;
+}
+
+static bool mayThrowBeCaughtedByHandlers(const CXXThrowExpr *CE,
+ const CXXTryStmt *TryStmt) {
+ bool Caught = false;
+ for (unsigned h = 0; h < TryStmt->getNumHandlers(); ++h) {
+ const CXXCatchStmt *CS = TryStmt->getHandler(h);
+ if (mayThrowBeCaughted(CE, CS)) {
+ Caught = true;
+ break;
+ }
+ }
+ return Caught;
+}
+
+static bool hasThrowOutNothrowingFuncInPath(CFGBlock &Block,
+ SourceLocation *OpLoc) {
+ bool HasThrowOutFunc = false;
+ for (const auto &B : Block) {
+ if (B.getKind() != CFGElement::Statement)
+ continue;
+ const CXXThrowExpr *CE =
+ dyn_cast<CXXThrowExpr>(B.getAs<CFGStmt>()->getStmt());
+ if (!CE)
+ continue;
+ else
+ HasThrowOutFunc = true;
+
+ (*OpLoc) = CE->getThrowLoc();
+ for (CFGBlock::const_succ_iterator I = Block.succ_begin(),
+ E = Block.succ_end();
+ I != E; ++I) {
+ if (*I && (*I)->getTerminator() &&
+ isa<CXXTryStmt>((*I)->getTerminator())) {
+ const CXXTryStmt *Terminator = cast<CXXTryStmt>((*I)->getTerminator());
+ if (mayThrowBeCaughtedByHandlers(CE, Terminator)) {
+ HasThrowOutFunc = false;
+ break;
+ }
+ }
+ }
+ }
+ return HasThrowOutFunc;
+}
+
+static bool hasThrowOutNonThrowingFunc(const FunctionDecl *FD,
+ SourceLocation *OpLoc, CFG *cfg) {
+
+ const unsigned ExitID = cfg->getExit().getBlockID();
+
+ SmallVector<ThrowState, 16> States(cfg->getNumBlockIDs(),
+ FoundNoPathForThrow);
+ States[cfg->getEntry().getBlockID()] = FoundPathWithNoThrowOutFunction;
+
+ SmallVector<CFGBlock *, 16> Stack;
+ Stack.push_back(&cfg->getEntry());
+ while (!Stack.empty()) {
+ CFGBlock *CurBlock = Stack.back();
+ Stack.pop_back();
+
+ unsigned ID = CurBlock->getBlockID();
+ ThrowState CurState = States[ID];
+ if (CurState == FoundPathWithNoThrowOutFunction) {
+ // Found a path to the exit node without a throw exression.
+ if (ExitID == ID)
+ continue;
+
+ if (hasThrowOutNothrowingFuncInPath(*CurBlock, OpLoc)) {
+ CurState = FoundPathForThrow;
+ }
+ }
+
+ // 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 (next_ID == ExitID && CurState == FoundPathForThrow) {
+ States[next_ID] = CurState;
+ } else 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 throw expression.
+ return States[ExitID] == FoundPathForThrow;
+}
+
+static void EmitDiagForCXXThrowInNonThrowingFunc(SourceLocation OpLoc, Sema &S,
+ const FunctionDecl *FD) {
+ S.Diag(OpLoc, diag::warn_throw_in_noexcept_func) << FD;
+ if (S.getLangOpts().CPlusPlus11 &&
+ (isa<CXXDestructorDecl>(FD) ||
+ FD->getDeclName().getCXXOverloadedOperator() == OO_Delete ||
+ FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete))
+ S.Diag(FD->getLocation(), diag::note_throw_in_dtor);
+ else
+ S.Diag(FD->getLocation(), diag::note_throw_in_function);
+}
+
+static void checkThrowInNonThrowingFucn(Sema &S, const FunctionDecl *FD,
+ AnalysisDeclContext &AC) {
+ CFG *cfg = AC.getCFG();
+ if (!cfg)
+ return;
+ if (cfg->getExit().pred_empty())
+ return;
+ SourceLocation OpLoc;
+ if (hasThrowOutNonThrowingFunc(FD, &OpLoc, cfg)) {
+ EmitDiagForCXXThrowInNonThrowingFunc(OpLoc, S, FD);
+ }
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+ if (const auto *FPT = FD->getType()->castAs<FunctionProtoType>())
+ if (FPT->getExceptionSpecType() != EST_None &&
+ FPT->isNothrow(FD->getASTContext()))
+ return true;
+ return false;
+}
+
+//===----------------------------------------------------------------------===//
// Check for missing return value.
//===----------------------------------------------------------------------===//
@@ -2127,6 +2279,14 @@
}
}
+ //Check for throw out in non-throwing function.
+ if (!Diags.isIgnored(diag::warn_throw_in_noexcept_func,
+ D->getLocStart())) {
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+ if (S.getLangOpts().CPlusPlus && isNoexcept(FD))
+ checkThrowInNonThrowingFucn(S, FD, AC);
+ }
+
// If none of the previous checks caused a CFG build, trigger one here
// for -Wtautological-overlap-compare
if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6336,6 +6336,15 @@
"cannot use '%0' with exceptions disabled">;
def err_objc_exceptions_disabled : Error<
"cannot use '%0' with Objective-C exceptions disabled">;
+def warn_throw_in_noexcept_func
+ : Warning<"%0 has a non-throwing exception specification but can still "
+ "throw, may result in unexpected program termination.">,
+ InGroup<Exceptions>;
+def note_throw_in_dtor
+ : Note<"destructor or deallocator has a (possible implicit) non-throwing "
+ "excepton specification">;
+def note_throw_in_function
+ : Note<"nonthrowing function declare here">;
def err_seh_try_outside_functions : Error<
"cannot use SEH '__try' in blocks, captured regions, or Obj-C method decls">;
def err_mixing_cxx_try_seh_try : Error<
Index: test/SemaCXX/warn-throw-out-noexcept-func.cpp
===================================================================
--- test/SemaCXX/warn-throw-out-noexcept-func.cpp
+++ test/SemaCXX/warn-throw-out-noexcept-func.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 %s -fdelayed-template-parsing -fcxx-exceptions -fexceptions -fsyntax-only -Wexceptions -verify -std=c++11
+struct A {
+ ~A();
+}; // implicitly noexcept(true)
+A::~A() { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+struct B {
+ int i;
+ ~B() noexcept(true) {}
+};
+struct R : A {
+ B b;
+ ~R() { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+
+struct M : A {
+ B b;
+ ~M() noexcept(false);
+};
+
+M::~M() noexcept(false) {
+ throw 1;
+}
+
+struct N : A {
+ B b;
+ ~N(); //implicitly noexcept(true)
+};
+
+N::~N() { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+struct X : A {
+ B b;
+ ~X() noexcept { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+struct Y : A {
+ B b;
+ ~Y() noexcept(true) { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+struct C {
+ int i;
+ ~C() noexcept(false) {}
+};
+struct D : A {
+ C c;
+ ~D() { //implicitly noexcept(false)
+ throw 1;
+ }
+};
+struct E : A {
+ C c;
+ ~E(); //implicitly noexcept(false)
+};
+E::~E() //implicitly noexcept(false)
+{
+ throw 1;
+}
+
+template <typename T>
+class A1 {
+ T b;
+
+public:
+ ~A1() { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+template <typename T>
+struct B1 {
+ T i;
+ ~B1() noexcept(true) {}
+};
+template <typename T>
+struct R1 : A1<T> //expected-note {{in instantiation of member function}}
+{
+ B1<T> b;
+ ~R1() { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+template <typename T>
+struct S1 : A1<T> {
+ B1<T> b;
+ ~S1() noexcept { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+void operator delete(void *ptr) noexcept { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+struct except_fun {
+ static const bool i = false;
+};
+struct noexcept_fun {
+ static const bool i = true;
+};
+template <typename T>
+struct dependent_warn {
+ ~dependent_warn() noexcept(T::i) {
+ throw 1;
+ }
+};
+template <typename T>
+struct dependent_warn_noexcept {
+ ~dependent_warn_noexcept() noexcept(T::i) { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+template <typename T>
+struct dependent_warn_both {
+ ~dependent_warn_both() noexcept(T::i) { // expected-note {{destructor or deallocator has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+ }
+};
+void foo() noexcept { //expected-note {{nonthrowing function declare here}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+void bar() noexcept {
+ try {
+ throw 1;
+ } catch (...) {
+ }
+}
+void f() noexcept {
+ try {
+ throw 12;
+ } catch (int) {
+ }
+}
+void g() noexcept {
+ try {
+ throw 12;
+ } catch (...) {
+ }
+}
+
+void h() noexcept { //expected-note {{nonthrowing function declare here}}
+ try {
+ throw 12; // expected-warning {{has a non-throwing exception specification but}}
+ } catch (const char *) {
+ }
+}
+
+void i() noexcept { //expected-note {{nonthrowing function declare here}}
+ try {
+ throw 12;
+ } catch (int) {
+ throw; // expected-warning {{has a non-throwing exception specification but}}
+ }
+}
+void j() noexcept { //expected-note {{nonthrowing function declare here}}
+ try {
+ throw 12;
+ } catch (int) {
+ throw "haha"; // expected-warning {{has a non-throwing exception specification but}}
+ }
+}
+
+void k() noexcept { //expected-note {{nonthrowing function declare here}}
+ try {
+ throw 12;
+ } catch (...) {
+ throw; // expected-warning {{has a non-throwing exception specification but}}
+ }
+}
+
+void loo(int i) noexcept { //expected-note {{nonthrowing function declare here}}
+ if (i)
+ try {
+ throw 12;
+ } catch (int) {
+ throw "haha"; //expected-warning {{has a non-throwing exception specification but}}
+ }
+ i = 10;
+}
+
+#define NOEXCEPT noexcept
+void with_macro() NOEXCEPT { //expected-note {{nonthrowing function declare here}}
+ throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+
+void with_try_block() try {
+ throw 2;
+} catch (...) {
+}
+
+int main() {
+ R1<int> o; //expected-note {{in instantiation of member function}}
+ S1<int> b; //expected-note {{in instantiation of member function}}
+ dependent_warn<except_fun> f;
+ dependent_warn_noexcept<noexcept_fun> f1; //expected-note {{in instantiation of member function}}
+ dependent_warn_both<except_fun> f2;
+ dependent_warn_both<noexcept_fun> f3; //expected-note {{in instantiation of member function}}
+}
Index: test/CXX/except/except.spec/p11.cpp
===================================================================
--- test/CXX/except/except.spec/p11.cpp
+++ test/CXX/except/except.spec/p11.cpp
@@ -1,12 +1,11 @@
// RUN: %clang_cc1 -std=c++11 -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
-// expected-no-diagnostics
// This is the "let the user shoot themselves in the foot" clause.
-void f() noexcept {
- throw 0; // no-error
+void f() noexcept { // expected-note {{nonthrowing function declare here}}
+ throw 0; // expected-warning {{has a non-throwing exception specification but}}
}
-void g() throw() {
- throw 0; // no-error
+void g() throw() { // expected-note {{nonthrowing function declare here}}
+ throw 0; // expected-warning {{has a non-throwing exception specification but}}
}
void h() throw(int) {
throw 0.0; // no-error
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
