Merge pull request #7106 from adrian-prantl/112939483-5.9

Degrade missing VFS overlays in serialized Swift options to warnings.

Author: adrian-prantl

lldb/include/lldb/Core/Module.h

@@ -844,9 +844,6 @@ class Module : public std::enable_shared_from_this<Module>,
 
 #ifdef LLDB_ENABLE_SWIFT
   void
-  ReportWarningCantLoadSwiftModule(std::string details,
-                                   llvm::Optional<lldb::user_id_t> debugger_id);
-  void
   ReportWarningToolchainMismatch(CompileUnit &comp_unit,
                                  llvm::Optional<lldb::user_id_t> debugger_id);
 
@@ -1110,7 +1107,6 @@ class Module : public std::enable_shared_from_this<Module>,
   std::once_flag m_optimization_warning;
   std::once_flag m_language_warning;
 #ifdef LLDB_ENABLE_SWIFT
-  std::once_flag m_swift_import_warning;
   std::once_flag m_toolchain_mismatch_warning;
 #endif
 

lldb/include/lldb/Target/Process.h

@@ -1350,14 +1350,6 @@ class Process : public std::enable_shared_from_this<Process>,
   void PrintWarningOptimization(const SymbolContext &sc);
 
 #ifdef LLDB_ENABLE_SWIFT
-  /// Prints a async warning message to the user one time per Process
-  /// for a Module whose Swift AST sections couldn't be loaded because
-  /// they aren't buildable on the current machine.
-  ///
-  /// @param [in] module
-  ///     The affected Module.
-  void PrintWarningCantLoadSwiftModule(Module &module, std::string details);
-
   /// Print a user-visible warning about Swift CUs compiled with a
   /// different Swift compiler than the one embedded in LLDB.
   void PrintWarningToolchainMismatch(const SymbolContext &sc);

lldb/source/Core/Module.cpp

@@ -1179,15 +1179,6 @@ void Module::ReportWarningUnsupportedLanguage(
 }
 
 #ifdef LLDB_ENABLE_SWIFT
-void Module::ReportWarningCantLoadSwiftModule(
-    std::string details, llvm::Optional<lldb::user_id_t> debugger_id) {
-  StreamString ss;
-  ss << GetFileSpec() << ": "
-     << "Cannot load Swift type information: " << details;
-  Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
-                          &m_swift_import_warning);
-}
-
 static llvm::VersionTuple GetAdjustedVersion(llvm::VersionTuple version) {
   return version;
 }

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp

@@ -1118,9 +1118,14 @@ static void printASTValidationError(
 }
 
 void SwiftASTContext::DiagnoseWarnings(Process &process, Module &module) const {
-  if (HasDiagnostics())
-    process.PrintWarningCantLoadSwiftModule(module,
-                                            GetAllDiagnostics().AsCString());
+  if (!HasDiagnostics())
+    return;
+  auto debugger_id = process.GetTarget().GetDebugger().GetID();
+  std::string msg;
+  llvm::raw_string_ostream(msg) << "Cannot load Swift type information for "
+                                << module.GetFileSpec().GetPath();
+  Debugger::ReportWarning(msg, debugger_id, &m_swift_import_warning);
+  StreamAllDiagnostics(debugger_id);
 }
 
 /// Locate the swift-plugin-server for a plugin library,
@@ -1503,8 +1508,8 @@ bool ShouldUnique(StringRef arg) {
 } // namespace
 
 // static
-void SwiftASTContext::AddExtraClangArgs(const std::vector<std::string>& source,
-                                        std::vector<std::string>& dest) {
+void SwiftASTContext::AddExtraClangArgs(const std::vector<std::string> &source,
+                                        std::vector<std::string> &dest) {
   llvm::StringSet<> unique_flags;
   for (auto &arg : dest)
     unique_flags.insert(arg);
@@ -1648,6 +1653,41 @@ void SwiftASTContext::RemapClangImporterOptions(
   }
 }
 
+void SwiftASTContext::FilterClangImporterOptions(
+    std::vector<std::string> &extra_args, SwiftASTContext *ctx) {
+  std::string ivfs_arg;
+  // Copy back a filtered version of ExtraArgs.
+  std::vector<std::string> orig_args(std::move(extra_args));
+  for (auto &arg : orig_args) {
+    // The VFS options turn into fatal errors when the referenced file
+    // is not found. Since the Xcode build system tends to create a
+    // lot of VFS overlays by default, stat them and emit a warning if
+    // the yaml file couldn't be found.
+    if (StringRef(arg).startswith("-ivfs")) {
+      // Stash the argument.
+      ivfs_arg = arg;
+      continue;
+    }
+    if (!ivfs_arg.empty()) {
+      auto clear_ivfs_arg = llvm::make_scope_exit([&] { ivfs_arg.clear(); });
+      if (!FileSystem::Instance().Exists(arg)) {
+        if (ctx) {
+          std::string error;
+          llvm::raw_string_ostream(error)
+              << "Ignoring missing VFS file: " << arg
+              << "\nThis is the likely root cause for any subsequent compiler "
+                 "errors.";
+          ctx->AddDiagnostic(eDiagnosticSeverityWarning, error);
+        }
+        continue;
+      }
+      // Keep it.
+      extra_args.push_back(ivfs_arg);
+    }
+    extra_args.push_back(std::move(arg));
+  }
+}
+
 /// Retrieve the .dSYM bundle for \p module.
 static llvm::Optional<StringRef> GetDSYMBundle(Module &module) {
   auto sym_file = module.GetSymbolFile();
@@ -1904,6 +1944,8 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
 
   // Apply source path remappings found in the module's dSYM.
   swift_ast_sp->RemapClangImporterOptions(module.GetSourceMappingList());
+  swift_ast_sp->FilterClangImporterOptions(
+      swift_ast_sp->GetClangImporterOptions().ExtraArgs, swift_ast_sp.get());
 
   // Add Swift interfaces in the .dSYM at the end of the search paths.
   // .swiftmodules win over .swiftinterfaces, when they are loaded
@@ -2396,6 +2438,8 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
 
   // Apply source path remappings found in the target settings.
   swift_ast_sp->RemapClangImporterOptions(target.GetSourcePathMap());
+  swift_ast_sp->FilterClangImporterOptions(
+      swift_ast_sp->GetClangImporterOptions().ExtraArgs, swift_ast_sp.get());
 
   // This needs to happen once all the import paths are set, or
   // otherwise no modules will be found.
@@ -2483,6 +2527,35 @@ Status SwiftASTContext::GetAllDiagnostics() const {
   return error;
 }
 
+void SwiftASTContext::StreamAllDiagnostics(
+    llvm::Optional<lldb::user_id_t> debugger_id) const {
+  Status error = m_fatal_errors;
+  if (!error.Success()) {
+    Debugger::ReportWarning(error.AsCString(), debugger_id,
+                            &m_swift_diags_streamed);
+    return;
+  }
+
+  // Retrieve the error message from the DiagnosticConsumer.
+  DiagnosticManager diagnostic_manager;
+  PrintDiagnostics(diagnostic_manager);
+  for (auto &diag : diagnostic_manager.Diagnostics())
+    if (diag) {
+      std::string msg = diag->GetMessage().str();
+      switch (diag->GetSeverity()) {
+      case eDiagnosticSeverityError:
+        Debugger::ReportError(msg, debugger_id, &m_swift_diags_streamed);
+        break;
+      case eDiagnosticSeverityWarning:
+      case eDiagnosticSeverityRemark:
+        Debugger::ReportWarning(msg, debugger_id, &m_swift_warning_streamed);
+        break;
+      }
+    }
+  static_cast<StoringDiagnosticConsumer *>(m_diagnostic_consumer_ap.get())
+      ->Clear();
+}
+
 void SwiftASTContext::LogFatalErrors() const {
   // Avoid spamming the health log with redundant copies of the fatal error.
   if (m_logged_fatal_error) {

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h

@@ -262,6 +262,9 @@ class SwiftASTContext : public TypeSystemSwift {
   static void AddExtraClangArgs(const std::vector<std::string>& source,
                                 std::vector<std::string>& dest);
   static std::string GetPluginServer(llvm::StringRef plugin_library_path);
+  /// Removes nonexisting VFS overlay options.
+  static void FilterClangImporterOptions(std::vector<std::string> &extra_args,
+                                         SwiftASTContext *ctx = nullptr);
 
   /// Add the target's swift-extra-clang-flags to the ClangImporter options.
   void AddUserClangArgs(TargetProperties &props);
@@ -859,6 +862,8 @@ class SwiftASTContext : public TypeSystemSwift {
   /// Called by the VALID_OR_RETURN macro to log all errors.
   void LogFatalErrors() const;
   Status GetAllDiagnostics() const;
+  /// Stream all diagnostics to the Debugger and clear them.
+  void StreamAllDiagnostics(llvm::Optional<lldb::user_id_t> debugger_id) const;
 
   llvm::TargetOptions *getTargetOptions();
 
@@ -903,6 +908,9 @@ class SwiftASTContext : public TypeSystemSwift {
   std::unique_ptr<swift::irgen::IRGenerator> m_ir_generator_ap;
   std::unique_ptr<swift::irgen::IRGenModule> m_ir_gen_module_ap;
   llvm::once_flag m_ir_gen_module_once;
+  mutable std::once_flag m_swift_import_warning;
+  mutable std::once_flag m_swift_diags_streamed;
+  mutable std::once_flag m_swift_warning_streamed;
   std::unique_ptr<swift::DiagnosticConsumer> m_diagnostic_consumer_ap;
   std::unique_ptr<swift::DependencyTracker> m_dependency_tracker;
   swift::ModuleDecl *m_scratch_module = nullptr;

lldb/source/Target/Process.cpp

@@ -6010,12 +6010,6 @@ void Process::PrintWarningOptimization(const SymbolContext &sc) {
 }
 
 #ifdef LLDB_ENABLE_SWIFT
-void Process::PrintWarningCantLoadSwiftModule(Module &module,
-                                              std::string details) {
-  module.ReportWarningCantLoadSwiftModule(std::move(details),
-                                          GetTarget().GetDebugger().GetID());
-}
-
 void Process::PrintWarningToolchainMismatch(const SymbolContext &sc) {
   if (GetTarget().GetProcessLaunchInfo().IsScriptedProcess())
     // It's very likely that the debugger used to launch the ScriptedProcess

lldb/test/API/lang/swift/clangimporter/missing_vfsoverlay/TestSwiftMissingVFSOverlay.py

@@ -4,7 +4,6 @@
 import lldbsuite.test.lldbutil as lldbutil
 import unittest2
 
-
 class TestSwiftMissingVFSOverlay(TestBase):
     mydir = TestBase.compute_mydir(__file__)
 
@@ -18,11 +17,11 @@ def setUp(self):
     @skipUnlessDarwin
     @swiftTest
     def test(self):
-        """Test that a broken Clang command line option is diagnosed
-        in the expression evaluator"""
+        """This used to be a test for a diagnostic, however,
+        this is no longer an unrecoverable error"""
         self.build()
         lldbutil.run_to_source_breakpoint(
             self, "break here", lldb.SBFileSpec("main.swift"),
             extra_images=["Foo"]
         )
-        self.expect("expr y", error=True, substrs=["IRGen", "overlay.yaml"])
+        self.expect("expr y", substrs=["42"])

lldb/test/Shell/Swift/DeserializationFailure.test

@@ -18,4 +18,5 @@ run
 expression 1
 
 # The {{ }} avoids accidentally matching the input script!
-# CHECK: a.out:{{ }}{{.*}}The serialized module is corrupted.
+# CHECK: {{ }}a.out
+# CHECK: {{ }}The serialized module is corrupted.

lldb/test/Shell/Swift/MissingVFSOverlay.test

@@ -15,7 +15,8 @@ run
 expression 1
 
 # The {{ }} avoids accidentally matching the input script!
-# CHECK: a.out:{{ }}
-# CHECK-SAME: error:
+# CHECK: warning:
+# CHECK-SAME: a.out
+# CHECK: warning:
+# CHECK-SAME: Ignoring missing VFS
 # CHECK-SAME: overlay.yaml
-# CHECK-SAME: not found

lldb/unittests/Symbol/TestSwiftASTContext.cpp

@@ -12,9 +12,10 @@
 
 #include "gtest/gtest.h"
 
+#include "Plugins/TypeSystem/Swift/SwiftASTContext.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
-#include "Plugins/TypeSystem/Swift/SwiftASTContext.h"
+#include "llvm/Support/FileUtilities.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -255,3 +256,40 @@ TEST(ClangArgs, DoubleDash) {
   // Check that all ignored arguments got removed.
   EXPECT_EQ(dest, std::vector<std::string>({"-v"}));
 }
+
+TEST_F(TestSwiftASTContext, IVFS) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+  llvm::SmallString<128> name;
+  auto ec = llvm::sys::fs::createTemporaryFile(
+      llvm::Twine(Info->test_case_name()) + "-" + Info->name(), "overlay.yaml",
+      name);
+  ASSERT_FALSE((bool)ec);
+  llvm::FileRemover remover(name);
+
+  std::string valid = name.str().str();
+  std::string invalid = name.str().drop_back(1).str() +"XXX";
+  std::vector<std::string> args;
+  args.push_back("-ivfsoverlay");
+  args.push_back(valid);
+
+  args.push_back("-ivfsoverlay");
+  args.push_back(invalid);
+
+  args.push_back("-ivfsstatcache");
+  args.push_back(valid);
+
+  args.push_back("-ivfsstatcache");
+  args.push_back(invalid);
+
+  std::vector<std::string> expected;
+  expected.push_back("-ivfsoverlay");
+  expected.push_back(valid);
+
+  expected.push_back("-ivfsstatcache");
+  expected.push_back(valid);
+
+  SwiftASTContext::FilterClangImporterOptions(args);
+
+  // Check that all ignored arguments got removed.
+  EXPECT_EQ(args, expected);
+}