[PATCH] D107632: [clangd] Avoid "expected one compiler job" by picking the first eligible job.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

This happens in createInvocationWithCommandLine but only clangd currently passes
ShouldRecoverOnErorrs (sic).

One cause of this (with correct command) is several -arch arguments for mac
multi-arch support.

Fixes https://github.com/clangd/clangd/issues/827


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107632

Files:
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/CompilerTests.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp


Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===================================================================
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -79,22 +79,24 @@
       }
     }
   }
-  if (Jobs.size() == 0 || !isa<driver::Command>(*Jobs.begin()) ||
-      (Jobs.size() > 1 && !OffloadCompilation)) {
+
+  bool PickFirstOfMany = OffloadCompilation || ShouldRecoverOnErorrs;
+  if (Jobs.size() == 0 || (Jobs.size() > 1 && !PickFirstOfMany)) {
     SmallString<256> Msg;
     llvm::raw_svector_ostream OS(Msg);
     Jobs.Print(OS, "; ", true);
     Diags->Report(diag::err_fe_expected_compiler_job) << OS.str();
     return nullptr;
   }
-
-  const driver::Command &Cmd = cast<driver::Command>(*Jobs.begin());
-  if (StringRef(Cmd.getCreator().getName()) != "clang") {
+  auto Cmd = llvm::find_if(Jobs, [](const driver::Command &Cmd) {
+    return StringRef(Cmd.getCreator().getName()) == "clang";
+  });
+  if (Cmd == Jobs.end()) {
     Diags->Report(diag::err_fe_expected_clang_command);
     return nullptr;
   }
 
-  const ArgStringList &CCArgs = Cmd.getArguments();
+  const ArgStringList &CCArgs = Cmd->getArguments();
   if (CC1Args)
     *CC1Args = {CCArgs.begin(), CCArgs.end()};
   auto CI = std::make_unique<CompilerInvocation>();
Index: clang-tools-extra/clangd/unittests/CompilerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CompilerTests.cpp
+++ clang-tools-extra/clangd/unittests/CompilerTests.cpp
@@ -18,6 +18,7 @@
 namespace clangd {
 namespace {
 
+using testing::ElementsAre;
 using testing::IsEmpty;
 
 TEST(BuildCompilerInvocation, DropsPCH) {
@@ -53,6 +54,22 @@
               IsEmpty());
 }
 
+MATCHER_P(Named, Name, "") { return arg.Name == Name; }
+
+TEST(BuildCompilerInvocation, MultiArch) {
+  TestTU TU = TestTU::withHeaderCode(R"cpp(
+    #ifdef __x86_64__
+    bool is64;
+    #elifdef __i386__
+    bool is32;
+    #endif
+  )cpp");
+  TU.ExtraArgs = {
+      "--target=x86_64-apple-darwin20", // Needed for driver multi-arch.
+      "-arch", "i386", "-arch", "x86_64"};
+  EXPECT_THAT(TU.headerSymbols(), ElementsAre(Named("is32")));
+}
+
 TEST(BuildCompilerInvocation, PragmaDebugCrash) {
   TestTU TU = TestTU::withCode("#pragma clang __debug parser_crash");
   TU.build(); // no-crash
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -624,10 +624,8 @@
   ClangdServer Server(CDB, FS, ClangdServer::optsForTest(), &DiagConsumer);
 
   auto FooCpp = testPath("foo.cpp");
-  // clang cannot create CompilerInvocation if we pass two files in the
-  // CompileCommand. We pass the file in ExtraFlags once and CDB adds another
-  // one in getCompileCommand().
-  CDB.ExtraClangFlags.push_back(FooCpp);
+  // clang cannot create CompilerInvocation in this case.
+  CDB.ExtraClangFlags.push_back("-###");
 
   // Clang can't parse command args in that case, but we shouldn't crash.
   runAddDocument(Server, FooCpp, "int main() {}");
@@ -1080,7 +1078,7 @@
   Opts.RunParser = CodeCompleteOptions::ParseIfReady;
 
   // This will make compile command broken and preamble absent.
-  CDB.ExtraClangFlags = {"yolo.cc"};
+  CDB.ExtraClangFlags = {"-###"};
   Server.addDocument(FooCpp, Code.code());
   ASSERT_TRUE(Server.blockUntilIdleForTest());
   auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107632.364738.patch
Type: text/x-patch
Size: 3614 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210806/0b3cf7fa/attachment.bin>