john.brawn updated this revision to Diff 542893.
john.brawn edited the summary of this revision.
john.brawn added a comment.

Restructured to check for hidden tags in the main loop. Also add a bunch of 
extra tests.


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,373 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+namespace A {
+  class X { }; // expected-note{{candidate found by name lookup is 'A::X'}}
+               // expected-note@-1{{candidate found by name lookup is 'A::X'}}
+}
+namespace B {
+  void X(int); // expected-note{{candidate found by name lookup is 'B::X'}}
+               // expected-note@-1{{candidate found by name lookup is 'B::X'}}
+}
+
+// Using directive doesn't cause A::X to be hidden, so X is ambiguous.
+namespace Test1a {
+  using namespace A;
+  using namespace B;
+
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test1b {
+  using namespace B;
+  using namespace A;
+
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+// The behaviour here should be the same as using namespaces A and B directly
+namespace Test2a {
+  namespace C {
+    using A::X; // expected-note{{candidate found by name lookup is 'Test2a::C::X'}}
+  }
+  namespace D {
+    using B::X; // expected-note{{candidate found by name lookup is 'Test2a::D::X'}}
+  }
+  using namespace C;
+  using namespace D;
+
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test2b {
+  namespace C {
+    using A::X; // expected-note{{candidate found by name lookup is 'Test2b::C::X'}}
+  }
+  namespace D {
+    using B::X; // expected-note{{candidate found by name lookup is 'Test2b::D::X'}}
+  }
+  using namespace D;
+  using namespace C;
+
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+// Defining a function X inside C should hide using A::X in C but not D, so the result is ambiguous.
+namespace Test3a {
+  namespace C {
+    using A::X;
+    void X(int); // expected-note{{candidate found by name lookup is 'Test3a::C::X'}}
+  }
+  namespace D {
+    using A::X; // expected-note{{candidate found by name lookup is 'Test3a::D::X'}}
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test3b {
+  namespace C {
+    using A::X;
+    void X(int); // expected-note{{candidate found by name lookup is 'Test3b::C::X'}}
+  }
+  namespace D {
+    using A::X; // expected-note{{candidate found by name lookup is 'Test3b::D::X'}}
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test3c {
+  namespace C {
+    void X(int); // expected-note{{candidate found by name lookup is 'Test3c::C::X'}}
+    using A::X;
+  }
+  namespace D {
+    using A::X; // expected-note{{candidate found by name lookup is 'Test3c::D::X'}}
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test3d {
+  namespace C {
+    void X(int); // expected-note{{candidate found by name lookup is 'Test3d::C::X'}}
+    using A::X;
+  }
+  namespace D {
+    using A::X; // expected-note{{candidate found by name lookup is 'Test3d::D::X'}}
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+// A::X hidden in both C and D by overloaded function, so the result is not ambiguous.
+namespace Test4a {
+  namespace C {
+    using A::X;
+    void X(int);
+  }
+  namespace D {
+    using A::X;
+    void X(int, int);
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test4b {
+  namespace C {
+    using A::X;
+    void X(int);
+  }
+  namespace D {
+    using A::X;
+    void X(int, int);
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test4c {
+  namespace C {
+    void X(int);
+    using A::X;
+  }
+  namespace D {
+    void X(int, int);
+    using A::X;
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test4d {
+  namespace C {
+    void X(int);
+    using A::X;
+  }
+  namespace D {
+    void X(int, int);
+    using A::X;
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1);
+  }
+}
+
+// B::X hides class X in C, so the the result is not ambiguous
+namespace Test5a {
+  namespace C {
+    using B::X;
+    class X { };
+  }
+  namespace D {
+    using B::X;
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test5b {
+  namespace C {
+    using B::X;
+    class X { };
+  }
+  namespace D {
+    using B::X;
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test5c {
+  namespace C {
+    class X { };
+    using B::X;
+  }
+  namespace D {
+    using B::X;
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test5d {
+  namespace C {
+    class X { };
+    using B::X;
+  }
+  namespace D {
+    using B::X;
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1);
+  }
+}
+
+// B::X hides class X declared in both C and D, so the result is not ambiguous.
+namespace Test6a {
+  namespace C {
+    class X { };
+    using B::X;
+  }
+  namespace D {
+    class X { };
+    using B::X;
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test6b {
+  namespace C {
+    class X { };
+    using B::X;
+  }
+  namespace D {
+    class X { };
+    using B::X;
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test6c {
+  namespace C {
+    using B::X;
+    class X { };
+  }
+  namespace D {
+    using B::X;
+    class X { };
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1);
+  }
+}
+
+namespace Test6d {
+  namespace C {
+    using B::X;
+    class X { };
+  }
+  namespace D {
+    using B::X;
+    class X { };
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1);
+  }
+}
+
+// function X inside C should hide class X in C but not D.
+namespace Test7a {
+  namespace C {
+    class X;
+    void X(int); // expected-note{{candidate found by name lookup is 'Test7a::C::X'}}
+  }
+  namespace D {
+    class X; // expected-note{{candidate found by name lookup is 'Test7a::D::X'}}
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test7b {
+  namespace C {
+    class X;
+    void X(int); // expected-note{{candidate found by name lookup is 'Test7b::C::X'}}
+  }
+  namespace D {
+    class X; // expected-note{{candidate found by name lookup is 'Test7b::D::X'}}
+  }
+  using namespace D;
+  using namespace C;
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test7c {
+  namespace C {
+    void X(int); // expected-note{{candidate found by name lookup is 'Test7c::C::X'}}
+    class X;
+  }
+  namespace D {
+    class X; // expected-note{{candidate found by name lookup is 'Test7c::D::X'}}
+  }
+  using namespace C;
+  using namespace D;
+  void f() {
+    X(1); // expected-error{{reference to 'X' is ambiguous}}
+  }
+}
+
+namespace Test7d {
+  namespace C {
+    void X(int); // expected-note{{candidate found by name lookup is 'Test7d::C::X'}}
+    class X;
+  }
+  namespace D {
+    class X; // expected-note{{candidate found by name lookup is 'Test7d::D::X'}}
+  }
+  using namespace D;
+  using namespace C;
+  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
@@ -513,21 +513,42 @@
   const NamedDecl *HasNonFunction = nullptr;
 
   llvm::SmallVector<const NamedDecl *, 4> EquivalentNonFunctions;
+  llvm::BitVector RemovedDecls(N);
 
-  unsigned UniqueTagIndex = 0;
-
-  unsigned I = 0;
-  while (I < N) {
+  for (unsigned I = 0; I < N; I++) {
     const NamedDecl *D = Decls[I]->getUnderlyingDecl();
     D = cast<NamedDecl>(D->getCanonicalDecl());
 
     // Ignore an invalid declaration unless it's the only one left.
     // Also ignore HLSLBufferDecl which not have name conflict with other Decls.
-    if ((D->isInvalidDecl() || isa<HLSLBufferDecl>(D)) && !(I == 0 && N == 1)) {
-      Decls[I] = Decls[--N];
+    if ((D->isInvalidDecl() || isa<HLSLBufferDecl>(D)) &&
+        N - RemovedDecls.count() > 1) {
+      RemovedDecls.set(I);
       continue;
     }
 
+    // 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 && isa<TagDecl>(D)) {
+      bool Hidden = false;
+      for (auto *OtherDecl : Decls) {
+        if (canHideTag(OtherDecl) &&
+            getContextForScopeMatching(OtherDecl)->Equals(
+                getContextForScopeMatching(Decls[I]))) {
+          RemovedDecls.set(I);
+          Hidden = true;
+          break;
+        }
+      }
+      if (Hidden)
+        continue;
+    }
+
     std::optional<unsigned> ExistingI;
 
     // Redeclarations of types via typedef can occur both within a scope
@@ -559,8 +580,9 @@
       // discard the other.
       if (isPreferredLookupResult(getSema(), getLookupKind(), Decls[I],
                                   Decls[*ExistingI]))
-        Decls[*ExistingI] = Decls[I];
-      Decls[I] = Decls[--N];
+        RemovedDecls.set(*ExistingI);
+      else
+        RemovedDecls.set(I);
       continue;
     }
 
@@ -571,7 +593,6 @@
     } else if (isa<TagDecl>(D)) {
       if (HasTag)
         Ambiguous = true;
-      UniqueTagIndex = I;
       HasTag = true;
     } else if (isa<FunctionTemplateDecl>(D)) {
       HasFunction = true;
@@ -587,7 +608,7 @@
         if (getSema().isEquivalentInternalLinkageDeclaration(HasNonFunction,
                                                              D)) {
           EquivalentNonFunctions.push_back(D);
-          Decls[I] = Decls[--N];
+          RemovedDecls.set(I);
           continue;
         }
 
@@ -595,28 +616,6 @@
       }
       HasNonFunction = D;
     }
-    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
@@ -625,9 +624,15 @@
     getSema().diagnoseEquivalentInternalLinkageDeclarations(
         getNameLoc(), HasNonFunction, EquivalentNonFunctions);
 
+  // Remove decls by replacing them with decls from the end (which
+  // means that we need to iterate from the end) and then truncating
+  // to the new size.
+  for (int I = RemovedDecls.find_last(); I >= 0; I = RemovedDecls.find_prev(I))
+    Decls[I] = Decls[--N];
   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