[PATCH] D30493: [change-namespace] avoid adding leading '::' when possible.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 2 06:38:47 PST 2017
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
More information about the cfe-commits
mailing list