sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: cfe-commits.
Thanks for fixing!
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:225
+ Cmd = tooling::getInsertArgumentAdjuster(
+ ToAppend, tooling::ArgumentInsertPosition::END)(Cmd, "");
----------------
This extra copy is really silly :-(
please `move(ToAppend)` and make the whole thing conditional on
`!ToAppend.empty()` (even though that'll be pretty much always...). Sadly
move(Cmd) won't work.
One day if we feel like shaving a yak, we could add migrate to an ArgsAdjuster
interface that takes a mutable CompileCommand&...
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:271
Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) {
C.CompileFlags.Edits.push_back([Add](std::vector<std::string> &Args) {
+ auto It = llvm::find(Args, "--");
----------------
Hmm, with this new logic in place, if we "normalized" command lines by moving
the filename to the end, we'd resolve
https://github.com/clangd/clangd/issues/555 without having to complicate the
config file model.
I wonder how hard that is...
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:272
C.CompileFlags.Edits.push_back([Add](std::vector<std::string> &Args) {
- Args.insert(Args.end(), Add.begin(), Add.end());
+ auto It = llvm::find(Args, "--");
+ Args.insert(It, Add.begin(), Add.end());
----------------
maybe a comment "The point to insert at - usually the end"?
(Since this looks suspiciously like an unhandled error case)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99523/new/
https://reviews.llvm.org/D99523
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
