[PATCH] clang: Remove libstdc++ gets hack

[urnathan]

Summary

This hack is a libstdc++ 4.8 workaround to do with gets. It was added
by 46d04a331c1ee Richard Smith, Sun Jan 8 04:01:15 2017 +0000. The
libstdc++ bug was fixed in 2016 for gcc 6.3 (released Dec2016) and
later.

This is more recent than the other libstdc++ hackectomy, but still over 4 years with a fix. WDYT?

[aaron.ballman]

I think it's reasonable to remove this hack as well, but I have similar questions about whether we should note it in the release notes and whether there's any compatibility statement we need to update elsewhere.

clang/test/SemaCXX/libstdcxx_gets_hack.cpp

11

[rsmith]

I think we should decide what version of libstdc++ we're prepared to support and document that somewhere, and it would make sense to remove the various libstdc++ compatibility changes that target older versions. Google still has builds that use Clang trunk, recent glibc, and libstdc++ 4.9, in >=C++14 mode, though (which I think is the combination that needs this workaround), so I'm not sure that the world is ready for Clang's minimum to be increased to 6.3 or later. (Of course Google could locally patch their libstdc++, but if there's one organization building in this mode still, it seems likely that there are others, and in particular version 4.9 was the last version prior to the introduction of the dual ABI, so there may be reasons people want to stop there in particular. I think at least the need to support libstdc++ 4.2 for Apple targets has disappeared now, though, so it's possible that we could drop support for 4.8 and earlier?)

If we do keep this hack, I think it would make sense to update the comments to refer to "specific old versions of libstdc++" (ideally specifying which ones) rather than talking inaccurately about libstdc++ in general. Though perhaps even describing this as a libstdc++ workaround is inappropriate -- wasn't this issue caused by glibc removing the function in C++ compilations, while it was still part of the C++ standard?

[urnathan]

yeah, I guess that would be a little agressive. See https://reviews.llvm.org/D100680 for a comment update

clang/lib/Sema/SemaDeclCXX.cpp

   LookupQualifiedName(R, LookupContext);
 
   // Try to correct typos if possible. If constructor name lookup finds no
   // results, that means the named class has no explicit constructors, and we
   // suppressed declaring implicit ones (probably because it's dependent or
   // invalid).
   if (R.empty() &&
       NameInfo.getName().getNameKind() != DeclarationName::CXXConstructorName) {
-    // HACK: Work around a bug in libstdc++'s detection of ::gets. Sometimes
-    // it will believe that glibc provides a ::gets in cases where it does not,
-    // and will try to pull it into namespace std with a using-declaration.
-    // Just ignore the using-declaration in that case.
-    auto *II = NameInfo.getName().getAsIdentifierInfo();
-    if (getLangOpts().CPlusPlus14 && II && II->isStr("gets") &&
-        CurContext->isStdNamespace() &&
-        isa<TranslationUnitDecl>(LookupContext) &&
-        getSourceManager().isInSystemHeader(UsingLoc))
-      return nullptr;
     UsingValidatorCCC CCC(HasTypenameKeyword, IsInstantiation, SS.getScopeRep(),
                           dyn_cast<CXXRecordDecl>(CurContext));
     if (TypoCorrection Corrected =
             CorrectTypo(R.getLookupNameInfo(), R.getLookupKind(), S, &SS, CCC,
                         CTK_ErrorRecovery)) {
       // We reject candidates where DroppedSpecifier == true, hence the
       // literal '0' below.
       diagnoseTypo(Corrected, PDiag(diag::err_no_member_suggest)

clang/test/SemaCXX/libstdcxx_gets_hack.cpp

 // 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.
 
+// The gcc bug was gfixed in 2016.  now check gets is not special
+
 #ifdef BE_THE_HEADER
 
 #pragma GCC system_header
 namespace std {
-  using ::gets;
+  using ::gets; // expected-error {{no member named '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