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

[Aleksandr Platonov via Phabricator via cfe-commits]

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<TargetOptions> TargetOpts(new TargetOptions);
----------------
kadircet wrote:
> i think you can just do `TargetRegistry::lookupTarget`
Seems we can't. `TargetRegistry::lookupTarget` returns `nullptr` for `arm-linux-gnueabihf` with error message: `No available targets are compatible with triple "arm-linux-gnueabihf"`


================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:337
 
-    std::vector<std::string> SystemIncludes =
+    llvm::Optional<DriverInfo> Info =
         QueriedDrivers.get(/*Key=*/(Driver + ":" + Lang).str(), [&] {
----------------
kadircet wrote:
> nit:
> ```
> if(auto Info = ...)
>   setTarget(addSystemIncludes(...), ...);
> return std::move(Cmd);
> ```
I used your advice except `std::move(Cmd)`, seems we do not need `std::move` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92012