Index: lib/Sema/SemaStmt.cpp =================================================================== --- lib/Sema/SemaStmt.cpp (revision 220684) +++ lib/Sema/SemaStmt.cpp (working copy) @@ -3206,36 +3206,27 @@ return new (Context) ObjCAutoreleasePoolStmt(AtLoc, Body); } -namespace { +static const CXXCatchStmt *getProblematicHandler( + Sema &S, const std::pair &Check, + const SmallVectorImpl> &List) { + QualType CheckTy = Check.first; + if (CheckTy->isPointerType() || CheckTy->isReferenceType()) + CheckTy = CheckTy->getPointeeType(); -class TypeWithHandler { - QualType t; - CXXCatchStmt *stmt; -public: - TypeWithHandler(const QualType &type, CXXCatchStmt *statement) - : t(type), stmt(statement) {} + for (const auto < : List) { + // Check whether the type matches one already in the list. + if (Check == LT) + return LT.second; - // An arbitrary order is fine as long as it places identical - // types next to each other. - bool operator<(const TypeWithHandler &y) const { - if (t.getAsOpaquePtr() < y.t.getAsOpaquePtr()) - return true; - if (t.getAsOpaquePtr() > y.t.getAsOpaquePtr()) - return false; - else - return getTypeSpecStartLoc() < y.getTypeSpecStartLoc(); - } + QualType LTTy = LT.first; + if (LTTy->isPointerType() || LTTy->isReferenceType()) + LTTy = LTTy->getPointeeType(); - bool operator==(const TypeWithHandler& other) const { - return t == other.t; + // Check whether the type is derived from one already in the list. + if (S.IsDerivedFrom(CheckTy, LTTy)) + return LT.second; } - - CXXCatchStmt *getCatchStmt() const { return stmt; } - SourceLocation getTypeSpecStartLoc() const { - return stmt->getExceptionDecl()->getTypeSpecStartLoc(); - } -}; - + return nullptr; } /// ActOnCXXTryBlock - Takes a try compound-statement and a number of @@ -3254,7 +3245,7 @@ assert(NumHandlers > 0 && "The parser shouldn't call this if there are no handlers."); - SmallVector TypesWithHandlers; + SmallVector, 8> TypesWithHandlers; for (unsigned i = 0; i < NumHandlers; ++i) { CXXCatchStmt *Handler = cast(Handlers[i]); @@ -3268,38 +3259,27 @@ const QualType CaughtType = Handler->getCaughtType(); const QualType CanonicalCaughtType = Context.getCanonicalType(CaughtType); - TypesWithHandlers.push_back(TypeWithHandler(CanonicalCaughtType, Handler)); - } + std::pair TWH(CanonicalCaughtType, Handler); - // Detect handlers for the same type as an earlier one. - if (NumHandlers > 1) { - llvm::array_pod_sort(TypesWithHandlers.begin(), TypesWithHandlers.end()); - - TypeWithHandler prev = TypesWithHandlers[0]; - for (unsigned i = 1; i < TypesWithHandlers.size(); ++i) { - TypeWithHandler curr = TypesWithHandlers[i]; - - if (curr == prev) { - Diag(curr.getTypeSpecStartLoc(), - diag::warn_exception_caught_by_earlier_handler) - << curr.getCatchStmt()->getCaughtType().getAsString(); - Diag(prev.getTypeSpecStartLoc(), - diag::note_previous_exception_handler) - << prev.getCatchStmt()->getCaughtType().getAsString(); - } - - prev = curr; + // Detect handlers that are skipped due to derivation inversion, or + // duplicated. + // FIXME: getProblematicHandler makes this a quadratic operation; it would + // be good to find a more efficient solution. + const CXXCatchStmt *Problem = + getProblematicHandler(*this, TWH, TypesWithHandlers); + if (Problem) { + Diag(Handler->getExceptionDecl()->getTypeSpecStartLoc(), + diag::warn_exception_caught_by_earlier_handler) + << Handler->getCaughtType().getAsString(); + Diag(Problem->getExceptionDecl()->getTypeSpecStartLoc(), + diag::note_previous_exception_handler) + << Problem->getCaughtType().getAsString(); } + TypesWithHandlers.push_back(TWH); } getCurFunction()->setHasBranchProtectedScope(); - // FIXME: We should detect handlers that cannot catch anything because an - // earlier handler catches a superclass. Need to find a method that is not - // quadratic for this. - // Neither of these are explicitly forbidden, but every compiler detects them - // and warns. - return CXXTryStmt::Create(Context, TryLoc, TryBlock, Handlers); } Index: test/CXX/drs/dr3xx.cpp =================================================================== --- test/CXX/drs/dr3xx.cpp (revision 220684) +++ test/CXX/drs/dr3xx.cpp (working copy) @@ -170,9 +170,9 @@ void f() { try { throw D(); - } catch (const A&) { + } catch (const A&) { // expected-note {{for type const struct dr308::A &}} // unreachable - } catch (const B&) { + } catch (const B&) { // expected-warning {{exception of type const struct dr308::B & will be caught by earlier handler}} // get here instead } } Index: test/SemaCXX/exceptions.cpp =================================================================== --- test/SemaCXX/exceptions.cpp (revision 220684) +++ test/SemaCXX/exceptions.cpp (working copy) @@ -145,3 +145,52 @@ } void rval_ref() throw (int &&); // expected-error {{rvalue reference type 'int &&' is not allowed in exception specification}} expected-warning {{C++11}} + +namespace HandlerInversion { +// RUN: %clang_cc1 -fcxx-exceptions -analyze -analyzer-checker=cplusplus -verify %s + +struct B {}; +struct D : B {}; +struct D2 : D {}; + +void f1() { + try { + } catch (B &b) { // expected-note {{for type struct HandlerInversion::B &}} + } catch (D &d) { // expected-warning {{exception of type struct HandlerInversion::D & will be caught by earlier handler}} + } +} + +void f2() { + try { + } catch (B *b) { // expected-note {{for type struct HandlerInversion::B *}} + } catch (D *d) { // expected-warning {{exception of type struct HandlerInversion::D * will be caught by earlier handler}} + } +} + +void f3() { + try { + } catch (D &d) { // Ok + } catch (B &b) { + } +} + +void f4() { + try { + } catch (B &b) { // Ok + } +} + +void f5() { + try { + } catch (int) { + } catch (float) { + } +} + +void f6() { + try { + } catch (B &b) { // expected-note {{for type struct HandlerInversion::B &}} + } catch (D2 &d) { // expected-warning {{exception of type struct HandlerInversion::D2 & will be caught by earlier handler}} + } +} +} Index: test/SemaCXX/unreachable-catch-clauses.cpp =================================================================== --- test/SemaCXX/unreachable-catch-clauses.cpp (revision 220684) +++ test/SemaCXX/unreachable-catch-clauses.cpp (working copy) @@ -8,7 +8,6 @@ void test() try {} -catch (BaseEx &e) { f(); } -catch (Ex1 &e) { f(); } // expected-note {{for type class Ex1 &}} +catch (BaseEx &e) { f(); } // expected-note {{for type class BaseEx &}} expected-note {{for type class BaseEx &}} +catch (Ex1 &e) { f(); } // expected-warning {{exception of type class Ex1 & will be caught by earlier handler}} catch (Ex2 &e) { f(); } // expected-warning {{exception of type Ex2 & will be caught by earlier handler}} -