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

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
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())
----------------
hokein wrote:
> 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). 
This is also for correctness since it is really not a conflict when symbol and namespace has the same outer-most namespace. I could've dropped "the symbol name would have been shortened" part. 


https://reviews.llvm.org/D30493