Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td (revision 221992) +++ 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 221992) +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy) @@ -5378,7 +5378,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 221992) +++ lib/Sema/SemaStmt.cpp (working copy) @@ -23,6 +23,7 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtObjC.h" #include "clang/AST/TypeLoc.h" +#include "clang/AST/TypeOrdering.h" #include "clang/Lex/Preprocessor.h" #include "clang/Sema/Initialization.h" #include "clang/Sema/Lookup.h" @@ -29,6 +30,7 @@ #include "clang/Sema/Scope.h" #include "clang/Sema/ScopeInfo.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" @@ -3206,36 +3208,75 @@ return new (Context) ObjCAutoreleasePoolStmt(AtLoc, Body); } -namespace { +class QualTypeExt { + QualType QT; + unsigned IsPointer : 1; + unsigned IsReference : 1; -class TypeWithHandler { - QualType t; - CXXCatchStmt *stmt; + // This is a special constructor to be used only with DenseMapInfo's + // getEmptyKey() and getTombstoneKey() functions. + friend struct llvm::DenseMapInfo ; + QualTypeExt(QualType QT, bool) + : QT(QT), IsPointer(false), IsReference(false) {} + public: - TypeWithHandler(const QualType &type, CXXCatchStmt *statement) - : t(type), stmt(statement) {} + /// Used when creating a QualTypeExt from a handler type; will determine + /// whether the type is a pointer or reference and will strip off the the top + /// level pointer and cv-qualifiers. + QualTypeExt(QualType Q) : QT(Q), IsPointer(false), IsReference(false) { + if (QT->isPointerType()) + IsPointer = true; + else if (QT->isReferenceType()) + IsReference = true; - // 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()) + if (IsPointer || IsReference) + QT = QT->getPointeeType(); + QT = QT.getUnqualifiedType(); + } + + /// Used when creating a QualTypeExt from a base class type; pretends the type + /// passed in had the pointer or reference qualifiers, does not need to get + /// an unqualified type. + QualTypeExt(QualType QT, bool IsPointer, bool IsReference) + : QT(QT), IsPointer(IsPointer), IsReference(IsReference) {} + + QualType underlying() const { return QT; } + bool isReference() const { return IsReference; } + bool isPointer() const { return IsPointer; } + + friend bool operator==(const QualTypeExt &LHS, const QualTypeExt &RHS) { + // If the reference or pointer qualification does not match, we can return + // early. + if (LHS.IsPointer != RHS.IsPointer) return false; - else - return getTypeSpecStartLoc() < y.getTypeSpecStartLoc(); + if (LHS.IsReference != RHS.IsReference) + return false; + // Otherwise, check the underlying type without cv-qualifiers. + return LHS.QT == RHS.QT; } +}; - bool operator==(const TypeWithHandler& other) const { - return t == other.t; +namespace llvm { +template <> struct DenseMapInfo { + static QualTypeExt getEmptyKey() { + return QualTypeExt(DenseMapInfo::getEmptyKey(), true); } - CXXCatchStmt *getCatchStmt() const { return stmt; } - SourceLocation getTypeSpecStartLoc() const { - return stmt->getExceptionDecl()->getTypeSpecStartLoc(); + static QualTypeExt getTombstoneKey() { + return QualTypeExt(DenseMapInfo::getTombstoneKey(), true); } + + static unsigned getHashValue(const QualTypeExt &Base) { + return DenseMapInfo::getHashValue(Base.underlying()); + } + + static bool isEqual(const QualTypeExt &LHS, const QualTypeExt &RHS) { + return LHS == RHS; + } }; +// It's OK to treat BaseSubobject as a POD type. +template <> struct isPodLike { static const bool value = true; }; } /// ActOnCXXTryBlock - Takes a try compound-statement and a number of @@ -3251,13 +3292,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; - + llvm::DenseMap 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 +3309,52 @@ 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 + SmallVector BaseClassTypes; + QualTypeExt HandlerQTE = Context.getCanonicalType(Handler->getCaughtType()); + BaseClassTypes.push_back(HandlerQTE); + while (!BaseClassTypes.empty()) { + QualTypeExt CurType = BaseClassTypes.pop_back_val(); + // See if the type matches one we have already handled. + auto I = HandledTypes.find(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.underlying(); + 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()) + BaseClassTypes.push_back(QualTypeExt(B.getType(), CurType.isPointer(), + CurType.isReference())); + } } + // Add the type the list of ones we have handled. + HandledTypes[HandlerQTE] = 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 221992) +++ 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 221992) +++ 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 221992) +++ 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 221992) +++ 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}}