[lldb][progress] Improve Swift progress reporting (#7769)

LLDB's progress reporting infrastructure has had the ability to add
details to an overall progress report (added in
https://reviews.llvm.org/D143690) but this had yet to be implemented in
existing progress reports. This commit adds this functionality to
progress reports made for Swift operations (such as caching user
imports) so that instead of sending several individual progress reports
for each step in these operations, one progress report is created and
then incrementally updated.

Importing Swift modules takes place in the Swift compiler which uses a
callback function so that LLDB can perform its progress report. This
originally called into a function in `lldb_private::SwiftASTContext` but
will instead use a lambda function to increment a locally created
progress report.

Related to: swiftlang/swift#69730

rdar://105286354

Author: chelcassanova

lldb/source/Core/Progress.cpp

@@ -30,10 +30,9 @@ Progress::~Progress() {
   // Make sure to always report progress completed when this object is
   // destructed so it indicates the progress dialog/activity should go away.
   std::lock_guard<std::mutex> guard(m_mutex);
-  if (!m_completed) {
+  if (!m_completed)
     m_completed = m_total;
-    ReportProgress();
-  }
+  ReportProgress();
 }
 
 void Progress::Increment(uint64_t amount, std::string update) {

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

@@ -473,18 +473,15 @@ void SwiftLanguageRuntimeImpl::ProcessModulesToAdd() {
 
   auto &target = m_process.GetTarget();
   auto exe_module = target.GetExecutableModule();
-  Progress progress(
-      llvm::formatv("Setting up Swift reflection for '{0}'",
-                    exe_module->GetFileSpec().GetFilename().AsCString()),
-      modules_to_add_snapshot.GetSize());
-
+  Progress progress("Setting up Swift reflection");
   size_t completion = 0;
 
   // Add all defered modules to reflection context that were added to
   // the target since this SwiftLanguageRuntime was created.
   modules_to_add_snapshot.ForEach([&](const ModuleSP &module_sp) -> bool {
     AddModuleToReflectionContext(module_sp);
-    progress.Increment(++completion);
+    progress.Increment(++completion,
+                       module_sp->GetFileSpec().GetFilename().AsCString());
     return true;
   });
 }

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

@@ -119,6 +119,7 @@
 #include "Plugins/SymbolFile/DWARF/DWARFASTParserClang.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 
+#include <memory>
 #include <mutex>
 #include <queue>
 #include <set>
@@ -2008,6 +2009,25 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
   {
     LLDB_SCOPED_TIMERF("%s (getStdlibModule)", m_description.c_str());
     const bool can_create = true;
+
+    // Report progress on module importing by using a callback function in
+    // swift::ASTContext
+    Progress progress("Importing Swift standard library");
+    swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
+        [&progress](llvm::StringRef module_name, bool is_overlay) {
+          progress.Increment(1, (is_overlay ? module_name.str() + " (overlay)"
+                                            : module_name.str()));
+        });
+
+    // Clear the callback function on scope exit to prevent an out-of-scope
+    // access of the progress local variable
+    auto on_exit = llvm::make_scope_exit([&]() {
+      swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
+          [](llvm::StringRef module_name, bool is_overlay) {
+            Progress("Importing Swift modules");
+          });
+    });
+
     swift::ModuleDecl *stdlib =
         swift_ast_sp->m_ast_context_ap->getStdlibModule(can_create);
     if (!stdlib || IsDWARFImported(*stdlib)) {
@@ -2499,6 +2519,25 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
   {
     LLDB_SCOPED_TIMERF("%s (getStdlibModule)", m_description.c_str());
     const bool can_create = true;
+
+    // Report progress on module importing by using a callback function in
+    // swift::ASTContext
+    Progress progress("Importing Swift standard library");
+    swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
+        [&progress](llvm::StringRef module_name, bool is_overlay) {
+          progress.Increment(1, (is_overlay ? module_name.str() + " (overlay)"
+                                            : module_name.str()));
+        });
+
+    // Clear the callback function on scope exit to prevent an out-of-scope
+    // access of the progress local variable
+    auto on_exit = llvm::make_scope_exit([&]() {
+      swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
+          [](llvm::StringRef module_name, bool is_overlay) {
+            Progress("Importing Swift modules");
+          });
+    });
+
     swift::ModuleDecl *stdlib =
         swift_ast_sp->m_ast_context_ap->getStdlibModule(can_create);
     if (!stdlib || IsDWARFImported(*stdlib)) {
@@ -3203,7 +3242,7 @@ swift::ASTContext *SwiftASTContext::GetASTContext() {
       GetLanguageOptions(), GetTypeCheckerOptions(), GetSILOptions(),
       GetSearchPathOptions(), GetClangImporterOptions(),
       GetSymbolGraphOptions(), GetSourceManager(), GetDiagnosticEngine(),
-      /*OutputBackend=*/nullptr, ReportModuleLoadingProgress));
+      /*OutputBackend=*/nullptr));
 
   if (getenv("LLDB_SWIFT_DUMP_DIAGS")) {
     // NOTE: leaking a swift::PrintingDiagnosticConsumer() here, but
@@ -3486,15 +3525,6 @@ void SwiftASTContext::CacheModule(swift::ModuleDecl *module) {
   m_swift_module_cache.insert({ID, module});
 }
 
-bool SwiftASTContext::ReportModuleLoadingProgress(llvm::StringRef module_name,
-                                                  bool is_overlay) {
-  Progress progress(llvm::formatv(is_overlay ? "Importing overlay module {0}"
-                                             : "Importing module {0}",
-                                  module_name)
-                        .str());
-  return true;
-}
-
 swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,
                                               Status &error, bool *cached) {
   if (cached)
@@ -3545,6 +3575,24 @@ swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,
   // Create a diagnostic consumer for the diagnostics produced by the import.
   auto import_diags = getScopedDiagnosticConsumer();
 
+  // Report progress on module importing by using a callback function in
+  // swift::ASTContext
+  Progress progress("Importing Swift modules");
+  ast->SetPreModuleImportCallback([&progress](llvm::StringRef module_name,
+                                              bool is_overlay) {
+    progress.Increment(
+        1, (is_overlay ? module_name.str() + " (overlay)" : module_name.str()));
+  });
+
+  // Clear the callback function on scope exit to prevent an out-of-scope access
+  // of the progress local variable
+  auto on_exit = llvm::make_scope_exit([&]() {
+    ast->SetPreModuleImportCallback(
+        [](llvm::StringRef module_name, bool is_overlay) {
+          Progress("Importing Swift modules");
+        });
+  });
+
   // Perform the import.
   swift::ModuleDecl *module_decl = ast->getModuleByName(module_basename_sref);
 
@@ -4152,10 +4200,9 @@ void SwiftASTContext::ValidateSectionModules(
   Status error;
 
   Progress progress(
-      llvm::formatv("Loading Swift module {0}",
+      llvm::formatv("Loading Swift module '{0}' dependencies",
                     module.GetFileSpec().GetFilename().AsCString()),
       module_names.size());
-
   size_t completion = 0;
 
   for (const std::string &module_name : module_names) {
@@ -4164,7 +4211,7 @@ void SwiftASTContext::ValidateSectionModules(
 
     // We have to increment the completion value even if we can't get the module
     // object to stay in-sync with the total progress reporting.
-    progress.Increment(++completion);
+    progress.Increment(++completion, module_name);
     if (!GetModule(module_info, error))
       module.ReportWarning("unable to load swift module \"{0}\" ({1})",
                            module_name.c_str(), error.AsCString());
@@ -8596,10 +8643,7 @@ bool SwiftASTContextForExpressions::CacheUserImports(
 
   auto src_file_imports = source_file.getImports();
 
-  Progress progress(llvm::formatv("Caching Swift user imports from '{0}'",
-                                  source_file.getFilename().data()),
-                    src_file_imports.size());
-
+  Progress progress("Importing modules used in expression");
   size_t completion = 0;
 
   /// Find all explicit imports in the expression.
@@ -8617,7 +8661,9 @@ bool SwiftASTContextForExpressions::CacheUserImports(
   source_file.walk(import_finder);
 
   for (const auto &attributed_import : src_file_imports) {
-    progress.Increment(++completion);
+    progress.Increment(
+        ++completion,
+        attributed_import.module.importedModule->getModuleFilename().str());
     swift::ModuleDecl *module = attributed_import.module.importedModule;
     if (module && import_finder.imports.count(module)) {
       std::string module_name;
@@ -8737,10 +8783,9 @@ bool SwiftASTContext::GetCompileUnitImportsImpl(
       llvm::formatv("Getting Swift compile unit imports for '{0}'",
                     compile_unit->GetPrimaryFile().GetFilename()),
       cu_imports.size());
-
   size_t completion = 0;
   for (const SourceModule &module : cu_imports) {
-    progress.Increment(++completion);
+    progress.Increment(++completion, module.path.back().GetStringRef().str());
     // When building the Swift stdlib with debug info these will
     // show up in "Swift.o", but we already imported them and
     // manually importing them will fail.

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

@@ -298,9 +298,6 @@ class SwiftASTContext : public TypeSystemSwift {
   swift::ModuleDecl *CreateModule(const SourceModule &module, Status &error,
                                   swift::ImplicitImportInfo importInfo);
 
-  static bool ReportModuleLoadingProgress(llvm::StringRef module_name,
-                                          bool is_overlay);
-
   // This function should only be called when all search paths
   // for all items in a swift::ASTContext have been setup to
   // allow for imports to happen correctly. Use with caution,

lldb/test/API/functionalities/progress_reporting/swift_progress_reporting/TestSwiftProgressReporting.py

@@ -37,11 +37,14 @@ def test_swift_progress_report(self):
         self.runCmd("expr boo")
         self.runCmd("v s")
 
-        beacons = [ "Loading Swift module",
-                    "Caching Swift user imports from",
-                    "Setting up Swift reflection for",
-                    "Getting Swift compile unit imports for",
-                    "Importing module", "Importing overlay module"]
+        beacons = [
+            "Loading Swift module",
+            "Importing modules used in expression",
+            "Setting up Swift reflection",
+            "Getting Swift compile unit imports for",
+            "Importing Swift modules",
+            "Importing Swift standard library",
+        ]
 
         while len(beacons):
             event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)