[Clang] Remove workaround for libstdc++4.7

[cor3ntin]

We document libstdc++4.8 as the minimum supported version, and we carried a hack for include/tr1/hashtable.h fixed in 4.7.

Cleanup some libstdc++ compatibility comments.

[llvmbot]

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

We document libstdc++4.8 as the minimum supported version, and we carried a hack for include/tr1/hashtable.h fixed in 4.7.

Cleanup some libstdc++ compatibility comments.

---

Full diff: https://github.com/llvm/llvm-project/pull/139693.diff

4 Files Affected:

• (modified) clang/lib/Parse/ParseDecl.cpp (+4-3)
• (modified) clang/lib/Sema/SemaInit.cpp (+9-29)
• (removed) clang/test/SemaCXX/libstdcxx_gets_hack.cpp (-28)
• (removed) clang/test/SemaCXX/libstdcxx_pointer_return_false_hack.cpp (-34)

diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 4fe3565687905..02b3a14134873 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4024,13 +4024,14 @@ void Parser::ParseDeclarationSpecifiers(
     }
 
     case tok::kw___is_signed:
-      // GNU libstdc++ 4.4 uses __is_signed as an identifier, but Clang
-      // typically treats it as a trait. If we see __is_signed as it appears
-      // in libstdc++, e.g.,
+      // HACK: before 2022-12, libstdc++ uses __is_signed as an identifier,
+      // but Clang typically treats it as a trait.
+      // If we see __is_signed as it appears in libstdc++, e.g.,
       //
       //   static const bool __is_signed;
       //
       // then treat __is_signed as an identifier rather than as a keyword.
+      // This was fixed by libstdc++ in December 2022.
       if (DS.getTypeSpecType() == TST_bool &&
           DS.getTypeQualifiers() == DeclSpec::TQ_const &&
           DS.getStorageClassSpec() == DeclSpec::SCS_static)
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index e5670dab03cb0..dc12bacc0158b 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -637,11 +637,11 @@ ExprResult InitListChecker::PerformEmptyInit(SourceLocation Loc,
   }
 
   InitializationSequence InitSeq(SemaRef, Entity, Kind, SubInit);
-  // libstdc++4.6 marks the vector default constructor as explicit in
-  // _GLIBCXX_DEBUG mode, so recover using the C++03 logic in that case.
-  // stlport does so too. Look for std::__debug for libstdc++, and for
-  // std:: for stlport.  This is effectively a compiler-side implementation of
-  // LWG2193.
+  // HACK: libstdc++ prior to 4.9 marks the vector default constructor
+  // as explicit in _GLIBCXX_DEBUG mode, so recover using the C++03 logic
+  // in that case. stlport does so too.
+  // Look for std::__debug for libstdc++, and for std:: for stlport.
+  // This is effectively a compiler-side implementation of LWG2193.
   if (!InitSeq && EmptyInitList && InitSeq.getFailureKind() ==
           InitializationSequence::FK_ExplicitConstructor) {
     OverloadCandidateSet::iterator Best;
@@ -6240,24 +6240,6 @@ static void TryUserDefinedConversion(Sema &S,
   }
 }
 
-/// An egregious hack for compatibility with libstdc++-4.2: in <tr1/hashtable>,
-/// a function with a pointer return type contains a 'return false;' statement.
-/// In C++11, 'false' is not a null pointer, so this breaks the build of any
-/// code using that header.
-///
-/// Work around this by treating 'return false;' as zero-initializing the result
-/// if it's used in a pointer-returning function in a system header.
-static bool isLibstdcxxPointerReturnFalseHack(Sema &S,
-                                              const InitializedEntity &Entity,
-                                              const Expr *Init) {
-  return S.getLangOpts().CPlusPlus11 &&
-         Entity.getKind() == InitializedEntity::EK_Result &&
-         Entity.getType()->isPointerType() &&
-         isa<CXXBoolLiteralExpr>(Init) &&
-         !cast<CXXBoolLiteralExpr>(Init)->getValue() &&
-         S.getSourceManager().isInSystemHeader(Init->getExprLoc());
-}
-
 /// The non-zero enum values here are indexes into diagnostic alternatives.
 enum InvalidICRKind { IIK_okay, IIK_nonlocal, IIK_nonscalar };
 
@@ -6943,12 +6925,10 @@ void InitializationSequence::InitializeFrom(Sema &S,
 
     AddPassByIndirectCopyRestoreStep(DestType, ShouldCopy);
   } else if (ICS.isBad()) {
-    if (isLibstdcxxPointerReturnFalseHack(S, Entity, Initializer))
-      AddZeroInitializationStep(Entity.getType());
-    else if (DeclAccessPair Found;
-             Initializer->getType() == Context.OverloadTy &&
-             !S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
-                                                   /*Complain=*/false, Found))
+    if (DeclAccessPair Found;
+        Initializer->getType() == Context.OverloadTy &&
+        !S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
+                                              /*Complain=*/false, Found))
       SetFailed(InitializationSequence::FK_AddressOfOverloadFailed);
     else if (Initializer->getType()->isFunctionType() &&
              isExprAnUnaddressableFunction(S, Initializer))
diff --git a/clang/test/SemaCXX/libstdcxx_gets_hack.cpp b/clang/test/SemaCXX/libstdcxx_gets_hack.cpp
deleted file mode 100644
index 0d915d01474c3..0000000000000
--- a/clang/test/SemaCXX/libstdcxx_gets_hack.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++14 -verify
-
-// This is a test for an egregious hack in Clang that works around
-// an issue with libstdc++'s detection of whether glibc provides a
-// ::gets function. If there is no ::gets, ignore
-//   using ::gets;
-// in namespace std.
-//
-// See PR18402 and gcc.gnu.org/PR77795 for more details.
-
-#ifdef BE_THE_HEADER
-
-#pragma GCC system_header
-namespace std {
-  using ::gets;
-  using ::getx; // expected-error {{no member named 'getx'}}
-}
-
-#else
-
-#define BE_THE_HEADER
-#include "libstdcxx_pointer_return_false_hack.cpp"
-
-namespace foo {
-  using ::gets; // expected-error {{no member named 'gets'}}
-}
-
-#endif
diff --git a/clang/test/SemaCXX/libstdcxx_pointer_return_false_hack.cpp b/clang/test/SemaCXX/libstdcxx_pointer_return_false_hack.cpp
deleted file mode 100644
index 17e1548ac50d6..0000000000000
--- a/clang/test/SemaCXX/libstdcxx_pointer_return_false_hack.cpp
+++ /dev/null
@@ -1,34 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify
-
-// This is a test for an egregious hack in Clang that works around
-// an issue with libstdc++-4.2's <tr1/hashtable> implementation.
-// The code in question returns 'false' from a function with a pointer
-// return type, which is ill-formed in C++11.
-
-#ifdef BE_THE_HEADER
-
-#pragma GCC system_header
-namespace std {
-  namespace tr1 {
-    template<typename T> struct hashnode;
-    template<typename T> struct hashtable {
-      typedef hashnode<T> node;
-      node *find_node() {
-        // This is ill-formed in C++11, per core issue 903, but we accept
-        // it anyway in a system header.
-        return false;
-      }
-    };
-  }
-}
-
-#else
-
-#define BE_THE_HEADER
-#include "libstdcxx_pointer_return_false_hack.cpp"
-
-auto *test1 = std::tr1::hashtable<int>().find_node();
-
-void *test2() { return false; } // expected-error {{cannot initialize}}
-
-#endif

[AaronBallman]

We document libstdc++4.8 as the minimum supported version

It took me quite a while to find that documentation, we probably should make this a bit more prominent from the getting started page at some point:

llvm-project/clang/docs/Toolchain.rst

Line 343 in c14acb7

`libstdc++ <https://gcc.gnu.org/onlinedocs/libstdc++/>`_ is GCC's

[AaronBallman]

The changes should come with a release note mentioning that we've removed the hack because it's no longer a supported libstdc++ (for the few users still relying on the hack).

Otherwise, LGTM!

[cor3ntin]

@AaronBallman We previously removed other such hacks, do you think a release note
is still useful 6ad7e87 ?

[AaronBallman]

@AaronBallman We previously removed other such hacks, do you think a release note is still useful 6ad7e87 ?

Yeah, I think a release note is still handy, just so users aren't surprised.