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

[Eric Liu via cfe-commits]

ioeric added inline comments.


================
Comment at: change-namespace/ChangeNamespace.cpp:566
+      break;
+    if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) {
+      for (const auto *UsingShadow : Using->shadows()) {
----------------
hokein wrote:
> Yeah, it works for most cases, but it can not guarantee that they are still in the same DeclContext after changing to new namespace.
> 
> For example, the following case where an using-decl is used in the namespace being renamed, we change `b::c` to `d::e`, although DeclContext `d` of callexpr `f()` is a nested DeclContext of `b` (also DeclContext of `using a::f`), but this assumption is wrong after changing namespace because we keep `using a::f` in its original namespace.
> 
> ```
> namespace a { void f(); }
> 
> namespace b {
> using a::f;
> namespace c {
> void d() { f(); }
> }  // c
> } // b
> ```
> 
> Not sure whether we should do this in our tool. I suspect there might be a lot of corner cases.
> 
I thought using decls in old namespaces should be moved with along with namespaces. If this is the case, the moving of using decls is unexpected (looking into this), but this patch is handling this correctly if using decls are not moved.


https://reviews.llvm.org/D25771