Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td (revision 230630) +++ 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 230630) +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy) @@ -5474,7 +5474,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 230630) +++ lib/Sema/SemaStmt.cpp (working copy) @@ -15,6 +15,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/ASTDiagnostic.h" #include "clang/AST/CharUnits.h" +#include "clang/AST/CXXInheritance.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/EvaluatedExprVisitor.h" #include "clang/AST/ExprCXX.h" @@ -23,6 +24,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 +31,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" @@ -3255,38 +3258,121 @@ return new (Context) ObjCAutoreleasePoolStmt(AtLoc, Body); } -namespace { +class QualTypeExt { + QualType QT; + unsigned IsPointer : 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; + enum Unique { ForDenseMap }; + QualTypeExt(QualType QT, Unique) : QT(QT), IsPointer(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) { + if (QT->isPointerType()) + IsPointer = 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 || QT->isReferenceType()) + QT = QT->getPointeeType(); + QT = QT.getUnqualifiedType(); + } + + /// Used when creating a QualTypeExt from a base class type; pretends the type + /// passed in had the pointer qualifier, does not need to get an unqualified + /// type. + QualTypeExt(QualType QT, bool IsPointer) + : QT(QT), IsPointer(IsPointer) {} + + QualType underlying() const { return QT; } + bool isPointer() const { return IsPointer; } + + friend bool operator==(const QualTypeExt &LHS, const QualTypeExt &RHS) { + // If the pointer qualification does not match, we can return early. + if (LHS.IsPointer != RHS.IsPointer) return false; - else - return getTypeSpecStartLoc() < y.getTypeSpecStartLoc(); + // 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(), + QualTypeExt::ForDenseMap); } - CXXCatchStmt *getCatchStmt() const { return stmt; } - SourceLocation getTypeSpecStartLoc() const { - return stmt->getExceptionDecl()->getTypeSpecStartLoc(); + static QualTypeExt getTombstoneKey() { + return QualTypeExt(DenseMapInfo::getTombstoneKey(), + QualTypeExt::ForDenseMap); } + + 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 QualTypeExt as a POD type. +template <> struct isPodLike { static const bool value = true; }; } +namespace { +class PublicBasesOfType { + ASTContext &Ctx; + const llvm::DenseMap &TypesToCheck; + const bool CheckAgainstPointer; + + CXXCatchStmt *FoundHandler; + CanQualType FoundHandlerType; + +public: + PublicBasesOfType(ASTContext &Ctx, + const llvm::DenseMap &T, + bool C) + : Ctx(Ctx), TypesToCheck(T), CheckAgainstPointer(C), + FoundHandler(nullptr) {} + + void setFoundHandler(CXXCatchStmt *F) { FoundHandler = F; } + CXXCatchStmt *getFoundHandler() const { return FoundHandler; } + + void setFoundHandlerType(QualType T) { + FoundHandlerType = Ctx.getCanonicalType(T); + } + CanQualType getFoundHandlerType() const { return FoundHandlerType; } + + bool checkAgainstPointer() const { return CheckAgainstPointer; } + const llvm::DenseMap &getTypesToCheck() const { + return TypesToCheck; + } +}; + +bool FindPublicBasesOfType(const CXXBaseSpecifier *, CXXBasePath &Path, + void *User) { + auto &PBOT = *reinterpret_cast(User); + if (Path.Access == AccessSpecifier::AS_public) { + for (const auto &P : Path) { + QualTypeExt Check(P.Base->getType(), PBOT.checkAgainstPointer()); + auto M = PBOT.getTypesToCheck(); + auto I = M.find(Check); + if (I != M.end()) { + PBOT.setFoundHandler(I->second); + PBOT.setFoundHandlerType(P.Base->getType()); + return true; + } + } + } + return false; +} +} + /// ActOnCXXTryBlock - Takes a try compound-statement and a number of /// handlers and creates a try statement from them. StmtResult Sema::ActOnCXXTryBlock(SourceLocation TryLoc, Stmt *TryBlock, @@ -3294,7 +3380,7 @@ // Don't report an error if 'try' is used in system headers. if (!getLangOpts().CXXExceptions && !getSourceManager().isInSystemHeader(TryLoc)) - Diag(TryLoc, diag::err_exceptions_disabled) << "try"; + Diag(TryLoc, diag::err_exceptions_disabled) << "try"; if (getCurScope() && getCurScope()->isOpenMPSimdDirectiveScope()) Diag(TryLoc, diag::err_omp_simd_region_cannot_use_stmt) << "try"; @@ -3308,55 +3394,72 @@ } 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 *H = cast(Handlers[i]); - for (unsigned i = 0; i < NumHandlers; ++i) { - CXXCatchStmt *Handler = cast(Handlers[i]); - if (!Handler->getExceptionDecl()) { + // Diagnose when the handler is a catch-all handler, but it isn't the last + // handler for the try block. [except.handle]p5. Also, skip exception + // declarations that are invalid, since we can't usefully report on them. + if (!H->getExceptionDecl()) { if (i < NumHandlers - 1) - return StmtError(Diag(Handler->getLocStart(), - diag::err_early_catch_all)); - + return StmtError(Diag(H->getLocStart(), diag::err_early_catch_all)); continue; - } + } else if (H->getExceptionDecl()->isInvalidDecl()) + 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 + QualTypeExt HandlerQTE = Context.getCanonicalType(H->getCaughtType()); - // 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(); + // We can ignore whether the type is a reference or a pointer; we need the + // underlying declaration type in order to get at the underlying record + // decl, if there is one. + QualType Underlying = HandlerQTE.underlying(); + if (auto *RD = Underlying->getAsCXXRecordDecl()) { + if (!RD->hasDefinition()) + continue; + // Check that none of the public, unambiguous base classes are in the + // map ([except.handle]p1). Give the base classes the same pointer + // qualification as the original type we are basing off of. This allows + // comparison against the handler type using the same top-level pointer + // as the original type. + CXXBasePaths Paths; + Paths.setOrigin(RD); + PublicBasesOfType PBOT(Context, HandledTypes, HandlerQTE.isPointer()); + if (RD->lookupInBases(FindPublicBasesOfType, &PBOT, Paths)) { + const CXXCatchStmt *Problem = PBOT.getFoundHandler(); + if (!Paths.isAmbiguous(PBOT.getFoundHandlerType())) { + Diag(H->getExceptionDecl()->getTypeSpecStartLoc(), + diag::warn_exception_caught_by_earlier_handler) + << H->getCaughtType(); + Diag(Problem->getExceptionDecl()->getTypeSpecStartLoc(), + diag::note_previous_exception_handler) + << Problem->getCaughtType(); + } } + } - prev = curr; + // Add the type the list of ones we have handled; diagnose if we've already + // handled it. + auto R = HandledTypes.insert(std::make_pair(H->getCaughtType(), H)); + if (!R.second) { + const CXXCatchStmt *Problem = R.first->second; + Diag(H->getExceptionDecl()->getTypeSpecStartLoc(), + diag::warn_exception_caught_by_earlier_handler) + << H->getCaughtType(); + Diag(Problem->getExceptionDecl()->getTypeSpecStartLoc(), + diag::note_previous_exception_handler) + << Problem->getCaughtType(); } } FSI->setHasCXXTry(TryLoc); - // 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 230630) +++ 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 230630) +++ 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 (94): +CHECK: Warnings without flags (93): CHECK-NEXT: ext_excess_initializers CHECK-NEXT: ext_excess_initializers_in_char_array_initializer CHECK-NEXT: ext_expected_semi_decl_list @@ -68,7 +68,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 230630) +++ test/SemaCXX/exceptions.cpp (working copy) @@ -145,3 +145,79 @@ } void rval_ref() throw (int &&); // expected-error {{rvalue reference type 'int &&' is not allowed in exception specification}} expected-warning {{C++11}} + +namespace HandlerInversion { +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 + } + + 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}} + } + + 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}} + } +} +} Index: test/SemaCXX/unreachable-catch-clauses.cpp =================================================================== --- test/SemaCXX/unreachable-catch-clauses.cpp (revision 230630) +++ 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 2{{for type 'BaseEx &'}} +catch (Ex1 &e) { f(); } // expected-warning {{exception of type 'Ex1 &' will be caught by earlier handler}} +catch (Ex2 &e) { f(); } // expected-warning {{exception of type 'Ex2 &' (aka 'Ex1 &') will be caught by earlier handler}}