sammccall added a comment.
Thanks, this seems really useful!
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:59
-std::vector<std::string> parseDriverOutput(llvm::StringRef Output) {
+std::pair<std::vector<std::string>, std::string>
+parseDriverOutput(llvm::StringRef Output) {
----------------
define a little struct holding these two, for clarity?
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:78
+ } else {
+ // Start from the beginning if target was not found.
+ StartIt = Lines.begin();
----------------
This part seems pretty confusing :-)
You could just search for Target: independently of the current scanning - I'm
not terribly worried about it appearing in the middle of the include list!
If we want to be strict though, it might be time to rewrite to be more stateful
e.g. a loop consuming chunks from an ArrayRef<StringRef>, or a state machine,
or something.
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:87
+ return {SystemIncludes, Target};
}
++StartIt;
----------------
this changes the error behavior: intentional?
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:212
+ if (!Target.empty()) {
+ Cmd.CommandLine.push_back("-target");
+ Cmd.CommandLine.push_back(Target);
----------------
`clang -target foo test.cc` seems to be a hard error in the driver if the
target is unknown.
(vs likely *some* functionality if we just didn't set the driver)
so this could regress some scenarios. Can we mitigate that?
(It's possible that we're running the driver in a mode where we proceed anyway,
but I can't remember :-()
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:307
+ addSystemIncludes(*Cmd, SystemIncludesAndTarget.first);
+ return setTarget(*Cmd, SystemIncludesAndTarget.second);
}
----------------
nit: while here, add std::move?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92012/new/
https://reviews.llvm.org/D92012
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
