Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td (revision 221885) +++ include/clang/Basic/DiagnosticGroups.td (working copy) @@ -106,6 +106,8 @@ DocumentationDeprecatedSync]>; def EmptyBody : DiagGroup<"empty-body">; +def Exceptions : DiagGroup<"exceptions">; + def GNUEmptyInitializer : DiagGroup<"gnu-empty-initializer">; def GNUEmptyStruct : DiagGroup<"gnu-empty-struct">; def ExtraTokens : DiagGroup<"extra-tokens">; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td (revision 221885) +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy) @@ -5376,7 +5376,8 @@ "left hand operand to %0 must be a %select{|pointer to }1class " "compatible with the right hand operand, but is %2">; def warn_exception_caught_by_earlier_handler : Warning< - "exception of type %0 will be caught by earlier handler">; + "exception of type %0 will be caught by earlier handler">, + InGroup; def note_previous_exception_handler : Note<"for type %0">; def err_exceptions_disabled : Error< "cannot use '%0' with exceptions disabled">; Index: lib/Sema/SemaStmt.cpp =================================================================== --- lib/Sema/SemaStmt.cpp (revision 221885) +++ lib/Sema/SemaStmt.cpp (working copy) @@ -33,6 +33,7 @@ #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" +#include using namespace clang; using namespace sema; @@ -3206,36 +3207,10 @@ return new (Context) ObjCAutoreleasePoolStmt(AtLoc, Body); } -namespace { - -class TypeWithHandler { - QualType t; - CXXCatchStmt *stmt; -public: - TypeWithHandler(const QualType &type, CXXCatchStmt *statement) - : t(type), stmt(statement) {} - - // 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(); - } - - bool operator==(const TypeWithHandler& other) const { - return t == other.t; - } - - CXXCatchStmt *getCatchStmt() const { return stmt; } - SourceLocation getTypeSpecStartLoc() const { - return stmt->getExceptionDecl()->getTypeSpecStartLoc(); - } -}; - +static QualType getUnderlyingTypeForHandler(QualType Q) { + if (Q->isPointerType() || Q->isReferenceType()) + Q = Q->getPointeeType(); + return Q.getUnqualifiedType(); } /// ActOnCXXTryBlock - Takes a try compound-statement and a number of @@ -3251,13 +3226,15 @@ Diag(TryLoc, diag::err_omp_simd_region_cannot_use_stmt) << "try"; const unsigned NumHandlers = Handlers.size(); - assert(NumHandlers > 0 && + assert(!Handlers.empty() && "The parser shouldn't call this if there are no handlers."); - SmallVector TypesWithHandlers; - + SmallVector, 8> HandledTypes; for (unsigned i = 0; i < NumHandlers; ++i) { CXXCatchStmt *Handler = cast(Handlers[i]); + + // Diagnose when the handler is a catch-all handler, but it isn't the last + // handler for the try block. [except.handle]p5 if (!Handler->getExceptionDecl()) { if (i < NumHandlers - 1) return StmtError(Diag(Handler->getLocStart(), @@ -3266,40 +3243,71 @@ continue; } - const QualType CaughtType = Handler->getCaughtType(); - const QualType CanonicalCaughtType = Context.getCanonicalType(CaughtType); - TypesWithHandlers.push_back(TypeWithHandler(CanonicalCaughtType, Handler)); - } + // Walk the type hierarchy to diagnose when this type has already been + // handled (duplication), or cannot be handled (derivation inversion). We + // ignore top-level cv-qualifiers, per [except.handle]p3 + std::list BaseClassTypes; + BaseClassTypes.push_back(Context.getCanonicalType(Handler->getCaughtType()) + .getUnqualifiedType()); + for (auto &CurType : BaseClassTypes) { + // See if the type matches one we have already handled. + auto I = std::find_if( + HandledTypes.begin(), HandledTypes.end(), + [CurType](const std::pair &TWH) { + // If the reference or pointer qualification does not match, we can + // return early. + if (CurType->isReferenceType() != TWH.first->isReferenceType()) + return false; + if (CurType->isPointerType() != TWH.first->isPointerType()) + return false; + // Otherwise, strip off the pointer/reference qualification and + // check the underlying type without cv-qualifiers. + return getUnderlyingTypeForHandler(TWH.first) == + getUnderlyingTypeForHandler(CurType); + }); - // 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(), + // This type has already been handled, so we should diagnose it. + if (I != HandledTypes.end()) { + const CXXCatchStmt *Problem = I->second; + Diag(Handler->getExceptionDecl()->getTypeSpecStartLoc(), diag::warn_exception_caught_by_earlier_handler) - << curr.getCatchStmt()->getCaughtType().getAsString(); - Diag(prev.getTypeSpecStartLoc(), + << Handler->getCaughtType(); + Diag(Problem->getExceptionDecl()->getTypeSpecStartLoc(), diag::note_previous_exception_handler) - << prev.getCatchStmt()->getCaughtType().getAsString(); + << Problem->getCaughtType(); + break; } - prev = curr; + // Get the next type in the type hierarchy. If there are no more types + // in the hierarchy, break out of the loop. At this point, we can ignore + // whether the type is a reference or a pointer; we need the underlying + // declaration type. + QualType Underlying = CurType; + if (Underlying->isPointerType() || Underlying->isReferenceType()) + Underlying = Underlying->getPointeeType(); + if (auto *RD = Underlying->getAsCXXRecordDecl()) { + if (!RD->hasDefinition()) + continue; + // Add direct base classes to the list of types to be checked. Give the + // base classes the same pointer/reference qualification as the original + // type we are basing off of. This allows comparison against the handler + // type using the same top-level * or & as the original type. + for (const auto &B : RD->bases()) { + QualType BTy = B.getType(); + if (CurType->isPointerType()) + BTy = Context.getPointerType(BTy); + else if (CurType->isReferenceType()) + BTy = Context.getLValueReferenceType(BTy); + BaseClassTypes.push_back(BTy); + } + } } + // Add the type the list of ones we have handled. + HandledTypes.push_back(std::make_pair(BaseClassTypes.front(), Handler)); } 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 221885) +++ 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 dr308::A &'}} // unreachable - } catch (const B&) { + } catch (const B&) { // expected-warning {{exception of type 'const dr308::B &' will be caught by earlier handler}} // get here instead } } Index: test/Misc/warning-flags.c =================================================================== --- test/Misc/warning-flags.c (revision 221885) +++ test/Misc/warning-flags.c (working copy) @@ -18,7 +18,7 @@ The list of warnings below should NEVER grow. It should gradually shrink to 0. -CHECK: Warnings without flags (98): +CHECK: Warnings without flags (97): CHECK-NEXT: ext_excess_initializers CHECK-NEXT: ext_excess_initializers_in_char_array_initializer CHECK-NEXT: ext_expected_semi_decl_list @@ -70,7 +70,6 @@ CHECK-NEXT: warn_dup_category_def CHECK-NEXT: warn_duplicate_protocol_def CHECK-NEXT: warn_enum_value_overflow -CHECK-NEXT: warn_exception_caught_by_earlier_handler CHECK-NEXT: warn_expected_qualified_after_typename CHECK-NEXT: warn_extraneous_char_constant CHECK-NEXT: warn_fe_cc_log_diagnostics_failure Index: test/SemaCXX/exceptions.cpp =================================================================== --- test/SemaCXX/exceptions.cpp (revision 221885) +++ test/SemaCXX/exceptions.cpp (working copy) @@ -145,3 +145,71 @@ } 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 'HandlerInversion::B &'}} + } catch (D &d) { // expected-warning {{exception of type 'HandlerInversion::D &' will be caught by earlier handler}} + } +} + +void f2() { + try { + } catch (B *b) { // expected-note {{for type 'HandlerInversion::B *'}} + } catch (D *d) { // expected-warning {{exception of type '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 'HandlerInversion::B &'}} + } catch (D2 &d) { // expected-warning {{exception of type 'HandlerInversion::D2 &' will be caught by earlier handler}} + } +} + +void f7() { + try { + } catch (B *b) { // Ok + } catch (D &d) { // Ok + } +} + +void f8() { + try { + } catch (const B &b) { // expected-note {{for type 'const HandlerInversion::B &'}} + } catch (D2 &d) { // expected-warning {{exception of type 'HandlerInversion::D2 &' will be caught by earlier handler}} + } + + try { + } catch (B &b) { // expected-note {{for type 'HandlerInversion::B &'}} + } catch (const D2 &d) { // expected-warning {{exception of type 'const HandlerInversion::D2 &' will be caught by earlier handler}} + } +} +} Index: test/SemaCXX/unreachable-catch-clauses.cpp =================================================================== --- test/SemaCXX/unreachable-catch-clauses.cpp (revision 221885) +++ 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 (Ex2 &e) { f(); } // expected-warning {{exception of type Ex2 & will be caught by earlier handler}} - +catch (BaseEx &e) { f(); } // expected-note {{for type 'BaseEx &'}} +catch (Ex1 &e) { f(); } // expected-warning {{exception of type 'Ex1 &' will be caught by earlier handler}} expected-note {{for type 'Ex1 &'}} +catch (Ex2 &e) { f(); } // expected-warning {{exception of type 'Ex2 &' (aka 'Ex1 &') will be caught by earlier handler}}