[PATCH] D48072: Sema: Fix PR12350 destructor name lookup, addressing (some of) DR244

[Jeremy Morse via Phabricator via cfe-commits]

jmorse created this revision.
jmorse added reviewers: rsmith, doug.gregor, majnemer.
Herald added a subscriber: cfe-commits.

Hi,

This patch tries to fix a problem in clangs implementation of C++11's
[basic.lookup.qual]p6 as demonstrated in PR12350, by:

- Re-enabling looking in name-specifier prefixes for destructor names, when the penultimate name is a class
- In that case, forces the destructor name to be sought as a type-name

Which has some history. Looking in the name-specifier prefix for a
destructor name was partially disabled in r96836 for C++ < 11 because
name hiding could hide the final destructor name lookup (PR6358), and
then for all C++ in r107835 because the initial fix for https://reviews.llvm.org/D244 (r209319)
wasn't in.

However, the spec still specifies looking in the name-specifier prefix, and
without doing so bugs like PR12350 can't be fixed. This patch gets around
the original name hiding issue by forcing a type-name lookup if we're
looking in the name-specifier prefix where the penultimate is a class,
avoiding any hiding.

This is almost certainly what the spec intended, IMHO. You can read
[basic.lookup.qual]p6 as supporting this interpretation if you take the
pseudo destructors phrasing "...the type-names are looked up _as_ types
in the scope..." and assume that the class-name form being looked
up "Similarly" means that the second class-name should also be looked
up _as_ a class-name.


Repository:
  rC Clang

https://reviews.llvm.org/D48072

Files:
  lib/Sema/SemaExprCXX.cpp
  test/CXX/drs/dr2xx.cpp
  test/SemaCXX/destructor.cpp


Index: test/SemaCXX/destructor.cpp
===================================================================
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -493,4 +493,23 @@
   x.foo1();
 }
 }
+
+namespace PR12350 {
+  class basic_string {
+  public:
+    ~basic_string();
+    char *something;
+  };
+
+  namespace notstd {
+    typedef basic_string string;
+  }
+
+  void
+  foo(notstd::string *bar)
+  {
+    bar->notstd::string::~string(); // OK
+  }
+}
+
 #endif // BE_THE_HEADER
Index: test/CXX/drs/dr2xx.cpp
===================================================================
--- test/CXX/drs/dr2xx.cpp
+++ test/CXX/drs/dr2xx.cpp
@@ -502,7 +502,7 @@
     f.N::F::~E();
     // This is valid; we look up the second F in the same scope in which we
     // found the first one, that is, 'N::'.
-    f.N::F::~F(); // FIXME: expected-error {{expected the class name after '~' to name a destructor}}
+    f.N::F::~F();
     // This is technically ill-formed; G is looked up in 'N::' and is not found;
     // as above, this is probably a bug in the standard.
     f.N::F::~G();
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -112,6 +112,7 @@
   DeclContext *LookupCtx = nullptr;
   bool isDependent = false;
   bool LookInScope = false;
+  LookupNameKind LookupKind = LookupOrdinaryName;
 
   if (SS.isInvalid())
     return nullptr;
@@ -144,7 +145,9 @@
       LookupCtx = DC;
       isDependent = false;
     } else if (DC && isa<CXXRecordDecl>(DC)) {
-      LookAtPrefix = false;
+      // Penultimate name is a class-name, we will LookAtPrefix. However, look
+      // up class-name's in the same scope as the first.
+      LookupKind = LookupNestedNameSpecifierName;
       LookInScope = true;
     }
 
@@ -184,7 +187,7 @@
   }
 
   TypeDecl *NonMatchingTypeDecl = nullptr;
-  LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
+  LookupResult Found(*this, &II, NameLoc, LookupKind);
   for (unsigned Step = 0; Step != 2; ++Step) {
     // Look for the name first in the computed lookup context (if we
     // have one) and, if that fails to find a match, in the scope (if


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48072.150919.patch
Type: text/x-patch
Size: 2216 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180612/45f59020/attachment.bin>