[PATCH] D27758: [change-namespace] don't crash when type reference is in function type parameter list.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289672: [change-namespace] don't crash when type reference 
is in function type… (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D27758?vs=81388&id=81400#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27758

Files:
  clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
  clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp


Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
===================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
@@ -172,6 +172,16 @@
       ReplacementText);
 }
 
+void addReplacementOrDie(
+    SourceLocation Start, SourceLocation End, llvm::StringRef ReplacementText,
+    const SourceManager &SM,
+    std::map<std::string, tooling::Replacements> *FileToReplacements) {
+  const auto R = createReplacement(Start, End, ReplacementText, SM);
+  auto Err = (*FileToReplacements)[R.getFilePath()].add(R);
+  if (Err)
+    llvm_unreachable(llvm::toString(std::move(Err)).c_str());
+}
+
 tooling::Replacement createInsertion(SourceLocation Loc,
                                      llvm::StringRef InsertText,
                                      const SourceManager &SM) {
@@ -574,11 +584,8 @@
   if (AfterSemi.isValid())
     End = AfterSemi.getLocWithOffset(-1);
   // Delete the forward declaration from the code to be moved.
-  const auto Deletion =
-      createReplacement(Start, End, "", *Result.SourceManager);
-  auto Err = FileToReplacements[Deletion.getFilePath()].add(Deletion);
-  if (Err)
-    llvm_unreachable(llvm::toString(std::move(Err)).c_str());
+  addReplacementOrDie(Start, End, "", *Result.SourceManager,
+                      &FileToReplacements);
   llvm::StringRef Code = Lexer::getSourceText(
       CharSourceRange::getTokenRange(
           Result.SourceManager->getSpellingLoc(Start),
@@ -608,6 +615,18 @@
     const DeclContext *DeclCtx, SourceLocation Start, SourceLocation End,
     const NamedDecl *FromDecl) {
   const auto *NsDeclContext = DeclCtx->getEnclosingNamespaceContext();
+  if (llvm::isa<TranslationUnitDecl>(NsDeclContext)) {
+    // This should not happen in usual unless the TypeLoc is in function type
+    // parameters, e.g `std::function<void(T)>`. In this case, DeclContext of
+    // `T` will be the translation unit. We simply use fully-qualified name
+    // here.
+    // Note that `FromDecl` must not be defined in the old namespace (according
+    // to `DeclMatcher`), so its fully-qualified name will not change after
+    // changing the namespace.
+    addReplacementOrDie(Start, End, FromDecl->getQualifiedNameAsString(),
+                        *Result.SourceManager, &FileToReplacements);
+    return;
+  }
   const auto *NsDecl = llvm::cast<NamespaceDecl>(NsDeclContext);
   // Calculate the name of the `NsDecl` after it is moved to new namespace.
   std::string OldNs = NsDecl->getQualifiedNameAsString();
@@ -667,10 +686,8 @@
   // NewNamespace is the global namespace.
   if (ReplaceName == FromDeclName && !NewNamespace.empty())
     ReplaceName = "::" + ReplaceName;
-  auto R = createReplacement(Start, End, ReplaceName, *Result.SourceManager);
-  auto Err = FileToReplacements[R.getFilePath()].add(R);
-  if (Err)
-    llvm_unreachable(llvm::toString(std::move(Err)).c_str());
+  addReplacementOrDie(Start, End, ReplaceName, *Result.SourceManager,
+                      &FileToReplacements);
 }
 
 // Replace the [Start, End] of `Type` with the shortest qualified name when the
@@ -731,11 +748,8 @@
   // FIXME: check if target_decl_name is in moved ns, which doesn't make much
   // sense. If this happens, we need to use name with the new namespace.
   // Use fully qualified name in UsingDecl for now.
-  auto R = createReplacement(Start, End, "using ::" + TargetDeclName,
-                             *Result.SourceManager);
-  auto Err = FileToReplacements[R.getFilePath()].add(R);
-  if (Err)
-    llvm_unreachable(llvm::toString(std::move(Err)).c_str());
+  addReplacementOrDie(Start, End, "using ::" + TargetDeclName,
+                      *Result.SourceManager, &FileToReplacements);
 }
 
 void ChangeNamespaceTool::fixDeclRefExpr(
Index: clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp
===================================================================
--- clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp
+++ clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp
@@ -10,12 +10,27 @@
   R operator()(ArgTypes...) const {}
 };
 
+namespace x {
 // CHECK: namespace x {
-// CHECK-NEXT: namespace y {
+class X {};
+}
+
 namespace na {
 namespace nb {
+// CHECK: namespace x {
+// CHECK-NEXT: namespace y {
 void f(function<void(int)> func, int param) { func(param); }
 void g() { f([](int x) {}, 1); }
+
+// x::X in function type parameter list will have translation unit context, so
+// we simply replace it with fully-qualified name.
+using TX = function<x::X(x::X)>;
+// CHECK: using TX = function<X(x::X)>;
+
+class A {};
+using TA = function<A(A)>;
+// CHECK: using TA = function<A(A)>;
+
 // CHECK: } // namespace y
 // CHECK-NEXT: } // namespace x
 }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits