DR126: partially implement the const-correct rules for exception handler matching.

While here, fix up the myriad other ways in which Sema's two "can this handler
catch that exception?" implementations get things wrong and unify them.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@322431 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Sema/Sema.h

@@ -1459,6 +1459,7 @@ public:
       const PartialDiagnostic &DiagID, const PartialDiagnostic & NoteID,
       const FunctionProtoType *Old, SourceLocation OldLoc,
       const FunctionProtoType *New, SourceLocation NewLoc);
+  bool handlerCanCatch(QualType HandlerType, QualType ExceptionType);
   bool CheckExceptionSpecSubset(const PartialDiagnostic &DiagID,
                                 const PartialDiagnostic &NestedDiagID,
                                 const PartialDiagnostic &NoteID,

lib/Sema/AnalysisBasedWarnings.cpp

@@ -287,48 +287,21 @@ enum ThrowState {
   FoundPathWithNoThrowOutFunction,
 };
 
-static bool isThrowCaught(const CXXThrowExpr *Throw,
-                          const CXXCatchStmt *Catch) {
-  const Type *CaughtType = Catch->getCaughtType().getTypePtrOrNull();
-  if (!CaughtType)
-    return true;
-  const Type *ThrowType = nullptr;
-  if (Throw->getSubExpr())
-    ThrowType = Throw->getSubExpr()->getType().getTypePtrOrNull();
-  if (!ThrowType)
-    return false;
-  if (ThrowType->isReferenceType())
-    ThrowType = ThrowType->castAs<ReferenceType>()
-                    ->getPointeeType()
-                    ->getUnqualifiedDesugaredType();
-  if (CaughtType->isReferenceType())
-    CaughtType = CaughtType->castAs<ReferenceType>()
-                     ->getPointeeType()
-                     ->getUnqualifiedDesugaredType();
-  if (ThrowType->isPointerType() && CaughtType->isPointerType()) {
-    ThrowType = ThrowType->getPointeeType()->getUnqualifiedDesugaredType();
-    CaughtType = CaughtType->getPointeeType()->getUnqualifiedDesugaredType();
-  }
-  if (CaughtType == ThrowType)
-    return true;
-  const CXXRecordDecl *CaughtAsRecordType =
-      CaughtType->getAsCXXRecordDecl();
-  const CXXRecordDecl *ThrowTypeAsRecordType = ThrowType->getAsCXXRecordDecl();
-  if (CaughtAsRecordType && ThrowTypeAsRecordType)
-    return ThrowTypeAsRecordType->isDerivedFrom(CaughtAsRecordType);
-  return false;
-}
-
-static bool isThrowCaughtByHandlers(const CXXThrowExpr *CE,
+static bool isThrowCaughtByHandlers(Sema &S,
+                                    const CXXThrowExpr *CE,
                                     const CXXTryStmt *TryStmt) {
   for (unsigned H = 0, E = TryStmt->getNumHandlers(); H < E; ++H) {
-    if (isThrowCaught(CE, TryStmt->getHandler(H)))
+    QualType Caught = TryStmt->getHandler(H)->getCaughtType();
+    if (Caught.isNull() || // catch (...) catches everything
+        (CE->getSubExpr() && // throw; is only caught by ...
+         S.handlerCanCatch(Caught, CE->getSubExpr()->getType())))
       return true;
   }
   return false;
 }
 
-static bool doesThrowEscapePath(CFGBlock Block, SourceLocation &OpLoc) {
+static bool doesThrowEscapePath(Sema &S, CFGBlock Block,
+                                SourceLocation &OpLoc) {
   for (const auto &B : Block) {
     if (B.getKind() != CFGElement::Statement)
       continue;
@@ -342,7 +315,7 @@ static bool doesThrowEscapePath(CFGBlock Block, SourceLocation &OpLoc) {
         continue;
       if (const auto *Terminator =
               dyn_cast_or_null<CXXTryStmt>(I->getTerminator()))
-        if (isThrowCaughtByHandlers(CE, Terminator))
+        if (isThrowCaughtByHandlers(S, CE, Terminator))
           return false;
     }
     return true;
@@ -350,8 +323,8 @@ static bool doesThrowEscapePath(CFGBlock Block, SourceLocation &OpLoc) {
   return false;
 }
 
-static bool hasThrowOutNonThrowingFunc(SourceLocation &OpLoc, CFG *BodyCFG) {
-
+static bool hasThrowOutNonThrowingFunc(Sema &S, SourceLocation &OpLoc,
+                                       CFG *BodyCFG) {
   unsigned ExitID = BodyCFG->getExit().getBlockID();
 
   SmallVector<ThrowState, 16> States(BodyCFG->getNumBlockIDs(),
@@ -369,7 +342,7 @@ static bool hasThrowOutNonThrowingFunc(SourceLocation &OpLoc, CFG *BodyCFG) {
       if (ExitID == ID)
         continue;
 
-      if (doesThrowEscapePath(*CurBlock, OpLoc))
+      if (doesThrowEscapePath(S, *CurBlock, OpLoc))
         CurState = FoundPathForThrow;
     }
 
@@ -419,7 +392,7 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD,
   if (BodyCFG->getExit().pred_empty())
     return;
   SourceLocation OpLoc;
-  if (hasThrowOutNonThrowingFunc(OpLoc, BodyCFG))
+  if (hasThrowOutNonThrowingFunc(S, OpLoc, BodyCFG))
     EmitDiagForCXXThrowInNonThrowingFunc(S, OpLoc, FD);
 }
 

lib/Sema/SemaExceptionSpec.cpp

@@ -626,6 +626,90 @@ bool Sema::CheckEquivalentExceptionSpec(const PartialDiagnostic &DiagID,
                                           New, NewLoc);
 }
 
+bool Sema::handlerCanCatch(QualType HandlerType, QualType ExceptionType) {
+  // [except.handle]p3:
+  //   A handler is a match for an exception object of type E if:
+
+  // HandlerType must be ExceptionType or derived from it, or pointer or
+  // reference to such types.
+  const ReferenceType *RefTy = HandlerType->getAs<ReferenceType>();
+  if (RefTy)
+    HandlerType = RefTy->getPointeeType();
+
+  //   -- the handler is of type cv T or cv T& and E and T are the same type
+  if (Context.hasSameUnqualifiedType(ExceptionType, HandlerType))
+    return true;
+
+  // FIXME: ObjC pointer types?
+  if (HandlerType->isPointerType() || HandlerType->isMemberPointerType()) {
+    if (RefTy && (!HandlerType.isConstQualified() ||
+                  HandlerType.isVolatileQualified()))
+      return false;
+
+    // -- the handler is of type cv T or const T& where T is a pointer or
+    //    pointer to member type and E is std::nullptr_t
+    if (ExceptionType->isNullPtrType())
+      return true;
+
+    // -- the handler is of type cv T or const T& where T is a pointer or
+    //    pointer to member type and E is a pointer or pointer to member type
+    //    that can be converted to T by one or more of
+    //    -- a qualification conversion
+    //    -- a function pointer conversion
+    bool LifetimeConv;
+    QualType Result;
+    // FIXME: Should we treat the exception as catchable if a lifetime
+    // conversion is required?
+    if (IsQualificationConversion(ExceptionType, HandlerType, false,
+                                  LifetimeConv) ||
+        IsFunctionConversion(ExceptionType, HandlerType, Result))
+      return true;
+
+    //    -- a standard pointer conversion [...]
+    if (!ExceptionType->isPointerType() || !HandlerType->isPointerType())
+      return false;
+
+    // Handle the "qualification conversion" portion.
+    Qualifiers EQuals, HQuals;
+    ExceptionType = Context.getUnqualifiedArrayType(
+        ExceptionType->getPointeeType(), EQuals);
+    HandlerType = Context.getUnqualifiedArrayType(
+        HandlerType->getPointeeType(), HQuals);
+    if (!HQuals.compatiblyIncludes(EQuals))
+      return false;
+
+    if (HandlerType->isVoidType() && ExceptionType->isObjectType())
+      return true;
+
+    // The only remaining case is a derived-to-base conversion.
+  }
+
+  //   -- the handler is of type cg T or cv T& and T is an unambiguous public
+  //      base class of E
+  if (!ExceptionType->isRecordType() || !HandlerType->isRecordType())
+    return false;
+  CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+                     /*DetectVirtual=*/false);
+  if (!IsDerivedFrom(SourceLocation(), ExceptionType, HandlerType, Paths) ||
+      Paths.isAmbiguous(Context.getCanonicalType(HandlerType)))
+    return false;
+
+  // Do this check from a context without privileges.
+  switch (CheckBaseClassAccess(SourceLocation(), HandlerType, ExceptionType,
+                               Paths.front(),
+                               /*Diagnostic*/ 0,
+                               /*ForceCheck*/ true,
+                               /*ForceUnprivileged*/ true)) {
+  case AR_accessible: return true;
+  case AR_inaccessible: return false;
+  case AR_dependent:
+    llvm_unreachable("access check dependent for unprivileged context");
+  case AR_delayed:
+    llvm_unreachable("access check delayed in non-declaration");
+  }
+  llvm_unreachable("unexpected access check result");
+}
+
 /// CheckExceptionSpecSubset - Check whether the second function type's
 /// exception specification is a subset (or equivalent) of the first function
 /// type. This is used by override and pointer assignment checks.
@@ -722,75 +806,23 @@ bool Sema::CheckExceptionSpecSubset(const PartialDiagnostic &DiagID,
          "Exception spec subset: non-dynamic case slipped through.");
 
   // Neither contains everything or nothing. Do a proper comparison.
-  for (const auto &SubI : Subset->exceptions()) {
-    // Take one type from the subset.
-    QualType CanonicalSubT = Context.getCanonicalType(SubI);
-    // Unwrap pointers and references so that we can do checks within a class
-    // hierarchy. Don't unwrap member pointers; they don't have hierarchy
-    // conversions on the pointee.
-    bool SubIsPointer = false;
-    if (const ReferenceType *RefTy = CanonicalSubT->getAs<ReferenceType>())
-      CanonicalSubT = RefTy->getPointeeType();
-    if (const PointerType *PtrTy = CanonicalSubT->getAs<PointerType>()) {
-      CanonicalSubT = PtrTy->getPointeeType();
-      SubIsPointer = true;
-    }
-    bool SubIsClass = CanonicalSubT->isRecordType();
-    CanonicalSubT = CanonicalSubT.getLocalUnqualifiedType();
-
-    CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
-                       /*DetectVirtual=*/false);
+  for (QualType SubI : Subset->exceptions()) {
+    if (const ReferenceType *RefTy = SubI->getAs<ReferenceType>())
+      SubI = RefTy->getPointeeType();
 
-    bool Contained = false;
     // Make sure it's in the superset.
-    for (const auto &SuperI : Superset->exceptions()) {
-      QualType CanonicalSuperT = Context.getCanonicalType(SuperI);
-      // SubT must be SuperT or derived from it, or pointer or reference to
-      // such types.
-      if (const ReferenceType *RefTy = CanonicalSuperT->getAs<ReferenceType>())
-        CanonicalSuperT = RefTy->getPointeeType();
-      if (SubIsPointer) {
-        if (const PointerType *PtrTy = CanonicalSuperT->getAs<PointerType>())
-          CanonicalSuperT = PtrTy->getPointeeType();
-        else {
-          continue;
-        }
-      }
-      CanonicalSuperT = CanonicalSuperT.getLocalUnqualifiedType();
-      // If the types are the same, move on to the next type in the subset.
-      if (CanonicalSubT == CanonicalSuperT) {
+    bool Contained = false;
+    for (QualType SuperI : Superset->exceptions()) {
+      // [except.spec]p5:
+      //   the target entity shall allow at least the exceptions allowed by the
+      //   source
+      //
+      // We interpret this as meaning that a handler for some target type would
+      // catch an exception of each source type.
+      if (handlerCanCatch(SuperI, SubI)) {
         Contained = true;
         break;
       }
-
-      // Otherwise we need to check the inheritance.
-      if (!SubIsClass || !CanonicalSuperT->isRecordType())
-        continue;
-
-      Paths.clear();
-      if (!IsDerivedFrom(SubLoc, CanonicalSubT, CanonicalSuperT, Paths))
-        continue;
-
-      if (Paths.isAmbiguous(Context.getCanonicalType(CanonicalSuperT)))
-        continue;
-
-      // Do this check from a context without privileges.
-      switch (CheckBaseClassAccess(SourceLocation(),
-                                   CanonicalSuperT, CanonicalSubT,
-                                   Paths.front(),
-                                   /*Diagnostic*/ 0,
-                                   /*ForceCheck*/ true,
-                                   /*ForceUnprivileged*/ true)) {
-      case AR_accessible: break;
-      case AR_inaccessible: continue;
-      case AR_dependent:
-        llvm_unreachable("access check dependent for unprivileged context");
-      case AR_delayed:
-        llvm_unreachable("access check delayed in non-declaration");
-      }
-
-      Contained = true;
-      break;
     }
     if (!Contained) {
       Diag(SubLoc, DiagID);

test/CXX/drs/dr1xx.cpp

@@ -238,7 +238,20 @@ namespace dr125 {
   };
 }
 
-namespace dr126 { // dr126: no
+namespace dr126 { // dr126: partial
+  // FIXME: We do not yet generate correct code for this change:
+  // eg:
+  //   catch (void*&) should catch void* but not int*
+  //   catch (void*) and catch (void*const&) should catch both
+  // Likewise:
+  //   catch (Base *&) should catch Base* but not Derived*
+  //   catch (Base *) should catch both
+  // In each case, we emit the same code for both catches.
+  //
+  // The ABI does not let us represent the language rule in the unwind tables.
+  // So, when catching by non-const (or volatile) reference to pointer, we
+  // should compare the exception type to the caught type and only accept an
+  // exact match.
 #if __cplusplus <= 201402L
   struct C {};
   struct D : C {};
@@ -262,12 +275,13 @@ namespace dr126 { // dr126: no
     virtual void gr() throw(C&);
     virtual void hr() throw(C&); // expected-note {{overridden}}
 
-    virtual void pv() throw(void*); // expected-note {{overridden}}
+    virtual void pv() throw(void*);
 
 #if __cplusplus >= 201103L
-    virtual void np() throw(C*); // expected-note {{overridden}}
-    virtual void npm() throw(int C::*); // expected-note {{overridden}}
-    virtual void nr() throw(C&); // expected-note {{overridden}}
+    virtual void np() throw(C*);
+    virtual void npm() throw(int C::*);
+    virtual void nr() throw(C*&); // expected-note {{overridden}}
+    virtual void ncr() throw(C*const&);
 #endif
 
     virtual void ref1() throw(C *const&);
@@ -275,7 +289,7 @@ namespace dr126 { // dr126: no
 
     virtual void v() throw(int);
     virtual void w() throw(const int);
-    virtual void x() throw(int*);
+    virtual void x() throw(int*); // expected-note {{overridden}}
     virtual void y() throw(const int*);
     virtual void z() throw(int); // expected-note {{overridden}}
   };
@@ -294,13 +308,14 @@ namespace dr126 { // dr126: no
     virtual void gr() throw(G&);
     virtual void hr() throw(H&); // expected-error {{more lax}}
 
-    virtual void pv() throw(C*); // expected-error {{more lax}} FIXME: This is valid.
+    virtual void pv() throw(C*);
 
 #if __cplusplus >= 201103L
     using nullptr_t = decltype(nullptr);
-    virtual void np() throw(nullptr_t*); // expected-error {{more lax}} FIXME: This is valid.
-    virtual void npm() throw(nullptr_t*); // expected-error {{more lax}} FIXME: This is valid.
-    virtual void nr() throw(nullptr_t&); // expected-error {{more lax}} This is not.
+    virtual void np() throw(nullptr_t);
+    virtual void npm() throw(nullptr_t&);
+    virtual void nr() throw(nullptr_t); // expected-error {{more lax}}
+    virtual void ncr() throw(nullptr_t);
 #endif
 
     virtual void ref1() throw(D *const &);
@@ -308,7 +323,7 @@ namespace dr126 { // dr126: no
 
     virtual void v() throw(const int);
     virtual void w() throw(int);
-    virtual void x() throw(const int*); // FIXME: 'const int*' is not allowed by A::h.
+    virtual void x() throw(const int*); // expected-error {{more lax}}
     virtual void y() throw(int*); // ok
     virtual void z() throw(long); // expected-error {{more lax}}
   };

test/SemaCXX/warn-throw-out-noexcept-func.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s  -fdelayed-template-parsing -fcxx-exceptions -fsyntax-only -Wexceptions -verify -fdeclspec -std=c++11
+// RUN: %clang_cc1 %s  -fdelayed-template-parsing -fcxx-exceptions -fsyntax-only -Wexceptions -verify -fdeclspec -std=c++17
 struct A_ShouldDiag {
   ~A_ShouldDiag(); // implicitly noexcept(true)
 };
@@ -325,3 +325,84 @@ int main() {
   ShouldDiagnose obj;
   ShouldNotDiagnose obj1;
 }
+
+namespace ExceptionInNamespace {
+  namespace N {
+    struct E {};
+  }
+  void run() throw() {
+    try {
+      throw N::E();
+    } catch (const N::E &e) {
+    }
+  }
+}
+
+namespace HandlerSpecialCases {
+  struct A {};
+  using CA = const A;
+
+  struct B : A {};
+  using CB = const B;
+
+  struct AmbigBase {};
+  struct AmbigMiddle : AmbigBase {};
+  struct AmbigDerived : AmbigBase, AmbigMiddle {}; // expected-warning {{inaccessible}}
+
+  struct PrivateBase {};
+  struct PrivateDerived : private PrivateBase { friend void bad3() throw(); };
+
+  void good() throw() {
+    try { throw CA(); } catch (volatile A&) {}
+    try { throw B(); } catch (A&) {}
+    try { throw B(); } catch (const volatile A&) {}
+    try { throw CB(); } catch (A&) {}
+    try { throw (int*)0; } catch (void* const volatile) {}
+    try { throw (int*)0; } catch (void* const &) {}
+    try { throw (B*)0; } catch (A*) {}
+    try { throw (B*)0; } catch (A* const &) {}
+    try { throw (void(*)() noexcept)0; } catch (void (*)()) {}
+    try { throw (void(*)() noexcept)0; } catch (void (*const &)()) {}
+    try { throw (int**)0; } catch (const int * const*) {}
+    try { throw (int**)0; } catch (const int * const* const&) {}
+    try { throw nullptr; } catch (int*) {}
+    try { throw nullptr; } catch (int* const&) {}
+  }
+
+  void bad1() throw() { // expected-note {{here}}
+    try { throw A(); } catch (const B&) {} // expected-warning {{still throw}}
+  }
+  void bad2() throw() { // expected-note {{here}}
+    try { throw AmbigDerived(); } catch (const AmbigBase&) {} // expected-warning {{still throw}}
+  }
+  void bad3() throw() { // expected-note {{here}}
+    try { throw PrivateDerived(); } catch (const PrivateBase&) {} // expected-warning {{still throw}}
+  }
+  void bad4() throw() { // expected-note {{here}}
+    try { throw (int*)0; } catch (void* &) {} // expected-warning {{still throw}}
+  }
+  void bad5() throw() { // expected-note {{here}}
+    try { throw (int*)0; } catch (void* const volatile &) {} // expected-warning {{still throw}}
+  }
+  void bad6() throw() { // expected-note {{here}}
+    try { throw (int* volatile)0; } catch (void* const volatile &) {} // expected-warning {{still throw}}
+  }
+  void bad7() throw() { // expected-note {{here}}
+    try { throw (AmbigDerived*)0; } catch (AmbigBase*) {} // expected-warning {{still throw}}
+  }
+  void bad8() throw() { // expected-note {{here}}
+    try { throw (PrivateDerived*)0; } catch (PrivateBase*) {} // expected-warning {{still throw}}
+  }
+  void bad9() throw() { // expected-note {{here}}
+    try { throw (B*)0; } catch (A* &) {} // expected-warning {{still throw}}
+  }
+  void bad10() throw() { // expected-note {{here}}
+    try { throw (void(*)())0; } catch (void (*)() noexcept) {} // expected-warning {{still throw}}
+  }
+  void bad11() throw() { // expected-note {{here}}
+    try { throw (int**)0; } catch (const int **) {} // expected-warning {{still throw}}
+  }
+  void bad12() throw() { // expected-note {{here}}
+    try { throw nullptr; } catch (int) {} // expected-warning {{still throw}}
+  }
+}

www/cxx_dr_status.html

@@ -795,7 +795,7 @@
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#126">126</a></td>
     <td>TC1</td>
     <td>Exception specifications and <TT>const</TT></td>
-    <td class="none" align="center">No</td>
+    <td class="partial" align="center">Partial</td>
   </tr>
   <tr id="127">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#127">127</a></td>
@@ -1292,7 +1292,7 @@
   <tr id="209">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#209">209</a></td>
     <td>NAD</td>
-    <td>Must friend declaration names be
+    <td>Must friend declaration names be
 accessible?</td>
     <td class="full" align="center">Yes</td>
   </tr>
@@ -1653,7 +1653,7 @@ accessible?</td>
   <tr id="269">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#269">269</a></td>
     <td>NAD</td>
-    <td>Order of initialization of multiply-defined static data members
+    <td>Order of initialization of multiply-defined static data members
 of class templates</td>
     <td class="na" align="center">N/A</td>
   </tr>
@@ -3268,8 +3268,8 @@ of class templates</td>
   <tr id="538">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#538">538</a></td>
     <td>CD1</td>
-    <td>Definition and usage
-of <I>structure</I>, <I>POD-struct</I>, <I>POD-union</I>,
+    <td>Definition and usage
+of <I>structure</I>, <I>POD-struct</I>, <I>POD-union</I>,
 and <I>POD class</I></td>
     <td class="na" align="center">N/A</td>
   </tr>
@@ -6493,7 +6493,7 @@ and <I>POD class</I></td>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1113">1113</a></td>
     <td>C++11</td>
     <td>Linkage of namespace member of unnamed namespace</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="partial" align="center">Partial</td>
   </tr>
   <tr id="1114">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1114">1114</a></td>