[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 8 14:47:52 PST 2016


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

LGTM with one nit.



================
Comment at: change-namespace/ChangeNamespace.cpp:275
+       (DiffOldNsSplitted.empty() ? "-" : DiffOldNsSplitted.front()))
+          .str();
+  auto IsInMovedNs =
----------------
ioeric wrote:
> hokein wrote:
> > Using an invalid name `-` is not an elegant solution to me. Is it possible to avoid it? 
> > Maybe we can explicitly specify `IsVisibleInNewNs` using the code like:
> > 
> > ```
> > Optional<ast_matchers::internal::Matcher<NamedDecl>> IsVisibleInNewNs = IsInMovedNs;
> > if (!DiffOldNsSplitted.empty() )  {
> >   std::string Prefix = ... 
> >   IsVisibleInNewNs = anyOf(*IsVisibleInNewNs, unless(hasAncestor(namespaceDecl(hasName(Prefix));
> > }
> > ```
> As per offline discussion, this seems to be impossible.
OK, then add a comment explicitly specifying that `"-"` is used as an invalid name.

I think the code can be simplified as:

```
std::string Prefix = "-";
if (!DiffOldNsSplitted.empty()) {
  Prefix =  (StringRef(FullOldNs).drop_back(DiffOldNamespace.size()) + DiffOldNsSplitted.front()).str();
}
```


https://reviews.llvm.org/D25771





More information about the cfe-commits mailing list