[SourceKit] Recover if compiler arguments have errors

If the compiler arguments have errors in them (e.g. because a file with the same name is used twice), we can often still fulfill SourceKit requests because the compiler argument errors are only relevant for later stages of the compilation process.

Instead of bailing out early, do a best effor retrieving the compiler arguments that are valid and ignoring the errors.

Fixes rdar://77618144

Author: ahoppen

include/swift/Driver/Driver.h

@@ -262,13 +262,17 @@ class Driver {
   /// Construct a compilation object for a given ToolChain and command line
   /// argument vector.
   ///
+  /// If \p AllowErrors is set to \c true, this method tries to build a
+  /// compilation even if there were errors.
+  ///
   /// \return A Compilation, or nullptr if none was built for the given argument
   /// vector. A null return value does not necessarily indicate an error
   /// condition; the diagnostics should be queried to determine if an error
   /// occurred.
   std::unique_ptr<Compilation>
   buildCompilation(const ToolChain &TC,
-                   std::unique_ptr<llvm::opt::InputArgList> ArgList);
+                   std::unique_ptr<llvm::opt::InputArgList> ArgList,
+                   bool AllowErrors = false);
 
   /// Parse the given list of strings into an InputArgList.
   std::unique_ptr<llvm::opt::InputArgList>

lib/Driver/Driver.cpp

@@ -860,7 +860,8 @@ getDriverBatchSizeLimit(llvm::opt::InputArgList &ArgList,
 
 std::unique_ptr<Compilation>
 Driver::buildCompilation(const ToolChain &TC,
-                         std::unique_ptr<llvm::opt::InputArgList> ArgList) {
+                         std::unique_ptr<llvm::opt::InputArgList> ArgList,
+                         bool AllowErrors) {
   llvm::PrettyStackTraceString CrashInfo("Compilation construction");
 
   llvm::sys::TimePoint<> StartTime = std::chrono::system_clock::now();
@@ -880,7 +881,7 @@ Driver::buildCompilation(const ToolChain &TC,
   // Perform toolchain specific args validation.
   TC.validateArguments(Diags, *TranslatedArgList, DefaultTargetTriple);
 
-  if (Diags.hadAnyError())
+  if (Diags.hadAnyError() && !AllowErrors)
     return nullptr;
 
   if (!handleImmediateArgs(*TranslatedArgList, TC)) {
@@ -891,7 +892,7 @@ Driver::buildCompilation(const ToolChain &TC,
   InputFileList Inputs;
   buildInputs(TC, *TranslatedArgList, Inputs);
 
-  if (Diags.hadAnyError())
+  if (Diags.hadAnyError() && !AllowErrors)
     return nullptr;
 
   // Determine the OutputInfo for the driver.
@@ -900,14 +901,14 @@ Driver::buildCompilation(const ToolChain &TC,
   OI.CompilerMode = computeCompilerMode(*TranslatedArgList, Inputs, BatchMode);
   buildOutputInfo(TC, *TranslatedArgList, BatchMode, Inputs, OI);
 
-  if (Diags.hadAnyError())
+  if (Diags.hadAnyError() && !AllowErrors)
     return nullptr;
 
   assert(OI.CompilerOutputType != file_types::ID::TY_INVALID &&
          "buildOutputInfo() must set a valid output type!");
 
   TC.validateOutputInfo(Diags, OI);
-  if (Diags.hadAnyError())
+  if (Diags.hadAnyError() && !AllowErrors)
     return nullptr;
 
   validateEmbedBitcode(*TranslatedArgList, OI, Diags);
@@ -919,15 +920,17 @@ Driver::buildCompilation(const ToolChain &TC,
   Optional<OutputFileMap> OFM = buildOutputFileMap(
       *TranslatedArgList, workingDirectory);
 
-  if (Diags.hadAnyError())
+  if (Diags.hadAnyError() && !AllowErrors)
     return nullptr;
 
   if (ArgList->hasArg(options::OPT_driver_print_output_file_map)) {
     if (OFM)
       OFM->dump(llvm::errs(), true);
     else
       Diags.diagnose(SourceLoc(), diag::error_no_output_file_map_specified);
-    return nullptr;
+    if (!AllowErrors) {
+      return nullptr;
+    }
   }
 
   const bool ShowIncrementalBuildDecisions =
@@ -1044,7 +1047,7 @@ Driver::buildCompilation(const ToolChain &TC,
   buildActions(TopLevelActions, TC, OI,
                whyIgnoreIncrementallity.empty() ? &outOfDateMap : nullptr, *C);
 
-  if (Diags.hadAnyError())
+  if (Diags.hadAnyError() && !AllowErrors)
     return nullptr;
 
   if (DriverPrintActions) {
@@ -1076,7 +1079,7 @@ Driver::buildCompilation(const ToolChain &TC,
   if (!whyIgnoreIncrementallity.empty())
     C->disableIncrementalBuild(whyIgnoreIncrementallity);
 
-  if (Diags.hadAnyError())
+  if (Diags.hadAnyError() && !AllowErrors)
     return nullptr;
 
   if (DriverPrintBindings)

lib/Driver/FrontendUtil.cpp

@@ -101,7 +101,7 @@ bool swift::driver::getSingleFrontendInvocationFromDriverArguments(
     return true;
 
   std::unique_ptr<Compilation> C =
-      TheDriver.buildCompilation(*TC, std::move(ArgList));
+      TheDriver.buildCompilation(*TC, std::move(ArgList), /*AllowErrors=*/true);
   if (!C || C->getJobs().empty())
     return true; // Don't emit an error; one should already have been emitted
 
@@ -113,7 +113,6 @@ bool swift::driver::getSingleFrontendInvocationFromDriverArguments(
   if (CompileCommands.size() != 1) {
     // TODO: include Jobs in the diagnostic.
     Diags.diagnose(SourceLoc(), diag::error_expected_one_frontend_job);
-    return true;
   }
 
   const Job *Cmd = *CompileCommands.begin();

lib/IDE/Utils.cpp

@@ -294,30 +294,38 @@ bool ide::initCompilerInvocation(
   StreamDiagConsumer DiagConsumer(ErrOS);
   Diags.addConsumer(DiagConsumer);
 
-  bool HadError = driver::getSingleFrontendInvocationFromDriverArguments(
-      Args, Diags, [&](ArrayRef<const char *> FrontendArgs) {
-    return Invocation.parseArgs(FrontendArgs, Diags);
-  }, /*ForceNoOutputs=*/true);
+  bool InvocationCreationFailed =
+      driver::getSingleFrontendInvocationFromDriverArguments(
+          Args, Diags,
+          [&](ArrayRef<const char *> FrontendArgs) {
+            return Invocation.parseArgs(FrontendArgs, Diags);
+          },
+          /*ForceNoOutputs=*/true);
 
   // Remove the StreamDiagConsumer as it's no longer needed.
   Diags.removeConsumer(DiagConsumer);
 
-  if (HadError) {
-    Error = std::string(ErrOS.str());
+  Error = std::string(ErrOS.str());
+  if (InvocationCreationFailed) {
     return true;
   }
 
+  std::string SymlinkResolveError;
   Invocation.getFrontendOptions().InputsAndOutputs =
       resolveSymbolicLinksInInputs(
           Invocation.getFrontendOptions().InputsAndOutputs,
-          UnresolvedPrimaryFile, FileSystem, Error);
+          UnresolvedPrimaryFile, FileSystem, SymlinkResolveError);
 
   // SourceKit functionalities want to proceed even if there are missing inputs.
   Invocation.getFrontendOptions().InputsAndOutputs
       .setShouldRecoverMissingInputs();
 
-  if (!Error.empty())
+  if (!SymlinkResolveError.empty()) {
+    // resolveSymbolicLinksInInputs fails if the unresolved primary file is not
+    // in the input files. We can't recover from that.
+    Error += SymlinkResolveError;
     return true;
+  }
 
   ClangImporterOptions &ImporterOpts = Invocation.getClangImporterOptions();
   ImporterOpts.DetailedPreprocessingRecord = true;

test/SourceKit/CursorInfo/Inputs/invalid_compiler_args/A/File.swift

@@ -0,0 +1 @@
+class Foo {}

test/SourceKit/CursorInfo/Inputs/invalid_compiler_args/B/File.swift

@@ -0,0 +1 @@
+class Bar {}

test/SourceKit/CursorInfo/invalid_compiler_args.swift

@@ -0,0 +1,12 @@
+// We should not fail if two distinct file have the same name - this is only an issue in CodeGen
+// RUN: %sourcekitd-test -req=cursor -pos=1:7 %S/Inputs/invalid_compiler_args/A/File.swift -- %S/Inputs/invalid_compiler_args/A/File.swift %S/Inputs/invalid_compiler_args/B/File.swift | %FileCheck %s
+
+// We can't do anything if the requested file is not in the compiler arguments
+// RUN: not %sourcekitd-test -req=cursor -pos=1:7 %S/Inputs/invalid_compiler_args/A/File.swift --
+// RUN: not %sourcekitd-test -req=cursor -pos=1:7 %S/Inputs/invalid_compiler_args/A/File.swift -- %S/Inputs/invalid_compiler_args/B/File.swift
+
+// Specifying a file twice should just ignore one of them
+// RUN: %sourcekitd-test -req=cursor -pos=1:7 %S/Inputs/invalid_compiler_args/A/File.swift -- %S/Inputs/invalid_compiler_args/A/File.swift %S/Inputs/invalid_compiler_args/A/File.swift
+
+
+// CHECK: source.lang.swift.decl.class

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

@@ -1788,8 +1788,10 @@ void SwiftLangSupport::getCursorInfo(
   std::string Error;
   SwiftInvocationRef Invok =
       ASTMgr->getInvocation(Args, InputFile, fileSystem, Error);
+  if (!Error.empty()) {
+    LOG_WARN_FUNC("error creating ASTInvocation: " << Error);
+  }
   if (!Invok) {
-    LOG_WARN_FUNC("failed to create an ASTInvocation: " << Error);
     Receiver(RequestResult<CursorInfoData>::fromError(Error));
     return;
   }