This revision was automatically updated to reflect the committed changes.
Closed by commit rG5075c6821982: [clangd] Improve symbol qualification in
DefineInline code action (authored by kadircet).
Changed prior to commit:
https://reviews.llvm.org/D69033?vs=230424&id=230851#toc
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/AST.h
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
@@ -1667,6 +1667,148 @@
EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
}
+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;
+ // FIXME: The last reference to cux() in body of foo should not be
+ // qualified, since there is a using directive inside the function body.
+ 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;
+ // FIXME: The last reference to cux() in body of foo should not be
+ // qualified, since there is a using directive inside the function body.
+ c::cux();
+ }
+
+ using namespace b;
+ using namespace c;
+ )cpp";
+ 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
@@ -135,8 +135,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
@@ -147,16 +149,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) {
@@ -193,7 +195,8 @@
if (!ND->getDeclContext()->isNamespace())
return;
- std::string Qualifier = printNamespaceScope(*ND->getDeclContext());
+ 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;
@@ -437,7 +440,7 @@
if (!ParamReplacements)
return ParamReplacements.takeError();
- auto QualifiedBody = qualifyAllDecls(Source);
+ auto QualifiedBody = qualifyAllDecls(Source, Target);
if (!QualifiedBody)
return QualifiedBody.takeError();
Index: clang-tools-extra/clangd/AST.h
===================================================================
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -137,6 +137,8 @@
/// InsertionPoint. i.e, if you have `using namespace
/// clang::clangd::bar`, this function will return an empty string for the
/// example above since no qualification is necessary in that case.
+/// FIXME: Also take using directives and namespace aliases inside function body
+/// into account.
std::string getQualification(ASTContext &Context,
const DeclContext *DestContext,
SourceLocation InsertionPoint,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
