[clang-tools-extra] 2ff7ca9 - [clangd] Avoid "expected one compiler job" by picking the first eligible job.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-08-13T00:36:30+02:00
New Revision: 2ff7ca98a99bbfc4f44b8ba1e2e4e594f8ee253e

URL: https://github.com/llvm/llvm-project/commit/2ff7ca98a99bbfc4f44b8ba1e2e4e594f8ee253e
DIFF: https://github.com/llvm/llvm-project/commit/2ff7ca98a99bbfc4f44b8ba1e2e4e594f8ee253e.diff

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

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

Differential Revision: https://reviews.llvm.org/D107632

Added: 
    clang/unittests/Frontend/UtilsTest.cpp

Modified: 
    clang-tools-extra/clangd/unittests/ClangdTests.cpp
    clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
    clang/unittests/Frontend/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp
index 07f5da1fbc52f..4a2cba52f93ab 100644
--- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -624,10 +624,8 @@ TEST(ClangdServerTest, InvalidCompileCommand) {
   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 @@ TEST(ClangdServerTest, FallbackWhenPreambleIsNotReady) {
   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));

diff  --git a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
index 2e23ebfdf1602..2a14b7aab52ee 100644
--- a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -79,22 +79,24 @@ std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
       }
     }
   }
-  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>();

diff  --git a/clang/unittests/Frontend/CMakeLists.txt b/clang/unittests/Frontend/CMakeLists.txt
index 3c25b43e95307..1a7b8193e1391 100644
--- a/clang/unittests/Frontend/CMakeLists.txt
+++ b/clang/unittests/Frontend/CMakeLists.txt
@@ -13,6 +13,7 @@ add_clang_unittest(FrontendTests
   PCHPreambleTest.cpp
   OutputStreamTest.cpp
   TextDiagnosticTest.cpp
+  UtilsTest.cpp
   )
 clang_target_link_libraries(FrontendTests
   PRIVATE

diff  --git a/clang/unittests/Frontend/UtilsTest.cpp b/clang/unittests/Frontend/UtilsTest.cpp
new file mode 100644
index 0000000000000..9f9c499591cef
--- /dev/null
+++ b/clang/unittests/Frontend/UtilsTest.cpp
@@ -0,0 +1,37 @@
+//===- unittests/Frontend/UtilsTest.cpp -----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Frontend/Utils.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace {
+
+TEST(BuildCompilerInvocationTest, RecoverMultipleJobs) {
+  // This generates multiple jobs and we recover by using the first.
+  std::vector<const char *> Args = {"clang", "--target=macho", "-arch",  "i386",
+                                    "-arch", "x86_64",         "foo.cpp"};
+  clang::IgnoringDiagConsumer D;
+  llvm::IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
+      clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, &D,
+                                                 false);
+  std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(
+      Args, CommandLineDiagsEngine, new llvm::vfs::InMemoryFileSystem(),
+      /*ShouldRecoverOnErrors=*/true);
+  ASSERT_TRUE(CI);
+  EXPECT_THAT(CI->TargetOpts->Triple, testing::StartsWith("i386-"));
+}
+
+} // namespace
+} // namespace clang