Merge pull request #37512 from ahoppen/pr/allow-driver-errors

[SourceKit] Recover if compiler arguments have errors

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/Frontend/Inputs/same_filename/A/File.swift

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

test/Frontend/Inputs/same_filename/B/File.swift

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

test/Frontend/same_filename_twice.swift

@@ -0,0 +1,13 @@
+// RUN: %empty-directory(%t)
+
+// A module should fail to be generated if the same filename is used twice and '-experimental-allow-module-with-compiler-errors' is not passed
+
+// RUN: not %target-swift-frontend -emit-module -o %t/no_allow_compiler_errors.swiftmodule %S/Inputs/same_filename/A/File.swift %S/Inputs/same_filename/B/File.swift
+// RUN: not ls %t/no_allow_compiler_errors.swiftmodule
+
+// If '-experimental-allow-module-with-compiler-errors' is passed, we should throw an error but still generate a module
+
+// RUN: %target-swift-frontend -emit-module -experimental-allow-module-with-compiler-errors -o %t/allow_compiler_errors.swiftmodule %S/Inputs/same_filename/A/File.swift %S/Inputs/same_filename/B/File.swift 2>&1 | %FileCheck %s
+// RUN: ls %t/allow_compiler_errors.swiftmodule
+
+// CHECK: filename "File.swift" used twice:

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;
   }