kadircet updated this revision to Diff 225416.
kadircet marked an inline comment as done.
kadircet added a comment.
- Handle using directives
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
@@ -1045,22 +1045,20 @@
namespace a {
template <typename T> class Bar {};
}
- using namespace a;
template <typename T>
void foo() /*Comment -_-*/ ;
+ using namespace a;
template <typename T>
void f^oo() {
Bar<T> B;
Bar<Bar<T>> q;
- }
- )cpp"),
+ })cpp"),
R"cpp(
namespace a {
template <typename T> class Bar {};
}
- using namespace a;
template <typename T>
void foo() /*Comment -_-*/ {
@@ -1068,7 +1066,7 @@
a::Bar<a::Bar<T>> q;
}
-
+ using namespace a;
)cpp");
}
@@ -1169,18 +1167,17 @@
class Foo{};
};
}
- using namespace a;
- using namespace b;
- using namespace c;
void foo() /*Comment -_-*/ ;
+ using namespace a;
+ using namespace b;
+ using namespace c;
void f^oo() {
Bar<int> B;
b::Foo foo;
a::Bar<Bar<int>>::Baz<Bar<int>> q;
- }
- )cpp"),
+ })cpp"),
R"cpp(
namespace a {
template <typename T> class Bar {
@@ -1191,9 +1188,6 @@
class Foo{};
};
}
- using namespace a;
- using namespace b;
- using namespace c;
void foo() /*Comment -_-*/ {
a::Bar<int> B;
@@ -1201,7 +1195,9 @@
a::Bar<a::Bar<int>>::Baz<a::Bar<int>> q;
}
-
+ using namespace a;
+ using namespace b;
+ using namespace c;
)cpp");
}
@@ -1223,12 +1219,12 @@
}
}
}
- using namespace a;
- using namespace b;
- using namespace c;
void foo() /*Comment -_-*/ ;
+ using namespace a;
+ using namespace b;
+ using namespace c;
void f^oo() {
a::Bar<int> B;
B.foo();
@@ -1239,8 +1235,7 @@
Bar<int>::y = 3;
bar();
c::test();
- }
- )cpp"),
+ })cpp"),
R"cpp(
namespace a {
template <typename T> class Bar {
@@ -1258,9 +1253,6 @@
}
}
}
- using namespace a;
- using namespace b;
- using namespace c;
void foo() /*Comment -_-*/ {
a::Bar<int> B;
@@ -1274,7 +1266,9 @@
a::b::c::test();
}
-
+ using namespace a;
+ using namespace b;
+ using namespace c;
)cpp");
}
@@ -1402,6 +1396,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
@@ -143,8 +143,53 @@
return true;
}
-// Rewrites body of FD to fully-qualify all of the decls inside.
-llvm::Expected<std::string> qualifyAllDecls(const FunctionDecl *FD) {
+// Gets the nested name specifiers necessary for spelling ND in DestContext.
+// It qualifies until first qualifier of ND that is either in
+// VisibleNamespaceDecls or encloses DestContext.
+// For example, if you want to qualify clang::clangd::bar::foo in
+// clang::clangd::x, this function will return bar. And if you have
+// clang::clangd::bar in VisibleNamespaceDecls, this function will return null.
+NestedNameSpecifier *getQualification(
+ ASTContext &Context, const DeclContext *DestContext, const NamedDecl *ND,
+ const llvm::DenseSet<const NamespaceDecl *> &VisibleNamespaceDecls) {
+ // Goes over all parents of ND until we find a comman ancestor for DestContext
+ // and ND. Any qualifier including and above common ancestor is redundant,
+ // therefore we stop at lowest common ancestor.
+ std::vector<const NamespaceDecl *> SourceParents;
+ for (const DeclContext *Context = ND->getLexicalDeclContext(); Context;
+ Context = Context->getLookupParent()) {
+ // Stop once we reach a common ancestor.
+ if (Context->Encloses(DestContext))
+ break;
+ // Inline namespace names are not spelled while qualifying a name, so skip
+ // those.
+ if (Context->isInlineNamespace())
+ continue;
+
+ auto *NSD = llvm::dyn_cast<NamespaceDecl>(Context);
+ assert(NSD && "Non-namespace decl context found.");
+ // Stop if this namespace is already visible at DestContext.
+ if (VisibleNamespaceDecls.count(NSD->getCanonicalDecl()))
+ break;
+ // Again, ananoymous namespaces are not spelled while qualifying a name.
+ if (NSD->isAnonymousNamespace())
+ continue;
+
+ SourceParents.push_back(NSD);
+ }
+
+ // Go over name-specifiers in reverse order to create necessary qualification,
+ // since we stored inner-most parent first.
+ NestedNameSpecifier *Result = nullptr;
+ for (const auto *Parent : llvm::reverse(SourceParents))
+ Result = NestedNameSpecifier::Create(Context, Result, Parent);
+ return Result;
+}
+
+// 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
@@ -155,16 +200,29 @@
// 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();
+ SourceLocation TargetLoc = Target->getBeginLoc();
const SourceManager &SM = FD->getASTContext().getSourceManager();
+
+ llvm::DenseSet<const NamespaceDecl *> VisibleNamespacesInTargetContext;
+ for (const auto *DC = TargetContext; DC; DC = DC->getLookupParent()) {
+ for (const auto *D : DC->decls()) {
+ if (!SM.isWrittenInSameFile(D->getLocation(), TargetLoc) ||
+ !SM.isBeforeInTranslationUnit(D->getLocation(), TargetLoc))
+ continue;
+ if (auto *UDD = llvm::dyn_cast<UsingDirectiveDecl>(D)) {
+ VisibleNamespacesInTargetContext.insert(
+ UDD->getNominatedNamespace()->getCanonicalDecl());
+ }
+ }
+ }
tooling::Replacements Replacements;
bool HadErrors = false;
findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) {
@@ -195,10 +253,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, ND,
+ VisibleNamespacesInTargetContext)) {
+ 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;
@@ -527,7 +593,7 @@
if (!ParamReplacements)
return ParamReplacements.takeError();
- auto QualifiedBody = qualifyAllDecls(Source);
+ auto QualifiedBody = qualifyAllDecls(Source, Target);
if (!QualifiedBody)
return QualifiedBody.takeError();
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
