john.brawn created this revision.
john.brawn added reviewers: dexonsmith, rsmith, rjmccall.
Herald added a project: All.
john.brawn requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When a function is declared in the same scope as a class with the same name
then the function hides that class. Currently this is done by a single check
after the main loop in LookupResult::resolveKind, but this can give the wrong
result when we have a using declaration in multiple namespace scopes in two
different ways:
- When the using declaration is hidden in one namespace but not the other we
can end up considering only the hidden one when deciding if the result is
ambiguous, causing an incorrect "not ambiguous" result.
- When two classes with the same name in different namespace scopes are both
hidden by using declarations this can result in incorrectly deciding the result
is ambiguous. There's currently a comment saying this is expected, but I don't
think that's correct.
Solve this by having a separate loop before the main loop to eliminate classes
that have been hidden by functions.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154503
Files:
clang/lib/Sema/SemaLookup.cpp
clang/test/SemaCXX/using-hiding.cpp
Index: clang/test/SemaCXX/using-hiding.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/using-hiding.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+namespace A {
+ class X { }; // expected-note{{candidate found by name lookup is 'A::X'}}
+}
+namespace B {
+ void X(int); // expected-note{{candidate found by name lookup is 'B::X'}}
+}
+
+// Using directive doesn't cause A::X to be hidden, so X is ambiguous.
+namespace Test1 {
+ using namespace A;
+ using namespace B;
+
+ void f() {
+ X(1); // expected-error{{reference to 'X' is ambiguous}}
+ }
+}
+
+// Using declaration causes A::X to be hidden, so X is not ambiguous.
+namespace Test2 {
+ using A::X;
+ using B::X;
+
+ void f() {
+ X(1);
+ }
+}
+
+// Behaviour here should be the same as including A and B directly.
+namespace Test3 {
+ namespace C {
+ using A::X; // expected-note{{candidate found by name lookup is 'Test3::C::X'}}
+ }
+ namespace D {
+ using B::X; // expected-note{{candidate found by name lookup is 'Test3::D::X'}}
+ }
+ using namespace C;
+ using namespace D;
+
+ void f() {
+ X(1); // expected-error{{reference to 'X' is ambiguous}}
+ }
+}
+
+// using B::X inside C should hide using A::X in C but not D, so the result is ambiguous.
+namespace Test4 {
+ namespace C {
+ using A::X;
+ using B::X; // expected-note{{candidate found by name lookup is 'Test4::C::X'}}
+ }
+ namespace D {
+ using A::X; // expected-note{{candidate found by name lookup is 'Test4::D::X'}}
+ }
+ using namespace C;
+ using namespace D;
+ void f() {
+ X(1); // expected-error{{reference to 'X' is ambiguous}}
+ }
+}
+
+// A::X hidden in both C and D, so the result is not ambiguous.
+namespace Test5 {
+ namespace C {
+ using A::X;
+ using B::X;
+ }
+ namespace D {
+ using A::X;
+ using B::X;
+ }
+ using namespace C;
+ using namespace D;
+ void f() {
+ X(1);
+ }
+}
+
+// D declares a different class X, but it's hidden so the result is not ambiguous.
+namespace Test6 {
+ namespace C {
+ using A::X;
+ using B::X;
+ }
+ namespace D {
+ class X { };
+ using B::X;
+ }
+ using namespace C;
+ using namespace D;
+ void f() {
+ X(1);
+ }
+}
+
+// X inside C should hide X in C but not D.
+namespace Test7 {
+ namespace C {
+ class X;
+ void X(int); // expected-note{{candidate found by name lookup is 'Test7::C::X'}}
+ }
+ namespace D {
+ class X; // expected-note{{candidate found by name lookup is 'Test7::D::X'}}
+ }
+ using namespace C;
+ using namespace D;
+ void f() {
+ X(1); // expected-error{{reference to 'X' is ambiguous}}
+ }
+}
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -504,6 +504,43 @@
// Don't do any extra resolution if we've already resolved as ambiguous.
if (ResultKind == Ambiguous) return;
+ // C++ [basic.scope.hiding]p2:
+ // A class name or enumeration name can be hidden by the name of
+ // an object, function, or enumerator declared in the same
+ // scope. If a class or enumeration name and an object, function,
+ // or enumerator are declared in the same scope (in any order)
+ // with the same name, the class or enumeration name is hidden
+ // wherever the object, function, or enumerator name is visible.
+ if (HideTags) {
+ // First collect all decls that can hide others and those that can be hidden
+ std::set<unsigned> CanHideOther, CanBeHidden;
+ for (unsigned I = 0; I < N; ++I) {
+ const NamedDecl *D = Decls[I]->getUnderlyingDecl();
+ D = cast<NamedDecl>(D->getCanonicalDecl());
+ if (isa<TagDecl>(D))
+ CanBeHidden.insert(I);
+ else if (canHideTag(D))
+ CanHideOther.insert(I);
+ }
+
+ // Collect those decls that will be hidden
+ std::set<unsigned, std::greater<unsigned>> HiddenDecls;
+ for (unsigned HiddenI : CanBeHidden) {
+ for (unsigned HiderI : CanHideOther) {
+ if (getContextForScopeMatching(Decls[HiderI])
+ ->Equals(getContextForScopeMatching(Decls[HiddenI]))) {
+ HiddenDecls.insert(HiddenI);
+ break;
+ }
+ }
+ }
+
+ // Now erase those decls that were hidden
+ for (auto I : HiddenDecls)
+ Decls.erase(I);
+ N = Decls.size();
+ }
+
llvm::SmallDenseMap<const NamedDecl *, unsigned, 16> Unique;
llvm::SmallDenseMap<QualType, unsigned, 16> UniqueTypes;
@@ -514,8 +551,6 @@
llvm::SmallVector<const NamedDecl *, 4> EquivalentNonFunctions;
- unsigned UniqueTagIndex = 0;
-
unsigned I = 0;
while (I < N) {
const NamedDecl *D = Decls[I]->getUnderlyingDecl();
@@ -571,7 +606,6 @@
} else if (isa<TagDecl>(D)) {
if (HasTag)
Ambiguous = true;
- UniqueTagIndex = I;
HasTag = true;
} else if (isa<FunctionTemplateDecl>(D)) {
HasFunction = true;
@@ -598,27 +632,6 @@
I++;
}
- // C++ [basic.scope.hiding]p2:
- // A class name or enumeration name can be hidden by the name of
- // an object, function, or enumerator declared in the same
- // scope. If a class or enumeration name and an object, function,
- // or enumerator are declared in the same scope (in any order)
- // with the same name, the class or enumeration name is hidden
- // wherever the object, function, or enumerator name is visible.
- // But it's still an error if there are distinct tag types found,
- // even if they're not visible. (ref?)
- if (N > 1 && HideTags && HasTag && !Ambiguous &&
- (HasFunction || HasNonFunction || HasUnresolved)) {
- const NamedDecl *OtherDecl = Decls[UniqueTagIndex ? 0 : N - 1];
- if (isa<TagDecl>(Decls[UniqueTagIndex]->getUnderlyingDecl()) &&
- getContextForScopeMatching(Decls[UniqueTagIndex])->Equals(
- getContextForScopeMatching(OtherDecl)) &&
- canHideTag(OtherDecl))
- Decls[UniqueTagIndex] = Decls[--N];
- else
- Ambiguous = true;
- }
-
// FIXME: This diagnostic should really be delayed until we're done with
// the lookup result, in case the ambiguity is resolved by the caller.
if (!EquivalentNonFunctions.empty() && !Ambiguous)
@@ -627,7 +640,8 @@
Decls.truncate(N);
- if (HasNonFunction && (HasFunction || HasUnresolved))
+ if ((HasNonFunction && (HasFunction || HasUnresolved)) ||
+ (HideTags && HasTag && (HasFunction || HasNonFunction || HasUnresolved)))
Ambiguous = true;
if (Ambiguous)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
