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
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
