kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/CompilerTests.cpp:59
+
+TEST(BuildCompilerInvocation, MultiArch) {
+ TestTU TU = TestTU::withHeaderCode(R"cpp(
----------------
maybe move this test to `clang/unittests/Frontend/ASTUnitTest.cpp`? someone
might send a patch to handle `-arch` specifically in clangd and it would no
longer be possible to test this behaviour :)
(another option would be to test this through some other flags that'll trigger
multiple compiler invocations, but I don't have any on top of my head)
================
Comment at: clang-tools-extra/clangd/unittests/CompilerTests.cpp:68
+ TU.ExtraArgs = {
+ "--target=x86_64-apple-darwin20", // Needed for driver multi-arch.
+ "-arch", "i386", "-arch", "x86_64"};
----------------
nit: `--target=macho` is enough. and AFAICT root reason to support multiple
architectures is mach object type.
================
Comment at: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp:92
- const driver::Command &Cmd = cast<driver::Command>(*Jobs.begin());
- if (StringRef(Cmd.getCreator().getName()) != "clang") {
Diags->Report(diag::err_fe_expected_clang_command);
----------------
hmm, this looks like a subtle behaviour change. I definitely like the behaviour
you proposed better, but maybe do it in a separate patch for making the revert
easier if need be?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107632/new/
https://reviews.llvm.org/D107632
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
