tom-anders added a comment.
This looks really cool :)
================
Comment at:
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:133
+ std::string Directives;
+ for (auto &Using : UsingToAdd)
+ Directives += llvm::formatv("using namespace {0};\n", Using);
----------------
nit: `const auto`
================
Comment at:
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:249
+ llvm::sort(UsingToAdd);
+ UsingToAdd.erase(std::unique(UsingToAdd.begin(), UsingToAdd.end()),
----------------
If you're sorting and removing duplicates in the end anyway, maybe [[
https://llvm.org/doxygen/classllvm_1_1StringSet.html | llvm::StringSet ]] would
be a better fit instead of `vector<string>`?
================
Comment at:
clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:286
+ using namespace ns1::ns2;
int main() { 1.5_w; }
)cpp"}};
----------------
Your code also handles cases where there are multiple inline namespaces
involved, right? If so, it would be a good idea also test this here (i.e. add
ns3 with another user-defined literal)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137817/new/
https://reviews.llvm.org/D137817
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
