[lldb] Remove the Swift-specific diagnosis frame callback

This feature was originally added to communicate Swift module import
failures in per-module SwiftASTContext instances powering "frame
variable" to the user. We no longer rely on per-module
SwiftASTContexts so it's no longer needed to store these diagnostics
long-term to display them to all debuggers using the same
lldb::Module. Instead this patch diagnoses them immediately using
Debugger::ReportWarning/Error, and since they can only happen when
setting up an expression evaluator, they will also be displayed in a
timely manner.

rdar://150066908

Author: adrian-prantl

lldb/include/lldb/Symbol/TypeSystem.h

@@ -578,13 +578,6 @@ class TypeSystem : public PluginInterface,
   // meaningless type itself, instead preferring to use the dynamic type
   virtual bool IsMeaninglessWithoutDynamicResolution(void *type);
 
-  /// A TypeSystem may belong to more than one debugger, so it doesn't
-  /// have a way to communicate errors. This method can be called by a
-  /// process to tell the TypeSystem to send any diagnostics to the
-  /// process so they can be surfaced to the user.
-  virtual void DiagnoseWarnings(Process &process,
-                                const SymbolContext &sc) const;
-
   virtual std::optional<llvm::json::Value> ReportStatistics();
 
   bool GetHasForcefullyCompletedTypes() const {

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

@@ -1197,18 +1197,6 @@ static void printASTValidationError(
     LLDB_LOG(log, "  -- {0}", ExtraOpt);
 }
 
-void SwiftASTContext::DiagnoseWarnings(Process &process,
-                                       const SymbolContext &sc) const {
-  if (!sc.module_sp || !HasDiagnostics())
-    return;
-  auto debugger_id = process.GetTarget().GetDebugger().GetID();
-  std::string msg;
-  llvm::raw_string_ostream(msg) << "Cannot load Swift type information for "
-                                << sc.module_sp->GetFileSpec().GetPath();
-  Debugger::ReportWarning(msg, debugger_id, &m_swift_import_warning);
-  StreamAllDiagnostics(debugger_id);
-}
-
 /// Locate the swift-plugin-server for a plugin library,
 /// by converting  ${toolchain}/usr/(local)?/lib/swift/host/plugins
 /// into           ${toolchain}/usr/bin/swift-plugin-server
@@ -3236,35 +3224,6 @@ Status SwiftASTContext::GetAllDiagnostics() const {
   return error;
 }
 
-void SwiftASTContext::StreamAllDiagnostics(
-    std::optional<lldb::user_id_t> debugger_id) const {
-  Status error = m_fatal_errors.Clone();
-  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 eSeverityError:
-        Debugger::ReportError(msg, debugger_id, &m_swift_diags_streamed);
-        break;
-      case eSeverityWarning:
-      case eSeverityInfo:
-        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) {
@@ -5376,8 +5335,22 @@ bool SwiftASTContext::HasClangImporterErrors() const {
 
 void SwiftASTContext::AddDiagnostic(lldb::Severity severity,
                                     llvm::StringRef message) {
-  assert(m_diagnostic_consumer_ap);
   HEALTH_LOG_PRINTF("%s", message.str().c_str());
+
+  if (auto target_sp = GetTargetWP().lock()) {
+    auto debugger_id = target_sp->GetDebugger().GetID();
+    switch (severity) {
+    case eSeverityError:
+      Debugger::ReportError(message.str(), debugger_id);
+      break;
+    case eSeverityWarning:
+    case eSeverityInfo:
+      Debugger::ReportWarning(message.str(), debugger_id);
+      break;
+    }
+  }
+
+  assert(m_diagnostic_consumer_ap);
   if (!m_diagnostic_consumer_ap.get())
     return;
 

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

@@ -482,15 +482,12 @@ class SwiftASTContext : public TypeSystemSwift {
   void RaiseFatalError(std::string msg) const { m_fatal_errors = Status(msg); }
   static bool HasFatalErrors(swift::ASTContext *ast_context);
   bool HasFatalErrors() const {
-    return m_fatal_errors.Fail() || HasFatalErrors(m_ast_context_ap.get());
+    return m_logged_fatal_error || m_fatal_errors.Fail() ||
+           HasFatalErrors(m_ast_context_ap.get());
   }
 
   /// Return only fatal errors.
   Status GetFatalErrors() const;
-  /// Notify the Process about any Swift or ClangImporter errors.
-  void DiagnoseWarnings(Process &process,
-                        const SymbolContext &sc) const override;
-
   void PrintDiagnostics(DiagnosticManager &diagnostic_manager,
                         uint32_t bufferID = UINT32_MAX, uint32_t first_line = 0,
                         uint32_t last_line = UINT32_MAX) const;
@@ -884,8 +881,6 @@ 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(std::optional<lldb::user_id_t> debugger_id) const;
 
   llvm::TargetOptions *getTargetOptions();
 
@@ -945,7 +940,6 @@ class SwiftASTContext : public TypeSystemSwift {
   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;

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

@@ -2378,13 +2378,6 @@ Status TypeSystemSwiftTypeRef::IsCompatible() {
   return {};
 }
 
-void TypeSystemSwiftTypeRef::DiagnoseWarnings(Process &process,
-                                              const SymbolContext &sc) const {
-  // This gets called only from Thread::FrameSelectedCallback(StackFrame).
-  if (auto swift_ast_context = GetSwiftASTContextOrNull(sc))
-    swift_ast_context->DiagnoseWarnings(process, sc);
-}
-
 plugin::dwarf::DWARFASTParser *TypeSystemSwiftTypeRef::GetDWARFParser() {
   if (!m_dwarf_ast_parser_up)
     m_dwarf_ast_parser_up.reset(new DWARFASTParserSwift(*this));

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

@@ -116,8 +116,6 @@ class TypeSystemSwiftTypeRef : public TypeSystemSwift {
   bool SupportsLanguage(lldb::LanguageType language) override;
   Status IsCompatible() override;
 
-  void DiagnoseWarnings(Process &process,
-                        const SymbolContext &sc) const override;
   plugin::dwarf::DWARFASTParser *GetDWARFParser() override;
   // CompilerDecl functions
   ConstString DeclGetName(void *opaque_decl) override {

lldb/source/Symbol/TypeSystem.cpp

@@ -183,9 +183,6 @@ bool TypeSystem::IsMeaninglessWithoutDynamicResolution(void *type) {
   return false;
 }
 
-void TypeSystem::DiagnoseWarnings(Process &process,
-                                  const SymbolContext &sc) const {}
-
 Status TypeSystem::IsCompatible() {
   // Assume a language is compatible. Override this virtual function
   // in your TypeSystem plug-in if version checking is desired.

lldb/source/Target/Thread.cpp

@@ -53,10 +53,6 @@
 #include "lldb/ValueObject/ValueObjectConstResult.h"
 #include "lldb/lldb-enumerations.h"
 
-#ifdef LLDB_ENABLE_SWIFT
-#include "Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.h"
-#endif
-
 #include <memory>
 #include <optional>
 
@@ -348,19 +344,6 @@ void Thread::FrameSelectedCallback(StackFrame *frame) {
     GetProcess()->PrintWarningToolchainMismatch(sc);
 #endif
   }
-#ifdef LLDB_ENABLE_SWIFT
-  {
-    SymbolContext msc =
-        frame->GetSymbolContext(eSymbolContextFunction | eSymbolContextModule);
-    Status error;
-    ExecutionContext exe_ctx;
-    frame->CalculateExecutionContext(exe_ctx);
-    if (auto target = frame->CalculateTarget())
-      if (auto swift_ast_ctx =
-              TypeSystemSwiftTypeRefForExpressions::GetForTarget(*target))
-        swift_ast_ctx->DiagnoseWarnings(*GetProcess(), msc);
-  }
-#endif
 }
 
 lldb::StopInfoSP Thread::GetStopInfo() {

lldb/test/Shell/Swift/DeserializationFailure.test

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

lldb/test/Shell/Swift/MissingVFSOverlay.test

@@ -15,5 +15,4 @@ run
 expression 1
 
 # The {{ }} avoids accidentally matching the input script!
-# CHECK: warning:{{ }}{{.*}}Swift{{.*}}a.out
 # CHECK: warning:{{ }}{{.*}}Ignoring missing VFS{{.*}}overlay.yaml