john.brawn updated this revision to Diff 537731.
john.brawn added a comment.

Same patch as previous, but with full context.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154503/new/

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,45 @@
   // 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);
+    }
+
+    if (!CanBeHidden.empty() && !CanHideOther.empty()) {
+      // 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 +553,6 @@
 
   llvm::SmallVector<const NamedDecl *, 4> EquivalentNonFunctions;
 
-  unsigned UniqueTagIndex = 0;
-
   unsigned I = 0;
   while (I < N) {
     const NamedDecl *D = Decls[I]->getUnderlyingDecl();
@@ -571,7 +608,6 @@
     } else if (isa<TagDecl>(D)) {
       if (HasTag)
         Ambiguous = true;
-      UniqueTagIndex = I;
       HasTag = true;
     } else if (isa<FunctionTemplateDecl>(D)) {
       HasFunction = true;
@@ -598,27 +634,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 +642,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

Reply via email to