kadircet updated this revision to Diff 227040.
kadircet added a comment.

- Rebase
- Move qualification logic into AST.h so that it can be used by define outline, 
see D69033 <https://reviews.llvm.org/D69033>.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69033

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.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
@@ -1714,6 +1714,138 @@
                             template <> inline void foo<int>(){})cpp")));
 }
 
+TEST_F(DefineInlineTest, DropCommonNamespecifiers) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a { namespace b { void aux(); } }
+  namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+  void bar();
+  namespace ns3 { void baz(); }
+  }
+  }
+
+  using namespace a;
+  using namespace a::b;
+  using namespace ns1::qq;
+  void ns1::ns2::ns3::b^az() {
+    foo();
+    bar();
+    baz();
+    ns1::ns2::ns3::baz();
+    aux();
+    test();
+  })cpp"),
+            R"cpp(
+  namespace a { namespace b { void aux(); } }
+  namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+  void bar();
+  namespace ns3 { void baz(){
+    foo();
+    bar();
+    baz();
+    ns1::ns2::ns3::baz();
+    a::b::aux();
+    qq::test();
+  } }
+  }
+  }
+
+  using namespace a;
+  using namespace a::b;
+  using namespace ns1::qq;
+  )cpp");
+
+  EXPECT_EQ(apply(R"cpp(
+  namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(); }
+  }
+
+  using namespace ns1::qq;
+  void ns1::ns2::b^az() { Foo f; B b; })cpp"),
+            R"cpp(
+  namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(){ qq::Foo f; qq::B b; } }
+  }
+
+  using namespace ns1::qq;
+  )cpp");
+
+  EXPECT_EQ(apply(R"cpp(
+  namespace ns1 {
+  namespace qq {
+    template<class T> struct Foo { template <class U> struct Bar {}; };
+    template<class T, class U>
+    using B = typename Foo<T>::template Bar<U>;
+  }
+  namespace ns2 { void baz(); }
+  }
+
+  using namespace ns1::qq;
+  void ns1::ns2::b^az() { B<int, bool> b; })cpp"),
+            R"cpp(
+  namespace ns1 {
+  namespace qq {
+    template<class T> struct Foo { template <class U> struct Bar {}; };
+    template<class T, class U>
+    using B = typename Foo<T>::template Bar<U>;
+  }
+  namespace ns2 { void baz(){ qq::B<int, bool> b; } }
+  }
+
+  using namespace ns1::qq;
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
+  // FIXME: The last reference to cux() in body of foo should not be qualified,
+  // since there is a using directive inside the function body.
+  EXPECT_EQ(apply(R"cpp(
+    namespace a {
+      void bar();
+      namespace b { struct Foo{}; void aux(); }
+      namespace c { void cux(); }
+    }
+    using namespace a;
+    using X = b::Foo;
+    void foo();
+
+    using namespace b;
+    using namespace c;
+    void ^foo() {
+      cux();
+      bar();
+      X x;
+      aux();
+      using namespace c;
+      cux();
+    })cpp"), R"cpp(
+    namespace a {
+      void bar();
+      namespace b { struct Foo{}; void aux(); }
+      namespace c { void cux(); }
+    }
+    using namespace a;
+    using X = b::Foo;
+    void foo(){
+      c::cux();
+      bar();
+      X x;
+      b::aux();
+      using namespace c;
+      c::cux();
+    }
+
+    using namespace b;
+    using namespace c;
+    )cpp");
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -136,8 +136,10 @@
   return true;
 }
 
-// Rewrites body of FD to fully-qualify all of the decls inside.
-llvm::Expected<std::string> qualifyAllDecls(const FunctionDecl *FD) {
+// Rewrites body of FD by re-spelling all of the names to make sure they are
+// still valid in context of Target.
+llvm::Expected<std::string> qualifyAllDecls(const FunctionDecl *FD,
+                                            const FunctionDecl *Target) {
   // There are three types of spellings that needs to be qualified in a function
   // body:
   // - Types:       Foo                 -> ns::Foo
@@ -148,16 +150,16 @@
   //    using ns3 = ns2     -> using ns3 = ns1::ns2
   //
   // Go over all references inside a function body to generate replacements that
-  // will fully qualify those. So that body can be moved into an arbitrary file.
+  // will qualify those. So that body can be moved into an arbitrary file.
   // We perform the qualification by qualyfying the first type/decl in a
   // (un)qualified name. e.g:
   //    namespace a { namespace b { class Bar{}; void foo(); } }
   //    b::Bar x; -> a::b::Bar x;
   //    foo(); -> a::b::foo();
-  // FIXME: Instead of fully qualyfying we should try deducing visible scopes at
-  // target location and generate minimal edits.
 
+  auto *TargetContext = Target->getLexicalDeclContext();
   const SourceManager &SM = FD->getASTContext().getSourceManager();
+
   tooling::Replacements Replacements;
   bool HadErrors = false;
   findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) {
@@ -191,10 +193,18 @@
     //   namespace a { class Bar { public: static int x; } }
     //   void foo() { Bar::x; }
     //                ~~~~~ -> we need to qualify Bar not x.
-    if (!ND->getDeclContext()->isNamespace())
+    if (!ND->getLexicalDeclContext()->isNamespace())
       return;
 
-    std::string Qualifier = printNamespaceScope(*ND->getDeclContext());
+    std::string Qualifier;
+    // FIXME: Also take using directives and namespace aliases inside function
+    // body into account.
+    if (auto *NNS = getQualification(FD->getASTContext(), TargetContext,
+                                     Target->getBeginLoc(), ND)) {
+      llvm::raw_string_ostream OS(Qualifier);
+      NNS->print(OS, FD->getASTContext().getPrintingPolicy(), true);
+    }
+
     if (auto Err = Replacements.add(
             tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) {
       HadErrors = true;
@@ -464,7 +474,7 @@
     if (!ParamReplacements)
       return ParamReplacements.takeError();
 
-    auto QualifiedBody = qualifyAllDecls(Source);
+    auto QualifiedBody = qualifyAllDecls(Source, Target);
     if (!QualifiedBody)
       return QualifiedBody.takeError();
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to