[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 13 05:21:19 PDT 2019


ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

Nice, thanks! The clangd parts obviously LGTM, but let's be more conservative with the driver bits (see the relevant comment)



================
Comment at: clang-tools-extra/clangd/test/target_info.test:28
+}
+# Make sure we have target passed into cc1 driver, which is printed due to -v in
+# the compile_commands.json
----------------
This is a marvellous hack!


================
Comment at: clang/lib/Driver/Driver.cpp:1055
+  else if (ClangNameParts.TargetIsValid)
+    TargetTriple = ClangNameParts.TargetPrefix;
+
----------------
I strongly think the driver is the right place to do this, but we have to watch out for breakages coming from this. The drivers code has way more uses than clangd code, so we should expect that people rely on all kinds of things there.

Could you separate the driver pieces into a separate change and add driver tests for them? To make it more focused and easier to revert in case of breakages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63194





More information about the cfe-commits mailing list