[PATCH] D26637: [change-namespace] handle constructor initializer: Derived : Base::Base() {} and added conflict detections

[Haojian Wu via cfe-commits]

hokein added inline comments.


================
Comment at: change-namespace/ChangeNamespace.cpp:346
+                             hasDeclaration(DeclMatcher.bind("from_decl"))))),
+                         unless(hasAncestor(typeLoc(loc(qualType(hasDeclaration(
+                             decl(equalsBoundNode("from_decl")))))))))
----------------
Maybe pull this `unless` matcher out and name it to make code more understandable.


================
Comment at: change-namespace/ChangeNamespace.cpp:650
+  // Types of CXXCtorInitializers do not need to be fixed.
+  for (const auto &T : BaseCtorInitializerTypeLocs)
+    if (Type == T)
----------------
use `std::find`?


================
Comment at: test/change-namespace/lambda-function.cpp:2
+// RUN: clang-change-namespace -old_namespace "na::nb" -new_namespace "x::y" --file_pattern ".*" %s -- -std=c++11 | sed 's,// CHECK.*,,' | FileCheck %s
+#include <functional>
+// CHECK: namespace x {
----------------
In practise, we don't use std headers directly in llvm lit test. You need to mock it by yourself...


================
Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:47
+    if (!tooling::runToolOnCodeWithArgs(Factory->create(), Code, {"-std=c++11"},
+                                   FileName))
+      return "";
----------------
nit: code indentation.


https://reviews.llvm.org/D26637