[PATCH] D92012: [clangd][query-driver] Extract target

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ca174b6420a: [clangd][query-driver] Extract target (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92012/new/ https://reviews.llvm.org

[PATCH] D92012: [clangd][query-driver] Extract target

ArcsinX marked 4 inline comments as done. ArcsinX added a comment. Thanks for review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92012/new/ https://reviews.llvm.org/D92012 ___ cfe-commits mailing list

[PATCH] D92012: [clangd][query-driver] Extract target

sammccall accepted this revision. sammccall added a comment. Still LG, thanks! Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:344 +} +return Cmd; } kadircet wrote: > kadircet wrote: > > ofc we don't need it. but the thing is we are retu

[PATCH] D92012: [clangd][query-driver] Extract target

kadircet added inline comments. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:344 +} +return Cmd; } kadircet wrote: > ofc we don't need it. but the thing is we are returning an > `Optional` hence the return statement needs to invoke a

[PATCH] D92012: [clangd][query-driver] Extract target

kadircet added inline comments. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:344 +} +return Cmd; } ofc we don't need it. but the thing is we are returning an `Optional` hence the return statement needs to invoke a constructor for `Opt

[PATCH] D92012: [clangd][query-driver] Extract target

ArcsinX marked 14 inline comments as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:67 + +bool isValidTarget(llvm::StringRef Triple) { + std::shared_ptr TargetOpts(new TargetOptions); kadircet wrote: > i think y

[PATCH] D92012: [clangd][query-driver] Extract target

ArcsinX updated this revision to Diff 307773. ArcsinX added a comment. Do not override existing target. Fix clang-tidy warning. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92012/new/ https://reviews.llvm.org/D92012 Files

[PATCH] D92012: [clangd][query-driver] Extract target

sammccall added a comment. Sorry, code reviews are racy :-) Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:134 elog("System include extraction: end marker missing: {0}", Output); -return {}; - } - - for (llvm::StringRef Line : llvm::make_range(StartIt,

[PATCH] D92012: [clangd][query-driver] Extract target

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! just nits, apart from the "-target already explicitly set" issue Kadir raised. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:99 + } +

[PATCH] D92012: [clangd][query-driver] Extract target

kadircet added inline comments. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:67 + +bool isValidTarget(llvm::StringRef Triple) { + std::shared_ptr TargetOpts(new TargetOptions); i think you can just do `TargetRegistry::lookupTarget`

[PATCH] D92012: [clangd][query-driver] Extract target

ArcsinX marked 5 inline comments as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:134 elog("System include extraction: end marker missing: {0}", Output); -return {}; - } - - for (llvm::StringRef Line : llvm::make_rang

[PATCH] D92012: [clangd][query-driver] Extract target

ArcsinX updated this revision to Diff 307585. ArcsinX added a comment. Add structure to hold system includes and target. Use state machine to go though lines of the driver output. Handle invalid target Use '--target=X' instead of '-target X' Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D92012: [clangd][query-driver] Extract target

kadircet added inline comments. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:212 + if (!Target.empty()) { +Cmd.CommandLine.push_back("-target"); +Cmd.CommandLine.push_back(Target); sammccall wrote: > `clang -target foo test.cc` seems to b

[PATCH] D92012: [clangd][query-driver] Extract target

sammccall added a comment. Thanks, this seems really useful! Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:59 -std::vector parseDriverOutput(llvm::StringRef Output) { +std::pair, std::string> +parseDriverOutput(llvm::StringRef Output) { define

[PATCH] D92012: [clangd][query-driver] Extract target

ArcsinX added reviewers: sammccall, kadircet. ArcsinX added a comment. I'm not sure if this solution is elegant enough. I will be happy to hear advices. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92012/new/ https://reviews.llvm.org/D92012

[PATCH] D92012: [clangd][query-driver] Extract target

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, mstorsjo, kadircet, arphaman. Herald added a project: clang. ArcsinX requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. In some cases system includes extractions is not enough, we als