From 310d7861ec1af4cb59198bef7a0852087f16d499 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Sun, 15 Feb 2015 22:18:04 +0000 Subject: [PATCH] Partial revert of r229336; this wasn't intended to go in. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@229338 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticGroups.td | 2 - include/clang/Basic/DiagnosticSemaKinds.td | 3 +- lib/Sema/SemaStmt.cpp | 179 ++++++--------------- test/CXX/drs/dr3xx.cpp | 4 +- test/Misc/warning-flags.c | 3 +- test/SemaCXX/exceptions.cpp | 78 --------- test/SemaCXX/unreachable-catch-clauses.cpp | 7 +- 7 files changed, 62 insertions(+), 214 deletions(-) diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 8669ba2a65..215ff06ddb 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -106,8 +106,6 @@ def Documentation : DiagGroup<"documentation", 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">; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7cf21b1318..d0949a2f4f 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5465,8 +5465,7 @@ def err_bad_memptr_lhs : Error< "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">, - InGroup; + "exception of type %0 will be caught by earlier handler">; def note_previous_exception_handler : Note<"for type %0">; def err_exceptions_disabled : Error< "cannot use '%0' with exceptions disabled">; diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 806d5385c3..4761c32f78 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -15,7 +15,6 @@ #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" @@ -24,14 +23,12 @@ #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" #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" @@ -3262,81 +3259,36 @@ Sema::ActOnObjCAutoreleasePoolStmt(SourceLocation AtLoc, Stmt *Body) { return new (Context) ObjCAutoreleasePoolStmt(AtLoc, Body); } -class QualTypeExt { - QualType QT; - unsigned IsPointer : 1; - - // 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) {} +namespace { +class TypeWithHandler { + QualType t; + CXXCatchStmt *stmt; public: - /// 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; - - 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; - // Otherwise, check the underlying type without cv-qualifiers. - return LHS.QT == RHS.QT; - } -}; - -namespace llvm { -template <> struct DenseMapInfo { - static QualTypeExt getEmptyKey() { - return QualTypeExt(DenseMapInfo::getEmptyKey(), - QualTypeExt::ForDenseMap); - } + TypeWithHandler(const QualType &type, CXXCatchStmt *statement) + : t(type), stmt(statement) {} - static QualTypeExt getTombstoneKey() { - return QualTypeExt(DenseMapInfo::getTombstoneKey(), - QualTypeExt::ForDenseMap); + // 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(); } - static unsigned getHashValue(const QualTypeExt &Base) { - return DenseMapInfo::getHashValue(Base.underlying()); + bool operator==(const TypeWithHandler& other) const { + return t == other.t; } - static bool isEqual(const QualTypeExt &LHS, const QualTypeExt &RHS) { - return LHS == RHS; + CXXCatchStmt *getCatchStmt() const { return stmt; } + SourceLocation getTypeSpecStartLoc() const { + return stmt->getExceptionDecl()->getTypeSpecStartLoc(); } }; -// It's OK to treat QualTypeExt as a POD type. -template <> struct isPodLike { static const bool value = true; }; -} - -static bool Frobble(const CXXBaseSpecifier *, CXXBasePath &Path, void *User) { - auto *Paths = reinterpret_cast(User); - if (Path.Access == AccessSpecifier::AS_public) { - if (auto *BRD = Path.back().Base->getType()->getAsCXXRecordDecl()) { - BRD->lookupInBases(Frobble, User, *Paths); - } - return true; - } - return false; } /// ActOnCXXTryBlock - Takes a try compound-statement and a number of @@ -3360,80 +3312,55 @@ StmtResult Sema::ActOnCXXTryBlock(SourceLocation TryLoc, Stmt *TryBlock, } const unsigned NumHandlers = Handlers.size(); - assert(!Handlers.empty() && + assert(NumHandlers > 0 && "The parser shouldn't call this if there are no handlers."); - llvm::DenseMap HandledTypes; + SmallVector TypesWithHandlers; + 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. Also, skip exception - // declarations that are invalid, since we can't usefully report on them. if (!Handler->getExceptionDecl()) { if (i < NumHandlers - 1) - return StmtError( - Diag(Handler->getLocStart(), diag::err_early_catch_all)); + return StmtError(Diag(Handler->getLocStart(), + diag::err_early_catch_all)); continue; - } else if (Handler->getExceptionDecl()->isInvalidDecl()) - continue; + } - // 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(Handler->getCaughtType()); - - // 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); - if (RD->lookupInBases(Frobble, &Paths, Paths)) { - for (const auto &B : Paths.front()) { - QualType BaseTy = B.Base->getType(); - if (!Paths.isAmbiguous(Context.getCanonicalType(BaseTy))) { - QualTypeExt Check(BaseTy, HandlerQTE.isPointer()); - auto I = HandledTypes.find(Check); - if (I != HandledTypes.end()) { - const CXXCatchStmt *Problem = I->second; - Diag(Handler->getExceptionDecl()->getTypeSpecStartLoc(), - diag::warn_exception_caught_by_earlier_handler) - << Handler->getCaughtType(); - Diag(Problem->getExceptionDecl()->getTypeSpecStartLoc(), - diag::note_previous_exception_handler) - << Problem->getCaughtType(); - } - } - } + const QualType CaughtType = Handler->getCaughtType(); + const QualType CanonicalCaughtType = Context.getCanonicalType(CaughtType); + TypesWithHandlers.push_back(TypeWithHandler(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(); } - } - // Add the type the list of ones we have handled; diagnose if we've already - // handled it. - auto R = HandledTypes.insert(std::make_pair(HandlerQTE, Handler)); - if (!R.second) { - const CXXCatchStmt *Problem = R.first->second; - Diag(Handler->getExceptionDecl()->getTypeSpecStartLoc(), - diag::warn_exception_caught_by_earlier_handler) - << Handler->getCaughtType(); - Diag(Problem->getExceptionDecl()->getTypeSpecStartLoc(), - diag::note_previous_exception_handler) - << Problem->getCaughtType(); + prev = curr; } } 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); } diff --git a/test/CXX/drs/dr3xx.cpp b/test/CXX/drs/dr3xx.cpp index 5ac4f013b7..cea4d64e7e 100644 --- a/test/CXX/drs/dr3xx.cpp +++ b/test/CXX/drs/dr3xx.cpp @@ -170,9 +170,9 @@ namespace dr308 { // dr308: yes void f() { try { throw D(); - } catch (const A&) { // expected-note {{for type 'const dr308::A &'}} + } catch (const A&) { // unreachable - } catch (const B&) { // expected-warning {{exception of type 'const dr308::B &' will be caught by earlier handler}} + } catch (const B&) { // get here instead } } diff --git a/test/Misc/warning-flags.c b/test/Misc/warning-flags.c index 86274ad0bd..657b44dc18 100644 --- a/test/Misc/warning-flags.c +++ b/test/Misc/warning-flags.c @@ -18,7 +18,7 @@ This test serves two purposes: The list of warnings below should NEVER grow. It should gradually shrink to 0. -CHECK: Warnings without flags (93): +CHECK: Warnings without flags (94): CHECK-NEXT: ext_excess_initializers CHECK-NEXT: ext_excess_initializers_in_char_array_initializer CHECK-NEXT: ext_expected_semi_decl_list @@ -68,6 +68,7 @@ CHECK-NEXT: warn_drv_pch_not_first_include 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 diff --git a/test/SemaCXX/exceptions.cpp b/test/SemaCXX/exceptions.cpp index fcf540f3b3..9646a9c3b3 100644 --- a/test/SemaCXX/exceptions.cpp +++ b/test/SemaCXX/exceptions.cpp @@ -145,81 +145,3 @@ namespace Decay { } 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 - } - - 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}} - } -} -} diff --git a/test/SemaCXX/unreachable-catch-clauses.cpp b/test/SemaCXX/unreachable-catch-clauses.cpp index 54187138d4..c75067f393 100644 --- a/test/SemaCXX/unreachable-catch-clauses.cpp +++ b/test/SemaCXX/unreachable-catch-clauses.cpp @@ -8,6 +8,7 @@ void f(); void test() try {} -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}} +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}} + -- 2.40.0