r229338 - Partial revert of r229336; this wasn't intended to go in.

Aaron Ballman aaron at aaronballman.com
Sun Feb 15 14:18:04 PST 2015


Author: aaronballman
Date: Sun Feb 15 16:18:04 2015
New Revision: 229338

URL: http://llvm.org/viewvc/llvm-project?rev=229338&view=rev
Log:
Partial revert of r229336; this wasn't intended to go in.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaStmt.cpp
    cfe/trunk/test/CXX/drs/dr3xx.cpp
    cfe/trunk/test/Misc/warning-flags.c
    cfe/trunk/test/SemaCXX/exceptions.cpp
    cfe/trunk/test/SemaCXX/unreachable-catch-clauses.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=229338&r1=229337&r2=229338&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Sun Feb 15 16:18:04 2015
@@ -106,8 +106,6 @@ def Documentation : DiagGroup<"documenta
                                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">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=229338&r1=229337&r2=229338&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Feb 15 16:18:04 2015
@@ -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<Exceptions>;
+  "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">;

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=229338&r1=229337&r2=229338&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Sun Feb 15 16:18:04 2015
@@ -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(Sourc
   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<QualTypeExt>;
-  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;
-  }
-};
+  TypeWithHandler(const QualType &type, CXXCatchStmt *statement)
+  : t(type), stmt(statement) {}
 
-namespace llvm {
-template <> struct DenseMapInfo<QualTypeExt> {
-  static QualTypeExt getEmptyKey() {
-    return QualTypeExt(DenseMapInfo<QualType>::getEmptyKey(),
-                       QualTypeExt::ForDenseMap);
-  }
-
-  static QualTypeExt getTombstoneKey() {
-    return QualTypeExt(DenseMapInfo<QualType>::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<QualType>::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<QualTypeExt> { static const bool value = true; };
-}
-
-static bool Frobble(const CXXBaseSpecifier *, CXXBasePath &Path, void *User) {
-  auto *Paths = reinterpret_cast<CXXBasePaths *>(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(Source
   }
 
   const unsigned NumHandlers = Handlers.size();
-  assert(!Handlers.empty() &&
+  assert(NumHandlers > 0 &&
          "The parser shouldn't call this if there are no handlers.");
 
-  llvm::DenseMap<QualTypeExt, CXXCatchStmt *> HandledTypes;
+  SmallVector<TypeWithHandler, 8> TypesWithHandlers;
+
   for (unsigned i = 0; i < NumHandlers; ++i) {
     CXXCatchStmt *Handler = cast<CXXCatchStmt>(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);
 }
 

Modified: cfe/trunk/test/CXX/drs/dr3xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr3xx.cpp?rev=229338&r1=229337&r2=229338&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr3xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr3xx.cpp Sun Feb 15 16:18:04 2015
@@ -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
     }
   }

Modified: cfe/trunk/test/Misc/warning-flags.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/warning-flags.c?rev=229338&r1=229337&r2=229338&view=diff
==============================================================================
--- cfe/trunk/test/Misc/warning-flags.c (original)
+++ cfe/trunk/test/Misc/warning-flags.c Sun Feb 15 16:18:04 2015
@@ -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_inc
 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

Modified: cfe/trunk/test/SemaCXX/exceptions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/exceptions.cpp?rev=229338&r1=229337&r2=229338&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/exceptions.cpp (original)
+++ cfe/trunk/test/SemaCXX/exceptions.cpp Sun Feb 15 16:18:04 2015
@@ -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}}
-  }
-}
-}

Modified: cfe/trunk/test/SemaCXX/unreachable-catch-clauses.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/unreachable-catch-clauses.cpp?rev=229338&r1=229337&r2=229338&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/unreachable-catch-clauses.cpp (original)
+++ cfe/trunk/test/SemaCXX/unreachable-catch-clauses.cpp Sun Feb 15 16:18:04 2015
@@ -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}}
+





More information about the cfe-commits mailing list