[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added a reviewer: tom-anders.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Existing version ignores symbols declared in an inline namespace `ns`
when removing `using namespace ns`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138028

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
       }
       
       int main() { 1.5_w; }
+    )cpp"},
+      {
+          R"cpp(
+      namespace a { inline namespace b { void foobar(); } }
+      using namespace a::[[b]];
+      int main() { foobar(); }
+    )cpp",
+          R"cpp(
+      namespace a { inline namespace b { void foobar(); } }
+      
+      int main() { a::b::foobar(); }
     )cpp"}};
   for (auto C : Cases)
     EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get<TranslationUnitDecl>();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool declareIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+    if (D->Equals(RHS))
+      return true;
     D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
             return; // This reference is already qualified.
 
           for (auto *T : Ref.Targets) {
-            if (!visibleContext(T->getDeclContext())
-                     ->Equals(TargetDirective->getNominatedNamespace()))
+            if (!declareIn(T, TargetDirective->getNominatedNamespace()))
               return;
             auto Kind = T->getDeclName().getNameKind();
             // Avoid adding qualifiers before operators, e.g.



Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
       }
       
       int main() { 1.5_w; }
+    )cpp"},
+      {
+          R"cpp(
+      namespace a { inline namespace b { void foobar(); } }
+      using namespace a::[[b]];
+      int main() { foobar(); }
+    )cpp",
+          R"cpp(
+      namespace a { inline namespace b { void foobar(); } }
+      
+      int main() { a::b::foobar(); }
     )cpp"}};
   for (auto C : Cases)
     EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get<TranslationUnitDecl>();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool declareIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+    if (D->Equals(RHS))
+      return true;
     D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
             return; // This reference is already qualified.
 
           for (auto *T : Ref.Targets) {
-            if (!visibleContext(T->getDeclContext())
-                     ->Equals(TargetDirective->getNominatedNamespace()))
+            if (!declareIn(T, TargetDirective->getNominatedNamespace()))
               return;
             auto Kind = T->getDeclName().getNameKind();
             // Avoid adding qualifiers before operators, e.g.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits