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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits