[lldb] Use one Progress event per root module build

[kastiglione]

Summary

Following the work done by @JDevlieghere in D143690, this changes how Clang module build
events are emitted.

Instead of one Progress event per module being built, a single Progress event is used to
encompass all modules, and each module build is sent as an Increment update.

[JDevlieghere]

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

228–233

Progress::Increment is taking a std::string by value, so if you think this will ever get called with an r-value reference, you should do the same and std::move it. Otherwise the StringRef or the const std::string& both require a copy.

[kastiglione]

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

228–233

Understood. The module names are references, I could change this function to take a string &&, but that would move the copy to the callee rather than here. Either way, I don't see a way to avoid a copy.

[JDevlieghere]

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

228–233

Yeah if you don't have an r-value reference it cannot be avoided, but I think it's still best to make this take it by value so in case there ever is one in the caller to this function, it is avoided.

[kastiglione]

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

228–233

Take it by value, or by r-value?

[JDevlieghere]

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

227–234

By value, like this ^

[kastiglione]

Use std::string argument

[JDevlieghere]

LGTM

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

public:
   void DumpDiagnostics(Stream &error_stream);
 
   void BeginSourceFile(const clang::LangOptions &LangOpts,
                        const clang::Preprocessor *PP = nullptr) override;
   void EndSourceFile() override;
 
 private:
   bool HandleModuleRemark(const clang::Diagnostic &info);
-  void SetCurrentModuleProgress(llvm::StringRef module_name);
+  void SetCurrentModuleProgress(std::string module_name);
 
   typedef std::pair<clang::DiagnosticsEngine::Level, std::string>
       IDAndDiagnostic;
   std::vector<IDAndDiagnostic> m_diagnostics;
   /// The DiagnosticPrinter used for creating the full diagnostic messages
   /// that are stored in m_diagnostics.
   std::shared_ptr<clang::TextDiagnosticPrinter> m_diag_printer;
   /// Output stream of m_diag_printer.
case clang::diag::remark_module_build: {
     return true;
   }
   case clang::diag::remark_module_build_done: {
     // The current module is done.
     m_module_build_stack.pop_back();
     if (m_module_build_stack.empty()) {
       m_current_progress_up = nullptr;
     } else {
-      // Update the progress to re-show the module that was currently being
-      // built from the time the now completed module was originally began.
+      // When the just completed module began building, a module that depends on
+      // it ("module A") was effectively paused. Update the progress to re-show
+      // "module A" as continuing to be built.
       const auto &resumed_module_name = m_module_build_stack.back();
       SetCurrentModuleProgress(resumed_module_name);
     }
 
     const auto &module_name = info.getArgStdStr(0);
     LLDB_LOG(log, "Finished building Clang module {0}", module_name);
     return true;
   }
   default:
     return false;
   }
 }
 
 void StoringDiagnosticConsumer::SetCurrentModuleProgress(
-    llvm::StringRef module_name) {
-  // Ensure the ordering of:
-  //   1. Completing the existing progress event.
-  //   2. Beginining a new progress event.
-  m_current_progress_up = nullptr;
-  m_current_progress_up = std::make_unique<Progress>(
-      llvm::formatv("Currently building module {0}", module_name));
+    std::string module_name) {
+  if (!m_current_progress_up)
+    m_current_progress_up =
+        std::make_unique<Progress>("Building Clang modules");
+
+  m_current_progress_up->Increment(1, std::move(module_name));
 }
 
 ClangModulesDeclVendor::ClangModulesDeclVendor()
     : ClangDeclVendor(eClangModuleDeclVendor) {}
 
 ClangModulesDeclVendor::~ClangModulesDeclVendor() = default;
 
 ClangModulesDeclVendorImpl::ClangModulesDeclVendorImpl(
     llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine> diagnostics_engine,

lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py

def test_clang_module_build_progress_report(self):
         )
 
         # Trigger module builds.
         self.expect("expression @import MyModule")
 
         event = lldbutil.fetch_next_event(self, listener, broadcaster)
         payload = lldb.SBDebugger.GetProgressFromEvent(event)
         message = payload[0]
-        self.assertEqual(message, "Currently building module MyModule")
+        self.assertEqual(message, "Building Clang modules")