[clang] remove old HACK for libstdc++ std::swap on exception specification bug

[mizvekov]

Summary

This removes a workaround for a libstdc++ bug where we need to eagerly
parse the exception specification.

libstdc++ had a bug where it shadows std::swap with a
member swap function then tries to call std::swap unqualified from the
exception specification of that function.

We would workaround that by detecting whether we were in such a case
and turn off delay-parsing of exception specifications.

Libstdc++ 6.1 (released 2016-04-27) appears to have resolved it as side-effect
of commit ddb63209a8d (2015-06-05).

GCC 10.1, released on May 7, 2020, appears to parse the exception
specification correctly, and does not include any sort of workaround for
std:: / system headers.

Signed-off-by: Matheus Izvekov <[e-mail]>

[mizvekov]

.

clang/include/clang/Sema/Sema.h

public:
   void checkExceptionSpecification(bool IsTopLevel,
                                    ExceptionSpecificationType EST,
                                    ArrayRef<ParsedType> DynamicExceptions,
                                    ArrayRef<SourceRange> DynamicExceptionRanges,
                                    Expr *NoexceptExpr,
                                    SmallVectorImpl<QualType> &Exceptions,
                                    FunctionProtoType::ExceptionSpecInfo &ESI);
 
-  /// Determine if we're in a case where we need to (incorrectly) eagerly
-  /// parse an exception specification to work around a libstdc++ bug.
-  bool isLibstdcxxEagerExceptionSpecHack(const Declarator &D);
-
   /// Add an exception-specification to the given member function
   /// (or member function template). The exception-specification was parsed
   /// after the method itself was declared.
   void actOnDelayedExceptionSpecification(Decl *Method,
          ExceptionSpecificationType EST,
          SourceRange SpecificationRange,
          ArrayRef<ParsedType> DynamicExceptions,
          ArrayRef<SourceRange> DynamicExceptionRanges,

clang/lib/Parse/ParseDecl.cpp

if (getLangOpts().CPlusPlus) {
 
       // Parse exception-specification[opt].
       // FIXME: Per [class.mem]p6, all exception-specifications at class scope
       // should be delayed, including those for non-members (eg, friend
       // declarations). But only applying this to member declarations is
       // consistent with what other implementations do.
       bool Delayed = D.isFirstDeclarationOfMember() &&
                      D.isFunctionDeclaratorAFunctionDeclaration();
-      if (Delayed && Actions.isLibstdcxxEagerExceptionSpecHack(D) &&
-          GetLookAheadToken(0).is(tok::kw_noexcept) &&
-          GetLookAheadToken(1).is(tok::l_paren) &&
-          GetLookAheadToken(2).is(tok::kw_noexcept) &&
-          GetLookAheadToken(3).is(tok::l_paren) &&
-          GetLookAheadToken(4).is(tok::identifier) &&
-          GetLookAheadToken(4).getIdentifierInfo()->isStr("swap")) {
-        // HACK: We've got an exception-specification
-        //   noexcept(noexcept(swap(...)))
-        // or
-        //   noexcept(noexcept(swap(...)) && noexcept(swap(...)))
-        // on a 'swap' member function. This is a libstdc++ bug; the lookup
-        // for 'swap' will only find the function we're currently declaring,
-        // whereas it expects to find a non-member swap through ADL. Turn off
-        // delayed parsing to give it a chance to find what it expects.
-        Delayed = false;
-      }
       ESpecType = tryParseExceptionSpecification(Delayed,
                                                  ESpecRange,
                                                  DynamicExceptions,
                                                  DynamicExceptionRanges,
                                                  NoexceptExpr,
                                                  ExceptionSpecTokens);
       if (ESpecType != EST_None)
         EndLoc = ESpecRange.getEnd();

clang/lib/Sema/SemaExceptionSpec.cpp

if (const PointerType *PtrTy = T->getAs<PointerType>())
     T = PtrTy->getPointeeType();
   else if (const ReferenceType *RefTy = T->getAs<ReferenceType>())
     T = RefTy->getPointeeType();
   else if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>())
     T = MPTy->getPointeeType();
   return T->getAs<FunctionProtoType>();
 }
 
-/// HACK: 2014-11-14 libstdc++ had a bug where it shadows std::swap with a
-/// member swap function then tries to call std::swap unqualified from the
-/// exception specification of that function. This function detects whether
-/// we're in such a case and turns off delay-parsing of exception
-/// specifications. Libstdc++ 6.1 (released 2016-04-27) appears to have
-/// resolved it as side-effect of commit ddb63209a8d (2015-06-05).
-bool Sema::isLibstdcxxEagerExceptionSpecHack(const Declarator &D) {
-  auto *RD = dyn_cast<CXXRecordDecl>(CurContext);
-
-  // All the problem cases are member functions named "swap" within class
-  // templates declared directly within namespace std or std::__debug or
-  // std::__profile.
-  if (!RD || !RD->getIdentifier() || !RD->getDescribedClassTemplate() ||
-      !D.getIdentifier() || !D.getIdentifier()->isStr("swap"))
-    return false;
-
-  auto *ND = dyn_cast<NamespaceDecl>(RD->getDeclContext());
-  if (!ND)
-    return false;
-
-  bool IsInStd = ND->isStdNamespace();
-  if (!IsInStd) {
-    // This isn't a direct member of namespace std, but it might still be
-    // libstdc++'s std::__debug::array or std::__profile::array.
-    IdentifierInfo *II = ND->getIdentifier();
-    if (!II || !(II->isStr("__debug") || II->isStr("__profile")) ||
-        !ND->isInStdNamespace())
-      return false;
-  }
-
-  // Only apply this hack within a system header.
-  if (!Context.getSourceManager().isInSystemHeader(D.getBeginLoc()))
-    return false;
-
-  return llvm::StringSwitch<bool>(RD->getIdentifier()->getName())
-      .Case("array", true)
-      .Case("pair", IsInStd)
-      .Case("priority_queue", IsInStd)
-      .Case("stack", IsInStd)
-      .Case("queue", IsInStd)
-      .Default(false);
-}
-
 ExprResult Sema::ActOnNoexceptSpec(SourceLocation NoexceptLoc,
                                    Expr *NoexceptExpr,
                                    ExceptionSpecificationType &EST) {
   // FIXME: This is bogus, a noexcept expression is not a condition.
   ExprResult Converted = CheckBooleanCondition(NoexceptLoc, NoexceptExpr);
   if (Converted.isInvalid()) {
     EST = EST_NoexceptFalse;
 

clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp

-// This is a test for an egregious hack in Clang that works around
-// an issue with GCC's <utility> implementation. std::pair::swap
-// has an exception specification that makes an unqualified call to
-// swap. This is invalid, because it ends up calling itself with
-// the wrong number of arguments.
-//
-// The same problem afflicts a bunch of other class templates. Those
-// affected are array, pair, priority_queue, stack, and queue.
-
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array -DPR28423
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=pair
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=priority_queue
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=stack
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=queue
-//
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array -DNAMESPACE=__debug
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array -DNAMESPACE=__profile
-
-// MSVC's standard library uses a very similar pattern that relies on delayed
-// parsing of exception specifications.
-//
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array -DMSVC
-
-#ifdef BE_THE_HEADER
-
-#pragma GCC system_header
-#ifdef PR28423
-using namespace std;
-#endif
-
-namespace std {
-  template<typename T> void swap(T &, T &);
-  template<typename T> void do_swap(T &a, T &b) noexcept(noexcept(swap(a, b))) {
-    swap(a, b);
-  }
-
-#ifdef NAMESPACE
-  namespace NAMESPACE {
-#define STD_CLASS std::NAMESPACE::CLASS
-#else
-#define STD_CLASS std::CLASS
-#endif
-
-  template<typename A, typename B> struct CLASS {
-#ifdef MSVC
-    void swap(CLASS &other) noexcept(noexcept(do_swap(member, other.member)));
-#endif
-    A member;
-#ifndef MSVC
-    void swap(CLASS &other) noexcept(noexcept(swap(member, other.member)));
-#endif
-  };
-
-//  template<typename T> void do_swap(T &, T &);
-//  template<typename A> struct vector {
-//    void swap(vector &other) noexcept(noexcept(do_swap(member, other.member)));
-//    A member;
-//  };
-
-#ifdef NAMESPACE
-  }
-#endif
-}
-
-#else
-
-#define BE_THE_HEADER
-#include __FILE__
-
-struct X {};
-using PX = STD_CLASS<X, X>;
-using PI = STD_CLASS<int, int>;
-void swap(X &, X &) noexcept;
-PX px;
-PI pi;
-
-static_assert(noexcept(px.swap(px)), "");
-static_assert(!noexcept(pi.swap(pi)), "");
-
-namespace sad {
-  template<typename T> void swap(T &, T &);
-
-  template<typename A, typename B> struct CLASS {
-    void swap(CLASS &other) noexcept(noexcept(swap(*this, other))); // expected-error {{too many arguments}} expected-note {{declared here}}
-    // expected-error@-1{{uses itself}} expected-note@-1{{in instantiation of}}
-  };
-
-  CLASS<int, int> pi;
-
-  static_assert(!noexcept(pi.swap(pi)), ""); // expected-note 2{{in instantiation of exception specification for 'swap'}}
-}
-
-#endif