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