Deserialize search path options in ProcessModule (NFCi)

That patch is one step towards removing the need for a separate
SwiftASTContext per lldb::Module. It moves the parsing of the headers
of modules found in Swift AST sections directly into ProcessModule()
since all that ProcessModule() needs is the search path options from
the serialized swift::CompilerInvocation in the module.

Right now this means that the compiler invocations are deserialized
twice, but this should be a cheap operation.

rdar://81717792

Author: adrian-prantl

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

@@ -1836,11 +1836,6 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language,
   // Add Swift interfaces in the .dSYM at the end of the search paths.
   // .swiftmodules win over .swiftinterfaces, when they are loaded
   // directly from the .swift_ast section.
-  //
-  // FIXME: Since these paths also end up in the scratch context, we
-  //        would need a mechanism to ensure that and newer versions
-  //        (in the library evolution sense, not the date on disk) win
-  //        over older versions of the same .swiftinterface.
   if (auto dsym = GetDSYMBundle(module)) {
     llvm::SmallString<256> path(*dsym);
     llvm::Triple triple(swift_ast_sp->GetTriple());
@@ -1949,7 +1944,7 @@ static lldb::ModuleSP GetUnitTestModule(lldb_private::ModuleList &modules) {
   return ModuleSP();
 }
 
-/// Scan a newly added lldb::Module fdor Swift modules and report any errors in
+/// Scan a newly added lldb::Module for Swift modules and report any errors in
 /// its module SwiftASTContext to Target.
 static void
 ProcessModule(ModuleSP module_sp, std::string m_description,
@@ -2068,20 +2063,60 @@ ProcessModule(ModuleSP module_sp, std::string m_description,
 
   if (ast_context->HasErrors())
     return;
-  if (use_all_compiler_flags ||
-      target.GetExecutableModulePointer() == module_sp.get()) {
-
-    const auto &opts = ast_context->GetSearchPathOptions();
-    module_search_paths.insert(module_search_paths.end(),
-                               opts.ImportSearchPaths.begin(),
-                               opts.ImportSearchPaths.end());
-    for (const auto &fwsp : opts.FrameworkSearchPaths)
-      framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
-    for (const std::string &arg : ast_context->GetClangArguments()) {
-      extra_clang_args.push_back(arg);
-      LOG_VERBOSE_PRINTF(LIBLLDB_LOG_TYPES, "adding Clang argument \"%s\".",
-                         arg.c_str());
-    }
+
+  // Load search path options from the module.
+  if (!use_all_compiler_flags &&
+      target.GetExecutableModulePointer() != module_sp.get())
+    return;
+
+  // Add Swift interfaces in the .dSYM at the end of the search paths.
+  // .swiftmodules win over .swiftinterfaces, when they are loaded
+  // directly from the .swift_ast section.
+  //
+  // FIXME: Since these paths end up in the scratch context, we would
+  //        need a mechanism to ensure that and newer versions (in the
+  //        library evolution sense, not the date on disk) win over
+  //        older versions of the same .swiftinterface.
+  if (auto dsym = GetDSYMBundle(*module_sp)) {
+    llvm::SmallString<256> path(*dsym);
+    llvm::Triple triple(ast_context->GetTriple());
+    StringRef arch = llvm::Triple::getArchTypeName(triple.getArch());
+    llvm::sys::path::append(path, "Contents", "Resources", "Swift", arch);
+    bool exists = false;
+    llvm::sys::fs::is_directory(path, exists);
+    if (exists)
+      module_search_paths.push_back(std::string(path));
+  }
+
+  // Create a one-off CompilerInvocation as a place to load the
+  // deserialized search path options into.
+  SymbolFile *sym_file = module_sp->GetSymbolFile();
+  if (!sym_file)
+    return;
+  bool found_swift_modules = false;
+  bool got_serialized_options = false;
+  llvm::SmallString<0> error;
+  llvm::raw_svector_ostream errs(error);
+  swift::CompilerInvocation invocation;
+  if (DeserializeAllCompilerFlags(invocation, *module_sp, m_description, errs,
+                                  got_serialized_options,
+                                  found_swift_modules)) {
+    // TODO: After removing DeserializeAllCompilerFlags from
+    //       CreateInstance(per-Module), errs will need to be
+    //       collected here and surfaced.
+  }
+
+  const auto &opts = invocation.getSearchPathOptions();
+  module_search_paths.insert(module_search_paths.end(),
+                             opts.ImportSearchPaths.begin(),
+                             opts.ImportSearchPaths.end());
+  for (const auto &fwsp : opts.FrameworkSearchPaths)
+    framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
+  auto &clang_opts = invocation.getClangImporterOptions().ExtraArgs;
+  for (const std::string &arg : clang_opts) {
+    extra_clang_args.push_back(arg);
+    LOG_VERBOSE_PRINTF(LIBLLDB_LOG_TYPES, "adding Clang argument \"%s\".",
+                       arg.c_str());
   }
 }
 

lldb/test/API/lang/swift/clangimporter/rewrite_clang_paths/TestSwiftRewriteClangPaths.py

@@ -97,15 +97,21 @@ def dotest(self, remap):
         found_rel = 0
         found_abs = 0
         found_ovl = 0
+        in_scratch_context = False
         logfile = open(log, "r")
         for line in logfile:
             self.assertFalse("remapped -iquote" in line)
-            if " remapped " in line:
-                if line[:-1].endswith('/user'): found_abs += 1;
-                continue
             if "error: " in line and "Foo" in line:
                 errs += 1
                 continue
+            if line.startswith(" SwiftASTContextForExpressions"):
+                in_scratch_context = True
+                if " remapped " in line:
+                    if line[:-1].endswith('/user'):
+                        found_abs += 1;
+                    continue
+            if not in_scratch_context:
+                continue
             if 'user/iquote-path'      in line: found_iquote += 1; continue
             if 'user/I-single'         in line: found_i1 += 1;     continue
             if 'user/I-double'         in line: found_i2 += 1;     continue
@@ -116,13 +122,13 @@ def dotest(self, remap):
 
         if remap:
             self.assertEqual(errs, 0, "expected no module import error")
-            # Module context + scratch context.
-            self.assertEqual(found_iquote, 2)
-            self.assertEqual(found_i1, 2)
-            self.assertEqual(found_i2, 2)
-            self.assertEqual(found_f, 2)
+            # Counting occurences in the scratch context.
+            self.assertEqual(found_iquote, 1)
+            self.assertEqual(found_i1, 1)
+            self.assertEqual(found_i2, 1)
+            self.assertEqual(found_f, 1)
             self.assertEqual(found_rel, 0)
             self.assertEqual(found_abs, 1)
-            self.assertEqual(found_ovl, 2)
+            self.assertEqual(found_ovl, 1)
         else:
-            self.assertTrue(errs > 0, "expected module import error")
+            self.assertGreater(errs, 0, "expected module import error")

lldb/test/Shell/Swift/astcontext_error.test

@@ -3,6 +3,7 @@
 # RUN: %target-swiftc -g %S/Inputs/ContextError.swift
 # RUN: %lldb ContextError -s %s | FileCheck %S/Inputs/ContextError.swift
 
+# Test that re-running a process doesn't emit bogus warnings.
 br set -p "here"
 run
 run