[PATCH] D79496: [clangd] Fix AddUsing tweak for out-of-line functions.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz created this revision.
adamcz added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

We used getEnclosingNamespaceContext(), which calls getParent() rather
than getLexicalParent(), so we would end up adding the "using" line in
places that do not affect the cursor location, or just return an error
when declaration was in another file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79496

Files:
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2663,6 +2663,23 @@
 
 void fun() {
   CALL(ff);
+})cpp"},
+            // Parent namespace != lexical parent namespace
+            {R"cpp(
+#include "test.hpp"
+namespace foo { void fun(); }
+
+void foo::fun() {
+  one::two::f^f();
+})cpp",
+             R"cpp(
+#include "test.hpp"
+using one::two::ff;
+
+namespace foo { void fun(); }
+
+void foo::fun() {
+  ff();
 })cpp"}};
   llvm::StringMap<std::string> EditedFiles;
   for (const auto &Case : Cases) {
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -141,10 +141,12 @@
   }
 
   // No relevant "using" statements. Try the nearest namespace level.
-  const auto *NS = Inputs.ASTSelection.commonAncestor()
-                       ->getDeclContext()
-                       .getEnclosingNamespaceContext();
-  if (auto *ND = dyn_cast<NamespaceDecl>(NS)) {
+  const DeclContext *ParentDeclCtx =
+      &Inputs.ASTSelection.commonAncestor()->getDeclContext();
+  while (ParentDeclCtx && !ParentDeclCtx->isFileContext()) {
+    ParentDeclCtx = ParentDeclCtx->getLexicalParent();
+  }
+  if (auto *ND = llvm::dyn_cast_or_null<NamespaceDecl>(ParentDeclCtx)) {
     auto Toks = Inputs.AST->getTokens().expandedTokens(ND->getSourceRange());
     const auto *Tok = llvm::find_if(Toks, [](const syntax::Token &Tok) {
       return Tok.kind() == tok::l_brace;



Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2663,6 +2663,23 @@
 
 void fun() {
   CALL(ff);
+})cpp"},
+            // Parent namespace != lexical parent namespace
+            {R"cpp(
+#include "test.hpp"
+namespace foo { void fun(); }
+
+void foo::fun() {
+  one::two::f^f();
+})cpp",
+             R"cpp(
+#include "test.hpp"
+using one::two::ff;
+
+namespace foo { void fun(); }
+
+void foo::fun() {
+  ff();
 })cpp"}};
   llvm::StringMap<std::string> EditedFiles;
   for (const auto &Case : Cases) {
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -141,10 +141,12 @@
   }
 
   // No relevant "using" statements. Try the nearest namespace level.
-  const auto *NS = Inputs.ASTSelection.commonAncestor()
-                       ->getDeclContext()
-                       .getEnclosingNamespaceContext();
-  if (auto *ND = dyn_cast<NamespaceDecl>(NS)) {
+  const DeclContext *ParentDeclCtx =
+      &Inputs.ASTSelection.commonAncestor()->getDeclContext();
+  while (ParentDeclCtx && !ParentDeclCtx->isFileContext()) {
+    ParentDeclCtx = ParentDeclCtx->getLexicalParent();
+  }
+  if (auto *ND = llvm::dyn_cast_or_null<NamespaceDecl>(ParentDeclCtx)) {
     auto Toks = Inputs.AST->getTokens().expandedTokens(ND->getSourceRange());
     const auto *Tok = llvm::find_if(Toks, [](const syntax::Token &Tok) {
       return Tok.kind() == tok::l_brace;

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