[PATCH] D69162: [clangd] Remove using-namespace present inside a namespace.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

One important question about running on the whole TU in all cases. Other than that LG



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:110
     return false;
-  if (!dyn_cast<Decl>(TargetDirective->getDeclContext()))
-    return false;
+  auto DeclCtx = TargetDirective->getDeclContext();
   // FIXME: Unavailable for namespaces containing using-namespace decl.
----------------
NIT: use `auto*`
knowing it's a pointer is quite useful when reading and writing the code (e.g. suggests one need to use `->` instead of `.` to access the variable)


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:179
+
+  if (ContainingNS) {
+    for (auto ReDeclNS : ContainingNS->redecls())
----------------
Could you explain why don't we always just run across the whole TU?
What disadvantages would that have?


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:182
+      findExplicitReferences(ReDeclNS, SelectRefToQualify);
+
+  } else {
----------------
NIT: remove empty line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69162/new/

https://reviews.llvm.org/D69162