v1nh1shungry added inline comments.

================
Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:212
   // Produce replacements to add the qualifiers.
   std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + 
"::";
   for (auto Loc : IdentsToQualify) {
----------------
tom-anders wrote:
> v1nh1shungry wrote:
> > We can replace `printUsingNamespaceName` with `printNamespaceScope` here so 
> > that we can get `a::foobar()` in the test. 
> > 
> > However, it can sometimes cause redundancy such as in the 10th test. 
> > 
> > And I don't know whether it is worth it. WDYT?
> Just making sure I understood this correctly:
> 
> If you replace `printUsingNamespaceName` with `printNamespaceScope`, then...
> 
> - ...in the test you added it would result in `a::foobar()` instead of 
> `a::b::foobar()` (which is better)
> - ... but in this test (which is the 10th test if I counted correctly):
>      
> ```
>  namespace a::b { struct Foo {}; }
>   using namespace a;
>   using namespace a::[[b]];
>   using namespace b;
>   int main() { Foo F;}
> ```
> what would be the result..? would you get `a::Foo` instead of `a::b::Foo`?
> 
Sorry, I mean the next test. I read `10` from the inlay hint but I forgot the 
index starts from `0` :(

The test I want to mention:
```
namespace a::b { struct Foo {}; }
using namespace a;
using namespace a::b;
using namespace [[b]];
int main() { Foo F;}
```

We will get `a::b::Foo` in both the 10th and 11th tests. So in the 10th test, 
we don't get any benefits and don't sacrifice anything. In the 11th test, we 
get more redundancy than the existing version.

Apologize again for my mistake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to