[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:99
+    return false;
+  if (!dyn_cast<Decl>(TargetDirective->getDeclContext()))
+    return false;
----------------
ilya-biryukov wrote:
> I believe this check is redundant in presence of `isTopLevelDecl()`...
> (It used to check the 'using namespace' is not inside a function body)
Aah. Makes sense.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:781
+        namespace bb { struct map {}; }
+        using namespace bb; // Qualifies this.
+      }
----------------
ilya-biryukov wrote:
> Argh, this should definitely be fixed :-(
> One simple way to handle this particular only qualify references, which come **after** the first `using namespace` we are removing.
> 
> There's a `SourceManager::isBeforeInTranslationUnit` function that can be used to find out whether something is written before or after a particular point.
> 
> This won't fix all of the cases, but fixing in the general case seems hard.
We also need to qualify the map in such cases.
I have made an attempt to fix this by traversing all the parents namespace decl and checking whether they are equal to the concerned namespace.
But this introduces other problems:
```
      namespace aa { namespace bb { struct map {}; }}
      using namespace aa::bb;
      using namespace a^a;
      int main() {
        map m;
      }
```
get changed to 
```
      namespace aa { namespace bb { struct map {}; }}
      using namespace aa::bb;
      
      int main() {
        aa::map m;
      }
```
Here aa::map is invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562