[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

v1nh1shungry via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 16 04:43:32 PST 2022


v1nh1shungry added a comment.

@kadircet Thank you for reviewing and giving suggestions! Please check my reply to your concerns. Sorry if I misunderstood anything.



================
Comment at: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294
+}
+using namespace ns1::ns2;
+using namespace ns1::ns3;
----------------
kadircet wrote:
> sorry i am having trouble understanding why we are:
> - only handling user defined literals from inline namespaces and not others?
> - producing using directives and not using declarations
> - inserting these at top level rather than where the usage is
The first question:

Because others have already been handled. Say

```
namespace a { inline namespace b { void foobar(); } }
using namespace ^a;
int main() { foobar(); }
```

can become

```
namespace a { inline namespace b { void foobar(); } }

int main() { a::foobar(); }
```

But user-defined literals just can't add a qualifier, right?

---

The second question:

Yes, this is a good idea. I didn't realize we can use using-declarations instead of using-directives.

---

The last question:

Hmm, I think it is cleaner if there are multiple usages. Since we can only remove the using-directives at the top level, this doesn't hurt anything. And it is the easiest solution to implement as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137817



More information about the cfe-commits mailing list