[clang-tools-extra] r296604 - [change-namespace] get insertion points of forward declarations correct.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Mar  1 04:29:39 2017
New Revision: 296604

URL: http://llvm.org/viewvc/llvm-project?rev=296604&view=rev
Log:
[change-namespace] get insertion points of forward declarations correct.

Summary:
Previously, the insertion points would conflict with the old namespace
deletion.

Reviewers: hokein

Reviewed By: hokein

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D30490

Modified:
    clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
    clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp

Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=296604&r1=296603&r2=296604&view=diff
==============================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Wed Mar  1 04:29:39 2017
@@ -676,19 +676,18 @@ void ChangeNamespaceTool::moveClassForwa
     const NamedDecl *FwdDecl) {
   SourceLocation Start = FwdDecl->getLocStart();
   SourceLocation End = FwdDecl->getLocEnd();
+  const SourceManager &SM = *Result.SourceManager;
   SourceLocation AfterSemi = Lexer::findLocationAfterToken(
-      End, tok::semi, *Result.SourceManager, Result.Context->getLangOpts(),
+      End, tok::semi, SM, Result.Context->getLangOpts(),
       /*SkipTrailingWhitespaceAndNewLine=*/true);
   if (AfterSemi.isValid())
     End = AfterSemi.getLocWithOffset(-1);
   // Delete the forward declaration from the code to be moved.
-  addReplacementOrDie(Start, End, "", *Result.SourceManager,
-                      &FileToReplacements);
+  addReplacementOrDie(Start, End, "", SM, &FileToReplacements);
   llvm::StringRef Code = Lexer::getSourceText(
-      CharSourceRange::getTokenRange(
-          Result.SourceManager->getSpellingLoc(Start),
-          Result.SourceManager->getSpellingLoc(End)),
-      *Result.SourceManager, Result.Context->getLangOpts());
+      CharSourceRange::getTokenRange(SM.getSpellingLoc(Start),
+                                     SM.getSpellingLoc(End)),
+      SM, Result.Context->getLangOpts());
   // Insert the forward declaration back into the old namespace after moving the
   // code from old namespace to new namespace.
   // Insertion information is stored in `InsertFwdDecls` and actual
@@ -697,8 +696,9 @@ void ChangeNamespaceTool::moveClassForwa
   const auto *NsDecl = Result.Nodes.getNodeAs<NamespaceDecl>("ns_decl");
   // The namespace contains the forward declaration, so it must not be empty.
   assert(!NsDecl->decls_empty());
-  const auto Insertion = createInsertion(NsDecl->decls_begin()->getLocStart(),
-                                         Code, *Result.SourceManager);
+  const auto Insertion = createInsertion(
+      getLocAfterNamespaceLBrace(NsDecl, SM, Result.Context->getLangOpts()),
+      Code, SM);
   InsertForwardDeclaration InsertFwd;
   InsertFwd.InsertionOffset = Insertion.getOffset();
   InsertFwd.ForwardDeclText = Insertion.getReplacementText().str();

Modified: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp?rev=296604&r1=296603&r2=296604&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp Wed Mar  1 04:29:39 2017
@@ -375,6 +375,36 @@ TEST_F(ChangeNamespaceTest, LeaveForward
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, InsertForwardDeclsProperly) {
+  std::string Code = "namespace na {\n"
+                     "namespace nb {\n"
+                     "\n"
+                     "class FWD;\n"
+                     "class FWD2;\n"
+                     "class A {\n"
+                     "  FWD *fwd;\n"
+                     "};\n"
+                     "\n"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+  std::string Expected = "namespace na {\n"
+                         "namespace nb {\n"
+                         "class FWD;\n"
+                         "class FWD2;\n"
+                         "} // namespace nb\n"
+                         "} // namespace na\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "\n"
+                         "class A {\n"
+                         "  ::na::nb::FWD *fwd;\n"
+                         "};\n"
+                         "\n"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, TemplateClassForwardDeclaration) {
   std::string Code = "namespace na {\n"
                      "namespace nb {\n"