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

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 21 05:14:43 PDT 2017


hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Sorry, I missed this patch.

LGTM with one nit.



================
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())
----------------
ioeric wrote:
> hokein wrote:
> > Why skipping the first element? And use `is_contained` instead?
> See newly added comments for reasoning.
I see. This sounds the `conflictInNamespace` is too coupled with caller because it relies on "it equals to the symbol's outermost namespace and the symbol name would have been shortened" assumption. It is not straightforward especially for readers who read the code at the first time.  So I'd like to search from 0 (and this operation is trivial). 


https://reviews.llvm.org/D30493





More information about the cfe-commits mailing list