kadircet updated this revision to Diff 230213.
kadircet marked 4 inline comments as done.
kadircet added a comment.
- Address comments
- Rebase
- Get rid of string literals inside macros
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
@@ -1727,6 +1727,146 @@
template <> inline void foo<int>(){})cpp")));
}
+TEST_F(DefineInlineTest, DropCommonNamespecifiers) {
+ struct {
+ llvm::StringRef Test;
+ llvm::StringRef Expected;
+ } Cases[] = {
+ {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"},
+ {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"},
+ {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"},
+ };
+ for (const auto &Case : Cases)
+ EXPECT_EQ(apply(Case.Test), Case.Expected) << Case.Test;
+}
+
+TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
+ llvm::StringRef Test = 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";
+ llvm::StringRef Expected = 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";
+ // 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(Test), Expected) << Test;
+}
+
} // 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) {
@@ -194,7 +196,10 @@
if (!ND->getDeclContext()->isNamespace())
return;
- std::string Qualifier = printNamespaceScope(*ND->getDeclContext());
+ // FIXME: Also take using directives and namespace aliases inside function
+ // body into account.
+ const std::string Qualifier = getQualification(
+ FD->getASTContext(), TargetContext, Target->getBeginLoc(), ND);
if (auto Err = Replacements.add(
tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) {
HadErrors = true;
@@ -463,7 +468,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
