[PATCH] D31176: [clang-rename] Support renaming qualified symbol
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299419: [clang-rename] Support renaming qualified symbol
(authored by hokein).
Changed prior to commit:
https://reviews.llvm.org/D31176?vs=93474=94021#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31176
Files:
clang-tools-extra/trunk/clang-rename/CMakeLists.txt
clang-tools-extra/trunk/clang-rename/RenamingAction.cpp
clang-tools-extra/trunk/clang-rename/RenamingAction.h
clang-tools-extra/trunk/clang-rename/USRFinder.cpp
clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
clang-tools-extra/trunk/clang-rename/USRLocFinder.h
clang-tools-extra/trunk/unittests/clang-rename/CMakeLists.txt
clang-tools-extra/trunk/unittests/clang-rename/ClangRenameTest.h
clang-tools-extra/trunk/unittests/clang-rename/ClangRenameTests.cpp
clang-tools-extra/trunk/unittests/clang-rename/RenameClassTest.cpp
Index: clang-tools-extra/trunk/unittests/clang-rename/ClangRenameTest.h
===
--- clang-tools-extra/trunk/unittests/clang-rename/ClangRenameTest.h
+++ clang-tools-extra/trunk/unittests/clang-rename/ClangRenameTest.h
@@ -0,0 +1,112 @@
+//===-- ClangRenameTests.cpp - clang-rename unit tests ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RenamingAction.h"
+#include "USRFindingAction.h"
+#include "unittests/Tooling/RewriterTestContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Format/Format.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include
+#include
+#include
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+
+struct Case {
+ std::string Before;
+ std::string After;
+ std::string OldName;
+ std::string NewName;
+};
+
+class ClangRenameTest : public testing::Test,
+public testing::WithParamInterface {
+protected:
+ void AppendToHeader(StringRef Code) { HeaderContent += Code.str(); }
+
+ std::string runClangRenameOnCode(llvm::StringRef Code,
+ llvm::StringRef OldName,
+ llvm::StringRef NewName) {
+std::string NewCode;
+llvm::raw_string_ostream(NewCode) << llvm::format(
+"#include \"%s\"\n%s", HeaderName.c_str(), Code.str().c_str());
+tooling::FileContentMappings FileContents = {{HeaderName, HeaderContent},
+ {CCName, NewCode}};
+clang::RewriterTestContext Context;
+Context.createInMemoryFile(HeaderName, HeaderContent);
+clang::FileID InputFileID = Context.createInMemoryFile(CCName, NewCode);
+
+rename::USRFindingAction FindingAction({}, {OldName});
+std::unique_ptr USRFindingActionFactory =
+tooling::newFrontendActionFactory();
+
+if (!tooling::runToolOnCodeWithArgs(
+USRFindingActionFactory->create(), NewCode, {"-std=c++11"}, CCName,
+"clang-rename", std::make_shared(),
+FileContents))
+ return "";
+
+const std::vector =
+FindingAction.getUSRList();
+std::vector NewNames = {NewName};
+std::map FileToReplacements;
+rename::QualifiedRenamingAction RenameAction(NewNames, USRList,
+ FileToReplacements);
+auto RenameActionFactory = tooling::newFrontendActionFactory();
+if (!tooling::runToolOnCodeWithArgs(
+RenameActionFactory->create(), NewCode, {"-std=c++11"}, CCName,
+"clang-rename", std::make_shared(),
+FileContents))
+ return "";
+
+formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm");
+return Context.getRewrittenText(InputFileID);
+ }
+
+ void CompareSnippets(StringRef Expected, StringRef Actual) {
+std::string ExpectedCode;
+llvm::raw_string_ostream(ExpectedCode) << llvm::format(
+"#include \"%s\"\n%s", HeaderName.c_str(), Expected.str().c_str());
+EXPECT_EQ(format(ExpectedCode), format(Actual));
+ }
+
+ std::string format(llvm::StringRef Code) {
+tooling::Replacements Replaces = format::reformat(
+format::getLLVMStyle(), Code, {tooling::Range(0, Code.size())});
+auto ChangedCode =
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein updated this revision to Diff 93474.
hokein marked 2 inline comments as done.
hokein added a comment.
Fix a few nits.
https://reviews.llvm.org/D31176
Files:
clang-rename/CMakeLists.txt
clang-rename/RenamingAction.cpp
clang-rename/RenamingAction.h
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
clang-rename/USRLocFinder.cpp
clang-rename/USRLocFinder.h
unittests/clang-rename/CMakeLists.txt
unittests/clang-rename/ClangRenameTest.h
unittests/clang-rename/ClangRenameTests.cpp
unittests/clang-rename/RenameClassTest.cpp
Index: unittests/clang-rename/RenameClassTest.cpp
===
--- /dev/null
+++ unittests/clang-rename/RenameClassTest.cpp
@@ -0,0 +1,668 @@
+//===-- RenameClassTest.cpp - unit tests for renaming classes -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameClassTest : public ClangRenameTest {
+public:
+ RenameClassTest() {
+AppendToHeader(R"(
+ namespace a {
+class Foo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+void func() {}
+ static int Constant;
+};
+class Goo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+};
+int Foo::Constant = 1;
+ } // namespace a
+ namespace b {
+ class Foo {};
+ } // namespace b
+
+ #define MACRO(x) x
+
+ template class ptr {};
+)");
+ }
+};
+
+INSTANTIATE_TEST_CASE_P(
+RenameClassTests, RenameClassTest,
+testing::ValuesIn(std::vector({
+// basic classes
+{"a::Foo f;", "b::Bar f;"},
+{"void f(a::Foo f) {}", "void f(b::Bar f) {}"},
+{"void f(a::Foo *f) {}", "void f(b::Bar *f) {}"},
+{"a::Foo f() { return a::Foo(); }", "b::Bar f() { return b::Bar(); }"},
+{"namespace a {a::Foo f() { return Foo(); }}",
+ "namespace a {b::Bar f() { return b::Bar(); }}"},
+{"void f(const a::Foo& a1) {}", "void f(const b::Bar& a1) {}"},
+{"void f(const a::Foo* a1) {}", "void f(const b::Bar* a1) {}"},
+{"namespace a { void f(Foo a1) {} }",
+ "namespace a { void f(b::Bar a1) {} }"},
+{"void f(MACRO(a::Foo) a1) {}", "void f(MACRO(b::Bar) a1) {}"},
+{"void f(MACRO(a::Foo a1)) {}", "void f(MACRO(b::Bar a1)) {}"},
+{"a::Foo::Nested ns;", "b::Bar::Nested ns;"},
+{"auto t = a::Foo::Constant;", "auto t = b::Bar::Constant;"},
+{"a::Foo::Nested ns;", "a::Foo::Nested2 ns;", "a::Foo::Nested",
+ "a::Foo::Nested2"},
+
+// use namespace and typedefs
+{"using a::Foo; Foo gA;", "using b::Bar; b::Bar gA;"},
+{"using a::Foo; void f(Foo gA) {}", "using b::Bar; void f(Bar gA) {}"},
+{"using a::Foo; namespace x { Foo gA; }",
+ "using b::Bar; namespace x { Bar gA; }"},
+{"struct S { using T = a::Foo; T a_; };",
+ "struct S { using T = b::Bar; T a_; };"},
+{"using T = a::Foo; T gA;", "using T = b::Bar; T gA;"},
+{"typedef a::Foo T; T gA;", "typedef b::Bar T; T gA;"},
+{"typedef MACRO(a::Foo) T; T gA;", "typedef MACRO(b::Bar) T; T gA;"},
+
+// struct members and other oddities
+{"struct S : public a::Foo {};", "struct S : public b::Bar {};"},
+{"struct F { void f(a::Foo a1) {} };",
+ "struct F { void f(b::Bar a1) {} };"},
+{"struct F { a::Foo a_; };", "struct F { b::Bar a_; };"},
+{"struct F { ptr a_; };", "struct F { ptr a_; };"},
+
+{"void f() { a::Foo::Nested ne; }", "void f() { b::Bar::Nested ne; }"},
+{"void f() { a::Goo::Nested ne; }", "void f() { a::Goo::Nested ne; }"},
+{"void f() { a::Foo::Nested::NestedEnum e; }",
+ "void f() { b::Bar::Nested::NestedEnum e; }"},
+{"void f() { auto e = a::Foo::Nested::NestedEnum::E1; }",
+ "void f() { auto e = b::Bar::Nested::NestedEnum::E1; }"},
+{"void f() { auto e = a::Foo::Nested::E1; }",
+ "void f() { auto e = b::Bar::Nested::E1; }"},
+
+// templates
+{"template struct Foo { T t; };\n"
+ "void f() { Foo foo; }",
+ "template struct Foo { T t; };\n"
+ "void f() { Foo foo; }"},
+{"template struct Foo { a::Foo a; };",
+ "template struct Foo { b::Bar a; };"},
+{"template void f(T t) {}\n"
+ "void g() { f(a::Foo()); }",
+ "template void f(T t) {}\n"
+ "void g() { f(b::Bar()); }"},
+{"template int f() { return 1; }\n"
+ "template <> int f() { return 2; }\n"
+
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg with a few nits.
Comment at: clang-rename/RenamingAction.cpp:104
+ USRList[I], NewNames[I], Context.getTranslationUnitDecl());
+ for (const auto AtomicChange : AtomicChanges) {
+for (const auto : AtomicChange.getReplacements()) {
Maybe add a `FIXME` to fully replace this with `AtomicChange`s in the future?
Comment at: clang-rename/USRLocFinder.h:38
+std::vector
+createAtomicChanges(llvm::ArrayRef USRs, llvm::StringRef NewName,
+Decl *TranslationUnitDecl);
I'd name this `createRenameAtomicChanges` for clarity.
https://reviews.llvm.org/D31176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein updated this revision to Diff 93469.
hokein marked an inline comment as done.
hokein added a comment.
update comments.
https://reviews.llvm.org/D31176
Files:
clang-rename/CMakeLists.txt
clang-rename/RenamingAction.cpp
clang-rename/RenamingAction.h
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
clang-rename/USRLocFinder.cpp
clang-rename/USRLocFinder.h
unittests/clang-rename/CMakeLists.txt
unittests/clang-rename/ClangRenameTest.h
unittests/clang-rename/ClangRenameTests.cpp
unittests/clang-rename/RenameClassTest.cpp
Index: unittests/clang-rename/RenameClassTest.cpp
===
--- /dev/null
+++ unittests/clang-rename/RenameClassTest.cpp
@@ -0,0 +1,668 @@
+//===-- RenameClassTest.cpp - unit tests for renaming classes -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameClassTest : public ClangRenameTest {
+public:
+ RenameClassTest() {
+AppendToHeader(R"(
+ namespace a {
+class Foo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+void func() {}
+ static int Constant;
+};
+class Goo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+};
+int Foo::Constant = 1;
+ } // namespace a
+ namespace b {
+ class Foo {};
+ } // namespace b
+
+ #define MACRO(x) x
+
+ template class ptr {};
+)");
+ }
+};
+
+INSTANTIATE_TEST_CASE_P(
+RenameClassTests, RenameClassTest,
+testing::ValuesIn(std::vector({
+// basic classes
+{"a::Foo f;", "b::Bar f;"},
+{"void f(a::Foo f) {}", "void f(b::Bar f) {}"},
+{"void f(a::Foo *f) {}", "void f(b::Bar *f) {}"},
+{"a::Foo f() { return a::Foo(); }", "b::Bar f() { return b::Bar(); }"},
+{"namespace a {a::Foo f() { return Foo(); }}",
+ "namespace a {b::Bar f() { return b::Bar(); }}"},
+{"void f(const a::Foo& a1) {}", "void f(const b::Bar& a1) {}"},
+{"void f(const a::Foo* a1) {}", "void f(const b::Bar* a1) {}"},
+{"namespace a { void f(Foo a1) {} }",
+ "namespace a { void f(b::Bar a1) {} }"},
+{"void f(MACRO(a::Foo) a1) {}", "void f(MACRO(b::Bar) a1) {}"},
+{"void f(MACRO(a::Foo a1)) {}", "void f(MACRO(b::Bar a1)) {}"},
+{"a::Foo::Nested ns;", "b::Bar::Nested ns;"},
+{"auto t = a::Foo::Constant;", "auto t = b::Bar::Constant;"},
+{"a::Foo::Nested ns;", "a::Foo::Nested2 ns;", "a::Foo::Nested",
+ "a::Foo::Nested2"},
+
+// use namespace and typedefs
+{"using a::Foo; Foo gA;", "using b::Bar; b::Bar gA;"},
+{"using a::Foo; void f(Foo gA) {}", "using b::Bar; void f(Bar gA) {}"},
+{"using a::Foo; namespace x { Foo gA; }",
+ "using b::Bar; namespace x { Bar gA; }"},
+{"struct S { using T = a::Foo; T a_; };",
+ "struct S { using T = b::Bar; T a_; };"},
+{"using T = a::Foo; T gA;", "using T = b::Bar; T gA;"},
+{"typedef a::Foo T; T gA;", "typedef b::Bar T; T gA;"},
+{"typedef MACRO(a::Foo) T; T gA;", "typedef MACRO(b::Bar) T; T gA;"},
+
+// struct members and other oddities
+{"struct S : public a::Foo {};", "struct S : public b::Bar {};"},
+{"struct F { void f(a::Foo a1) {} };",
+ "struct F { void f(b::Bar a1) {} };"},
+{"struct F { a::Foo a_; };", "struct F { b::Bar a_; };"},
+{"struct F { ptr a_; };", "struct F { ptr a_; };"},
+
+{"void f() { a::Foo::Nested ne; }", "void f() { b::Bar::Nested ne; }"},
+{"void f() { a::Goo::Nested ne; }", "void f() { a::Goo::Nested ne; }"},
+{"void f() { a::Foo::Nested::NestedEnum e; }",
+ "void f() { b::Bar::Nested::NestedEnum e; }"},
+{"void f() { auto e = a::Foo::Nested::NestedEnum::E1; }",
+ "void f() { auto e = b::Bar::Nested::NestedEnum::E1; }"},
+{"void f() { auto e = a::Foo::Nested::E1; }",
+ "void f() { auto e = b::Bar::Nested::E1; }"},
+
+// templates
+{"template struct Foo { T t; };\n"
+ "void f() { Foo foo; }",
+ "template struct Foo { T t; };\n"
+ "void f() { Foo foo; }"},
+{"template struct Foo { a::Foo a; };",
+ "template struct Foo { b::Bar a; };"},
+{"template void f(T t) {}\n"
+ "void g() { f(a::Foo()); }",
+ "template void f(T t) {}\n"
+ "void g() { f(b::Bar()); }"},
+{"template int f() { return 1; }\n"
+ "template <> int f() { return 2; }\n"
+
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:359
+
+ // Returns a list of using declarations which are needed to update.
+ const std::vector () const {
ioeric wrote:
> hokein wrote:
> > ioeric wrote:
> > > I think these are using shadows only?
> > These are interested `UsingDecl`s which contain `UsingShadowDecl` of the
> > symbol declarations being renamed.
> Sure. But maybe also document it?
I have documented it at the "UsingDecls" member definition.
Comment at: clang-rename/USRLocFinder.h:36
+/// \return Replacement for renaming.
+std::vector
+createRenameReplacement(llvm::ArrayRef USRs,
ioeric wrote:
> hokein wrote:
> > ioeric wrote:
> > > Why use `std::vector` instead of `tooling::Replacements`?
> > Seems that we don't get many benefits from using `tooling::Replacements`
> > here. This function could be called multiple times (for renaming multiple
> > symbols), we need to merge/add all replacements in caller side. if using
> > `tooling::Replacements`, we will merge twice (one is in the API
> > implementation).
> I think what we really want is `AtomicChange`. We shouldn't be using
> `std::vector` or `std::set` replacements anymore.
Good idea. Done. I think we might still need to refine the interface in the
future.
https://reviews.llvm.org/D31176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein updated this revision to Diff 93468.
hokein marked 2 inline comments as done.
hokein added a comment.
Use AtomicChange.
https://reviews.llvm.org/D31176
Files:
clang-rename/CMakeLists.txt
clang-rename/RenamingAction.cpp
clang-rename/RenamingAction.h
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
clang-rename/USRLocFinder.cpp
clang-rename/USRLocFinder.h
unittests/clang-rename/CMakeLists.txt
unittests/clang-rename/ClangRenameTest.h
unittests/clang-rename/ClangRenameTests.cpp
unittests/clang-rename/RenameClassTest.cpp
Index: unittests/clang-rename/RenameClassTest.cpp
===
--- /dev/null
+++ unittests/clang-rename/RenameClassTest.cpp
@@ -0,0 +1,668 @@
+//===-- RenameClassTest.cpp - unit tests for renaming classes -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameClassTest : public ClangRenameTest {
+public:
+ RenameClassTest() {
+AppendToHeader(R"(
+ namespace a {
+class Foo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+void func() {}
+ static int Constant;
+};
+class Goo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+};
+int Foo::Constant = 1;
+ } // namespace a
+ namespace b {
+ class Foo {};
+ } // namespace b
+
+ #define MACRO(x) x
+
+ template class ptr {};
+)");
+ }
+};
+
+INSTANTIATE_TEST_CASE_P(
+RenameClassTests, RenameClassTest,
+testing::ValuesIn(std::vector({
+// basic classes
+{"a::Foo f;", "b::Bar f;"},
+{"void f(a::Foo f) {}", "void f(b::Bar f) {}"},
+{"void f(a::Foo *f) {}", "void f(b::Bar *f) {}"},
+{"a::Foo f() { return a::Foo(); }", "b::Bar f() { return b::Bar(); }"},
+{"namespace a {a::Foo f() { return Foo(); }}",
+ "namespace a {b::Bar f() { return b::Bar(); }}"},
+{"void f(const a::Foo& a1) {}", "void f(const b::Bar& a1) {}"},
+{"void f(const a::Foo* a1) {}", "void f(const b::Bar* a1) {}"},
+{"namespace a { void f(Foo a1) {} }",
+ "namespace a { void f(b::Bar a1) {} }"},
+{"void f(MACRO(a::Foo) a1) {}", "void f(MACRO(b::Bar) a1) {}"},
+{"void f(MACRO(a::Foo a1)) {}", "void f(MACRO(b::Bar a1)) {}"},
+{"a::Foo::Nested ns;", "b::Bar::Nested ns;"},
+{"auto t = a::Foo::Constant;", "auto t = b::Bar::Constant;"},
+{"a::Foo::Nested ns;", "a::Foo::Nested2 ns;", "a::Foo::Nested",
+ "a::Foo::Nested2"},
+
+// use namespace and typedefs
+{"using a::Foo; Foo gA;", "using b::Bar; b::Bar gA;"},
+{"using a::Foo; void f(Foo gA) {}", "using b::Bar; void f(Bar gA) {}"},
+{"using a::Foo; namespace x { Foo gA; }",
+ "using b::Bar; namespace x { Bar gA; }"},
+{"struct S { using T = a::Foo; T a_; };",
+ "struct S { using T = b::Bar; T a_; };"},
+{"using T = a::Foo; T gA;", "using T = b::Bar; T gA;"},
+{"typedef a::Foo T; T gA;", "typedef b::Bar T; T gA;"},
+{"typedef MACRO(a::Foo) T; T gA;", "typedef MACRO(b::Bar) T; T gA;"},
+
+// struct members and other oddities
+{"struct S : public a::Foo {};", "struct S : public b::Bar {};"},
+{"struct F { void f(a::Foo a1) {} };",
+ "struct F { void f(b::Bar a1) {} };"},
+{"struct F { a::Foo a_; };", "struct F { b::Bar a_; };"},
+{"struct F { ptr a_; };", "struct F { ptr a_; };"},
+
+{"void f() { a::Foo::Nested ne; }", "void f() { b::Bar::Nested ne; }"},
+{"void f() { a::Goo::Nested ne; }", "void f() { a::Goo::Nested ne; }"},
+{"void f() { a::Foo::Nested::NestedEnum e; }",
+ "void f() { b::Bar::Nested::NestedEnum e; }"},
+{"void f() { auto e = a::Foo::Nested::NestedEnum::E1; }",
+ "void f() { auto e = b::Bar::Nested::NestedEnum::E1; }"},
+{"void f() { auto e = a::Foo::Nested::E1; }",
+ "void f() { auto e = b::Bar::Nested::E1; }"},
+
+// templates
+{"template struct Foo { T t; };\n"
+ "void f() { Foo foo; }",
+ "template struct Foo { T t; };\n"
+ "void f() { Foo foo; }"},
+{"template struct Foo { a::Foo a; };",
+ "template struct Foo { b::Bar a; };"},
+{"template void f(T t) {}\n"
+ "void g() { f(a::Foo()); }",
+ "template void f(T t) {}\n"
+ "void g() { f(b::Bar()); }"},
+{"template int f() { return 1; }\n"
+ "template <> int f() { return 2; }\n"
+
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
ioeric added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:195
+// Find all locations identified by the given USRs. Traverse the AST and find
+// every AST node whose USR is in the given USRs' set.
+class RenameLocFinder
hokein wrote:
> ioeric wrote:
> > I think this also does some renaming?
> No, this class is only responsible for finding rename locations and other
> information which are used for renaming. The renaming stuff is done by
> `USRSymbolRenamer`.
Sure. I meant you should also document it.
Comment at: clang-rename/USRLocFinder.cpp:359
+
+ // Returns a list of using declarations which are needed to update.
+ const std::vector () const {
hokein wrote:
> ioeric wrote:
> > I think these are using shadows only?
> These are interested `UsingDecl`s which contain `UsingShadowDecl` of the
> symbol declarations being renamed.
Sure. But maybe also document it?
Comment at: clang-rename/USRLocFinder.h:36
+/// \return Replacement for renaming.
+std::vector
+createRenameReplacement(llvm::ArrayRef USRs,
hokein wrote:
> ioeric wrote:
> > Why use `std::vector` instead of `tooling::Replacements`?
> Seems that we don't get many benefits from using `tooling::Replacements`
> here. This function could be called multiple times (for renaming multiple
> symbols), we need to merge/add all replacements in caller side. if using
> `tooling::Replacements`, we will merge twice (one is in the API
> implementation).
I think what we really want is `AtomicChange`. We shouldn't be using
`std::vector` or `std::set` replacements anymore.
https://reviews.llvm.org/D31176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:195
+// Find all locations identified by the given USRs. Traverse the AST and find
+// every AST node whose USR is in the given USRs' set.
+class RenameLocFinder
ioeric wrote:
> I think this also does some renaming?
No, this class is only responsible for finding rename locations and other
information which are used for renaming. The renaming stuff is done by
`USRSymbolRenamer`.
Comment at: clang-rename/USRLocFinder.cpp:217
+
+ // FIXME: For renaming declarations/definitions, prefix qualifiers should be
+ // filtered out.
ioeric wrote:
> Could you be more specific in this FIXME? I don't quite get it. Maybe an
> example?
You can also see the `FIXME` in the test.
Comment at: clang-rename/USRLocFinder.cpp:359
+
+ // Returns a list of using declarations which are needed to update.
+ const std::vector () const {
ioeric wrote:
> I think these are using shadows only?
These are interested `UsingDecl`s which contain `UsingShadowDecl` of the symbol
declarations being renamed.
Comment at: clang-rename/USRLocFinder.h:36
+/// \return Replacement for renaming.
+std::vector
+createRenameReplacement(llvm::ArrayRef USRs,
ioeric wrote:
> Why use `std::vector` instead of `tooling::Replacements`?
Seems that we don't get many benefits from using `tooling::Replacements` here.
This function could be called multiple times (for renaming multiple symbols),
we need to merge/add all replacements in caller side. if using
`tooling::Replacements`, we will merge twice (one is in the API implementation).
https://reviews.llvm.org/D31176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein updated this revision to Diff 92954.
hokein marked 3 inline comments as done.
hokein added a comment.
Add comments and polish tests.
https://reviews.llvm.org/D31176
Files:
clang-rename/RenamingAction.cpp
clang-rename/RenamingAction.h
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
clang-rename/USRLocFinder.cpp
clang-rename/USRLocFinder.h
unittests/clang-rename/CMakeLists.txt
unittests/clang-rename/ClangRenameTest.h
unittests/clang-rename/ClangRenameTests.cpp
unittests/clang-rename/RenameClassTest.cpp
Index: unittests/clang-rename/RenameClassTest.cpp
===
--- /dev/null
+++ unittests/clang-rename/RenameClassTest.cpp
@@ -0,0 +1,668 @@
+//===-- RenameClassTest.cpp - unit tests for renaming classes -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameClassTest : public ClangRenameTest {
+public:
+ RenameClassTest() {
+AppendToHeader(R"(
+ namespace a {
+class Foo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+void func() {}
+ static int Constant;
+};
+class Goo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+};
+int Foo::Constant = 1;
+ } // namespace a
+ namespace b {
+ class Foo {};
+ } // namespace b
+
+ #define MACRO(x) x
+
+ template class ptr {};
+)");
+ }
+};
+
+INSTANTIATE_TEST_CASE_P(
+RenameClassTests, RenameClassTest,
+testing::ValuesIn(std::vector({
+// basic classes
+{"a::Foo f;", "b::Bar f;"},
+{"void f(a::Foo f) {}", "void f(b::Bar f) {}"},
+{"void f(a::Foo *f) {}", "void f(b::Bar *f) {}"},
+{"a::Foo f() { return a::Foo(); }", "b::Bar f() { return b::Bar(); }"},
+{"namespace a {a::Foo f() { return Foo(); }}",
+ "namespace a {b::Bar f() { return b::Bar(); }}"},
+{"void f(const a::Foo& a1) {}", "void f(const b::Bar& a1) {}"},
+{"void f(const a::Foo* a1) {}", "void f(const b::Bar* a1) {}"},
+{"namespace a { void f(Foo a1) {} }",
+ "namespace a { void f(b::Bar a1) {} }"},
+{"void f(MACRO(a::Foo) a1) {}", "void f(MACRO(b::Bar) a1) {}"},
+{"void f(MACRO(a::Foo a1)) {}", "void f(MACRO(b::Bar a1)) {}"},
+{"a::Foo::Nested ns;", "b::Bar::Nested ns;"},
+{"auto t = a::Foo::Constant;", "auto t = b::Bar::Constant;"},
+{"a::Foo::Nested ns;", "a::Foo::Nested2 ns;", "a::Foo::Nested",
+ "a::Foo::Nested2"},
+
+// use namespace and typedefs
+{"using a::Foo; Foo gA;", "using b::Bar; b::Bar gA;"},
+{"using a::Foo; void f(Foo gA) {}", "using b::Bar; void f(Bar gA) {}"},
+{"using a::Foo; namespace x { Foo gA; }",
+ "using b::Bar; namespace x { Bar gA; }"},
+{"struct S { using T = a::Foo; T a_; };",
+ "struct S { using T = b::Bar; T a_; };"},
+{"using T = a::Foo; T gA;", "using T = b::Bar; T gA;"},
+{"typedef a::Foo T; T gA;", "typedef b::Bar T; T gA;"},
+{"typedef MACRO(a::Foo) T; T gA;", "typedef MACRO(b::Bar) T; T gA;"},
+
+// struct members and other oddities
+{"struct S : public a::Foo {};", "struct S : public b::Bar {};"},
+{"struct F { void f(a::Foo a1) {} };",
+ "struct F { void f(b::Bar a1) {} };"},
+{"struct F { a::Foo a_; };", "struct F { b::Bar a_; };"},
+{"struct F { ptr a_; };", "struct F { ptr a_; };"},
+
+{"void f() { a::Foo::Nested ne; }", "void f() { b::Bar::Nested ne; }"},
+{"void f() { a::Goo::Nested ne; }", "void f() { a::Goo::Nested ne; }"},
+{"void f() { a::Foo::Nested::NestedEnum e; }",
+ "void f() { b::Bar::Nested::NestedEnum e; }"},
+{"void f() { auto e = a::Foo::Nested::NestedEnum::E1; }",
+ "void f() { auto e = b::Bar::Nested::NestedEnum::E1; }"},
+{"void f() { auto e = a::Foo::Nested::E1; }",
+ "void f() { auto e = b::Bar::Nested::E1; }"},
+
+// templates
+{"template struct Foo { T t; };\n"
+ "void f() { Foo foo; }",
+ "template struct Foo { T t; };\n"
+ "void f() { Foo foo; }"},
+{"template struct Foo { a::Foo a; };",
+ "template struct Foo { b::Bar a; };"},
+{"template void f(T t) {}\n"
+ "void g() { f(a::Foo()); }",
+ "template void f(T t) {}\n"
+ "void g() { f(b::Bar()); }"},
+{"template int f() { return 1; }\n"
+ "template <> int f() { return 2; }\n"
+ "int g()
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
ioeric added inline comments.
Comment at: clang-rename/USRFinder.cpp:200
+ // Also find all USRs of nested declarations.
+ NestedNameSpecifierLocFinder Finder(const_cast(Context));
ioeric wrote:
> It is unclear to me what `nested declarations` are.
But what is the nested name? Is it the nested name specifier? Of what?
Comment at: clang-rename/USRLocFinder.cpp:195
+// Find all locations identified by the given USRs. Traverse the AST and find
+// every AST node whose USR is in the given USRs' set.
+class RenameLocFinder
I think this also does some renaming?
Comment at: clang-rename/USRLocFinder.cpp:217
+
+ // FIXME: For renaming declarations/definitions, prefix qualifiers should be
+ // filtered out.
Could you be more specific in this FIXME? I don't quite get it. Maybe an
example?
Comment at: clang-rename/USRLocFinder.cpp:359
+
+ // Returns a list of using declarations which are needed to update.
+ const std::vector () const {
I think these are using shadows only?
Comment at: clang-rename/USRLocFinder.h:36
+/// \return Replacement for renaming.
+std::vector
+createRenameReplacement(llvm::ArrayRef USRs,
Why use `std::vector` instead of `tooling::Replacements`?
https://reviews.llvm.org/D31176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:157
+clang::NestedNameSpecifierLoc nested_name_specifier =
+TL.castAs().getQualifierLoc();
+if (nested_name_specifier.getNestedNameSpecifier())
ioeric wrote:
> Is this cast always safe?
Yes, because we have checked the type of the TL is an elaborated type. I have
changed to `getAs` which makes the indication more explicitly.
Comment at: clang-rename/USRLocFinder.cpp:196
+
+class RenameLocFindingASTVisitor
+: public clang::RecursiveASTVisitor {
ioeric wrote:
> ioeric wrote:
> > Comments.
> I think this visitor deserves a file of its own.
I'm not sure it is the right time to do it at the moment since it is in early
stage. I'd keep it here. We can do it when needed.
Comment at: clang-rename/USRLocFinder.cpp:368
+private:
+ bool IsTypeAliasWhichWillBeRenamedElsewhere(TypeLoc TL) const {
+while (!TL.isNull()) {
ioeric wrote:
> Note this is not expected behavior for alias types which would always be
> skipped in this check.
Acked. Add a FIXME.
https://reviews.llvm.org/D31176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
hokein updated this revision to Diff 92636.
hokein marked 14 inline comments as done.
hokein added a comment.
Address review comments.
https://reviews.llvm.org/D31176
Files:
clang-rename/RenamingAction.cpp
clang-rename/RenamingAction.h
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
clang-rename/USRLocFinder.cpp
clang-rename/USRLocFinder.h
unittests/clang-rename/CMakeLists.txt
unittests/clang-rename/ClangRenameTest.h
unittests/clang-rename/ClangRenameTests.cpp
unittests/clang-rename/RenameClassTest.cpp
Index: unittests/clang-rename/RenameClassTest.cpp
===
--- /dev/null
+++ unittests/clang-rename/RenameClassTest.cpp
@@ -0,0 +1,670 @@
+//===-- RenameClassTest.cpp - unit tests for renaming classes -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameClassTest : public ClangRenameTest {
+public:
+ RenameClassTest() {
+AppendToHeader(R"(
+ namespace a {
+class Foo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+void func() {}
+};
+class Goo {
+ public:
+struct Nested {
+ enum NestedEnum {E1, E2};
+};
+};
+ } // namespace a
+ namespace b {
+ class Foo {};
+ } // namespace b
+
+ #define MACRO(x) x
+
+ template class ptr {};
+)");
+ }
+};
+
+INSTANTIATE_TEST_CASE_P(
+RenameClassTests, RenameClassTest,
+testing::ValuesIn(std::vector({
+// basic classes
+{"a::Foo f;", "b::Bar f;", "a::Foo", "b::Bar"},
+{"void f(a::Foo f) {}", "void f(b::Bar f) {}", "a::Foo", "b::Bar"},
+{"void f(a::Foo *f) {}", "void f(b::Bar *f) {}", "a::Foo", "b::Bar"},
+{"a::Foo f() { return a::Foo(); }", "b::Bar f() { return b::Bar(); }",
+ "a::Foo", "b::Bar"},
+{"namespace a {a::Foo f() { return Foo(); }}",
+ "namespace a {b::Bar f() { return b::Bar(); }}", "a::Foo", "b::Bar"},
+{"void f(const a::Foo& a1) {}", "void f(const b::Bar& a1) {}", "a::Foo",
+ "b::Bar"},
+{"void f(const a::Foo* a1) {}", "void f(const b::Bar* a1) {}", "a::Foo",
+ "b::Bar"},
+{"namespace a { void f(Foo a1) {} }",
+ "namespace a { void f(b::Bar a1) {} }", "a::Foo", "b::Bar"},
+{"void f(MACRO(a::Foo) a1) {}", "void f(MACRO(b::Bar) a1) {}", "a::Foo",
+ "b::Bar"},
+{"void f(MACRO(a::Foo a1)) {}", "void f(MACRO(b::Bar a1)) {}", "a::Foo",
+ "b::Bar"},
+
+// use namespace and typedefs
+{"using a::Foo; Foo gA;", "using b::Bar; b::Bar gA;"},
+{"using a::Foo; void f(Foo gA) {}", "using b::Bar; void f(Bar gA) {}"},
+{"using a::Foo; namespace x { Foo gA; }",
+ "using b::Bar; namespace x { Bar gA; }"},
+{"struct S { using T = a::Foo; T a_; };",
+ "struct S { using T = b::Bar; T a_; };"},
+{"using T = a::Foo; T gA;", "using T = b::Bar; T gA;"},
+{"typedef a::Foo T; T gA;", "typedef b::Bar T; T gA;"},
+{"typedef MACRO(a::Foo) T; T gA;", "typedef MACRO(b::Bar) T; T gA;"},
+
+// struct members and other oddities
+{"struct S : public a::Foo {};", "struct S : public b::Bar {};"},
+{"struct F { void f(a::Foo a1) {} };",
+ "struct F { void f(b::Bar a1) {} };"},
+{"struct F { a::Foo a_; };", "struct F { b::Bar a_; };"},
+{"struct F { ptr a_; };", "struct F { ptr a_; };"},
+
+{"void f() { a::Foo::Nested ne; }", "void f() { b::Bar::Nested ne; }",
+ "a::Foo", "b::Bar"},
+{"void f() { a::Goo::Nested ne; }", "void f() { a::Goo::Nested ne; }",
+ "a::Foo", "b::Bar"},
+{"void f() { a::Foo::Nested::NestedEnum e; }",
+ "void f() { b::Bar::Nested::NestedEnum e; }", "a::Foo", "b::Bar"},
+{"void f() { auto e = a::Foo::Nested::NestedEnum::E1; }",
+ "void f() { auto e = b::Bar::Nested::NestedEnum::E1; }", "a::Foo",
+ "b::Bar"},
+{"void f() { auto e = a::Foo::Nested::E1; }",
+ "void f() { auto e = b::Bar::Nested::E1; }", "a::Foo", "b::Bar"},
+
+// templates
+{"template struct Foo { T t; };\n"
+ "void f() { Foo foo; }",
+ "template struct Foo { T t; };\n"
+ "void f() { Foo foo; }"},
+{"template struct Foo { a::Foo a; };",
+ "template struct Foo { b::Bar a; };"},
+{"template void f(T t) {}\n"
+ "void g() { f(a::Foo()); }",
+ "template void f(T t) {}\n"
+ "void g() { f(b::Bar()); }"},
+{"template int f() {
[PATCH] D31176: [clang-rename] Support renaming qualified symbol
ioeric added a comment.
I think this is a great start!
First round with some nits.
Comment at: clang-rename/RenamingAction.cpp:87
+class QualifiedRenamingASTConsumer : public ASTConsumer {
+public:
Comments.
Comment at: clang-rename/RenamingAction.h:45
+class QualifiedRenamingAction {
+public:
Comments. Same for other classes.
Comment at: clang-rename/RenamingAction.h:60
+ const std::vector
+ const std::vector
+ std::map
What is `USRList`?
Comment at: clang-rename/USRFinder.cpp:139
+ if (Name != Decl->getQualifiedNameAsString() &&
+ Name != "::" + Decl->getQualifiedNameAsString()) {
return true;
nit: No braces for one-liners.
Comment at: clang-rename/USRFinder.cpp:200
+ // Also find all USRs of nested declarations.
+ NestedNameSpecifierLocFinder Finder(const_cast(Context));
It is unclear to me what `nested declarations` are.
Comment at: clang-rename/USRLocFinder.cpp:152
+clang::SourceLocation StartLocationForType(clang::TypeLoc TL) {
+ // For elaborated types (e.g. `struct a::A`) we want the portion after the
You don't need "clang::" here and elsewhere.
Comment at: clang-rename/USRLocFinder.cpp:156
+ if (TL.getTypeLocClass() == clang::TypeLoc::Elaborated) {
+clang::NestedNameSpecifierLoc nested_name_specifier =
+TL.castAs().getQualifierLoc();
Variable names should be LLVM style.
Comment at: clang-rename/USRLocFinder.cpp:157
+clang::NestedNameSpecifierLoc nested_name_specifier =
+TL.castAs().getQualifierLoc();
+if (nested_name_specifier.getNestedNameSpecifier())
Is this cast always safe?
Comment at: clang-rename/USRLocFinder.cpp:169
+ TL.getTypeLocClass() == clang::TypeLoc::Qualified) {
+TL = TL.getNextTypeLoc();
+ }
nit: No braces around one-liners. Same below.
Comment at: clang-rename/USRLocFinder.cpp:196
+
+class RenameLocFindingASTVisitor
+: public clang::RecursiveASTVisitor {
Comments.
Comment at: clang-rename/USRLocFinder.cpp:196
+
+class RenameLocFindingASTVisitor
+: public clang::RecursiveASTVisitor {
ioeric wrote:
> Comments.
I think this visitor deserves a file of its own.
Comment at: clang-rename/USRLocFinder.cpp:368
+private:
+ bool IsTypeAliasWhichWillBeRenamedElsewhere(TypeLoc TL) const {
+while (!TL.isNull()) {
Note this is not expected behavior for alias types which would always be
skipped in this check.
Comment at: clang-rename/USRLocFinder.cpp:466
+
+ for (const auto : Finder.getRenameInfos()) {
+std::string ReplacedName = NewName.str();
`Loc` seems to be a bad name here. It is usually used for TypeLoc.
Comment at: clang-rename/USRLocFinder.h:34
+std::vector
+getQualifiedRenameLocsOfUSRs(llvm::ArrayRef USRs,
+ llvm::StringRef OldName, llvm::StringRef NewName,
Comments.
https://reviews.llvm.org/D31176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
