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

[Benjamin Kramer via cfe-commits]

bkramer added inline comments.


================
Comment at: change-namespace/ChangeNamespace.cpp:546
+                 << llvm::toString(std::move(Err)) << "\n";
+    assert(false);
+  }
----------------
So is this an error or not? If you can hit this by using the tool it should bail out here. If not use llvm_unreachable instead of assert(false).


================
Comment at: change-namespace/ChangeNamespace.cpp:637
+                 << llvm::toString(std::move(Err)) << "\n";
+    assert(false);
+  }
----------------
same here.


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


================
Comment at: change-namespace/ChangeNamespace.cpp:707
+                 << llvm::toString(std::move(Err)) << "\n";
+    assert(false);
+  }
----------------
same here.


================
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 {
----------------
You cannot rely on an STL implementation being around in a test. Unless we have a fake <functional> lying around this will fail in some environments. (I know this has been around for a while, just bringing it up now)


https://reviews.llvm.org/D26637