[PATCH] D30493: [change-namespace] avoid adding leading '::' when possible.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

I like removing the leading "::" when possible. :)



================
Comment at: change-namespace/ChangeNamespace.cpp:291
+  assert(!SymbolSplitted.empty());
+  SymbolSplitted.pop_back();
+
----------------
Is this needed? Looks like you are removing the name of the symbol here, but from the code below, you only use the first element of it.  The QualifiedSymbol should always be a fully-qualified name with at least 1 namespace qualifier in the code, right?


================
Comment at: change-namespace/ChangeNamespace.cpp:296
+    assert(!NsSplitted.empty());
+    for (auto I = NsSplitted.begin() + 1, E = NsSplitted.end(); I != E; ++I) {
+      if (*I == SymbolSplitted.front())
----------------
Why skipping the first element? And use `is_contained` instead?


================
Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:718
       "void f() {\n"
-      " auto *ref1 = ::na::A::f;\n"
-      " auto *ref2 = ::na::a_f;\n"
-      " auto *ref3 = ::na::s_f;\n"
+      " auto *ref1 = na::A::f;\n"
+      " auto *ref2 = na::a_f;\n"
----------------
The code style is not correct. Also fix it.


https://reviews.llvm.org/D30493