Merge pull request #4083 from adrian-prantl/89836973-framework-paths

Partially revert 70d2ba0.

Author: adrian-prantl

lldb/include/lldb/Target/Target.h

@@ -169,6 +169,8 @@ class TargetProperties : public Properties {
 
   llvm::StringRef GetSwiftExtraClangFlags() const;
 
+  bool GetSwiftCreateModuleContextsInParallel() const;
+
   bool GetSwiftReadMetadataFromFileCache() const;
 
   bool GetSwiftUseReflectionSymbols() const;

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

@@ -2078,14 +2078,45 @@ ProcessModule(ModuleSP module_sp, std::string m_description,
   module_search_paths.insert(module_search_paths.end(),
                              opts.getImportSearchPaths().begin(),
                              opts.getImportSearchPaths().end());
-  for (const auto &fwsp : opts.getFrameworkSearchPaths())
-    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(GetLog(LLDBLog::Types), "adding Clang argument \"%s\".",
                        arg.c_str());
   }
+  // FIXME: Unfortunately this:
+  //
+  //   for (const auto &fwsp : opts.getFrameworkSearchPaths())
+  //    framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
+  //
+  // is insufficient, as ClangImporter can discover more framework
+  // search paths on the fly. Once a better solution is found,
+  // warmup_contexts can be retired (again).
+  {
+    SymbolFile *sym_file = module_sp->GetSymbolFile();
+    if (!sym_file)
+      return;
+    Status sym_file_error;
+    auto type_system_or_err =
+        sym_file->GetTypeSystemForLanguage(lldb::eLanguageTypeSwift);
+    if (!type_system_or_err) {
+      llvm::consumeError(type_system_or_err.takeError());
+      return;
+    }
+    auto ts = llvm::dyn_cast_or_null<TypeSystemSwift>(&*type_system_or_err);
+    if (!ts)
+      return;
+
+    SwiftASTContext *ast_context = ts->GetSwiftASTContext();
+    if (ast_context && !ast_context->HasErrors()) {
+      if (use_all_compiler_flags ||
+          target.GetExecutableModulePointer() == module_sp.get()) {
+        const auto &opts = ast_context->GetSearchPathOptions();
+        for (const auto &fwsp : opts.getFrameworkSearchPaths())
+          framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
+      }
+    }
+  }
 }
 
 lldb::TypeSystemSP SwiftASTContext::CreateInstance(
@@ -2145,7 +2176,26 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
     handled_sdk_path = true;
   }
 
+  auto warmup_astcontexts = [&]() {
+    if (target.GetSwiftCreateModuleContextsInParallel()) {
+      // The first call to GetTypeSystemForLanguage() on a module will
+      // trigger the import (and thus most likely the rebuild) of all
+      // the Clang modules that were imported in this module. This can
+      // be a lot of work (potentially ten seconds per module), but it
+      // can be performed in parallel.
+      llvm::ThreadPool pool(llvm::hardware_concurrency());
+      for (size_t mi = 0; mi != num_images; ++mi) {
+        auto module_sp = target.GetImages().GetModuleAtIndex(mi);
+        pool.async([=] {
+          GetModuleSwiftASTContext(*module_sp);
+        });
+      }
+      pool.wait();
+    }
+  };
+
   if (!handled_sdk_path) {
+    warmup_astcontexts();
     for (size_t mi = 0; mi != num_images; ++mi) {
       ModuleSP module_sp = target.GetImages().GetModuleAtIndex(mi);
       if (!HasSwiftModules(*module_sp))

lldb/source/Target/Target.cpp

@@ -4163,6 +4163,18 @@ void TargetProperties::SetInjectLocalVariables(ExecutionContext *exe_ctx,
                                             true);
 }
 
+bool TargetProperties::GetSwiftCreateModuleContextsInParallel() const {
+  const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
+      nullptr, false, ePropertyExperimental);
+  OptionValueProperties *exp_values =
+      exp_property->GetValue()->GetAsProperties();
+  if (exp_values)
+    return exp_values->GetPropertyAtIndexAsBoolean(
+        nullptr, ePropertySwiftCreateModuleContextsInParallel, true);
+  else
+    return true;
+}
+
 bool TargetProperties::GetSwiftReadMetadataFromFileCache() const {
   const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
       nullptr, false, ePropertyExperimental);

lldb/source/Target/TargetProperties.td

@@ -4,6 +4,9 @@ let Definition = "target_experimental" in {
   def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
     Global, DefaultTrue,
     Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
+  def SwiftCreateModuleContextsInParallel : Property<"swift-create-module-contexts-in-parallel", "Boolean">,
+    DefaultTrue,
+    Desc<"Create the per-module Swift AST contexts in parallel.">;
   def SwiftReadMetadataFromFileCache: Property<"swift-read-metadata-from-file-cache", "Boolean">,
     DefaultTrue,
     Desc<"Read Swift reflection metadata from the file cache instead of the process when possible">;

lldb/test/API/lang/swift/framework_paths/Direct.swift.in

@@ -0,0 +1,5 @@
+@_implementationOnly import Discovered
+public class D {
+    public init() { member = Invisible() }
+    private let member : Invisible
+}

lldb/test/API/lang/swift/framework_paths/Discovered.h

@@ -0,0 +1 @@
+struct Invisible {};

lldb/test/API/lang/swift/framework_paths/Discovered.m

@@ -0,0 +1 @@
+#include "Discovered.h"

lldb/test/API/lang/swift/framework_paths/Makefile

@@ -0,0 +1,29 @@
+SWIFT_SOURCES := main.swift
+
+SWIFTFLAGS_EXTRAS = -F $(BUILDDIR) -framework Direct -Xlinker -rpath -Xlinker $(BUILDDIR)
+
+all: Direct.framework $(EXE)
+
+include Makefile.rules
+
+Discovered.framework: Discovered.h
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_ONLY=YES \
+		DYLIB_NAME=Discovered \
+		DYLIB_OBJC_SOURCES=Discovered.m \
+		FRAMEWORK_HEADERS=$(SRCDIR)/Discovered.h \
+		FRAMEWORK_MODULES=$(SRCDIR)/module.modulemap \
+		FRAMEWORK=Discovered
+
+Direct.framework: $(SRCDIR)/Direct.swift.in Discovered.framework
+	mkdir -p $(BUILDDIR)/secret_path
+	cp $< $(BUILDDIR)/Direct.swift
+	mv Discovered.framework $(BUILDDIR)/secret_path
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_NAME=Direct \
+		DYLIB_SWIFT_SOURCES=Direct.swift \
+		DYLIB_MODULENAME=Direct \
+		FRAMEWORK=Direct \
+		SWIFTFLAGS_EXTRAS=-F$(BUILDDIR)/secret_path
+	rm -f $(BUILDDIR)/Direct.swiftmodule $(BUILDDIR)/Direct.swiftinterface $(BUILDDIR)/Direct.swift
+	ln -s $(BUILDDIR)/Direct.framework/Direct $(BUILDDIR)/Direct # FIXME

lldb/test/API/lang/swift/framework_paths/TestSwiftFrameworkPaths.py

@@ -0,0 +1,32 @@
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbtest as lldbtest
+import lldbsuite.test.lldbutil as lldbutil
+import os
+import unittest2
+
+
+class TestSwiftSystemFramework(lldbtest.TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+    mydir = lldbtest.TestBase.compute_mydir(__file__)
+
+    @swiftTest
+    @skipIf(oslist=no_match(["macosx"]))
+    def test_system_framework(self):
+        """Test the discovery of framework search paths from framework dependencies."""
+        self.build()
+        target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
+            self, 'break here', lldb.SBFileSpec('main.swift'))
+
+        log = self.getBuildArtifact("types.log")
+        self.runCmd('log enable lldb types -f "%s"' % log)
+        self.expect("expression -- 0")
+        pos = 0
+        import io
+        with open(log, "r", encoding='utf-8') as logfile:
+            for line in logfile:
+                if "SwiftASTContextForExpressions::LogConfiguration()" in line and \
+                   "/secret_path" in line:
+                    pos += 1
+        self.assertEqual(pos, 1, "framework search path discovery is broken")

lldb/test/API/lang/swift/framework_paths/main.swift

@@ -0,0 +1,4 @@
+import Direct
+let d = D()
+print("break here")
+print(d)

lldb/test/API/lang/swift/framework_paths/module.modulemap

@@ -0,0 +1 @@
+framework module Discovered { header "Discovered.h" }