Unify all mechanisms for delaying diagnostics in SwiftASTContext (NFC)

Author: adrian-prantl

lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp

@@ -1974,31 +1974,18 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
   // If IRGen failed without errors, the root cause may be a fatal
   // Clang diagnostic.
   if (m_swift_ast_ctx.HasErrors() || m_swift_ast_ctx.HasClangImporterErrors()) {
-    diagnostic_manager.Printf(eDiagnosticSeverityRemark,
-                              "couldn't IRGen expression.");
+    diagnostic_manager.PutString(eDiagnosticSeverityRemark,
+                                 "couldn't IRGen expression.");
     DiagnoseSwiftASTContextError();
     return ParseResult::unrecoverable_error;
   }
 
   if (!m_module) {
-    auto &warnings = m_swift_ast_ctx.GetModuleImportWarnings();
-    for (StringRef message : warnings) {
-      // FIXME: Don't store diagnostics as strings.
-      auto severity = eDiagnosticSeverityWarning;
-      if (message.consume_front("warning: "))
-        severity = eDiagnosticSeverityWarning;
-      if (message.consume_front("error: "))
-        severity = eDiagnosticSeverityError;
-      diagnostic_manager.PutString(severity, message);
-    }
-    std::string error = "couldn't IRGen expression";
     diagnostic_manager.Printf(
-        eDiagnosticSeverityError, "couldn't IRGen expression: %s",
-        warnings.empty()
-            ? "Please enable the expression log by running \"log enable lldb "
-              "expr\", then run the failing expression again, and file a "
-              "bugreport with the log output."
-            : "Please check the above error messages for possible root causes.");
+        eDiagnosticSeverityError,
+        "couldn't IRGen expression. Please enable the expression log by "
+        "running \"log enable lldb expr\", then run the failing expression "
+        "again, and file a bug report with the log output.");
     return ParseResult::unrecoverable_error;
   }
 

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

@@ -1052,8 +1052,8 @@ static void printASTValidationError(
 }
 
 void SwiftASTContext::DiagnoseWarnings(Process &process, Module &module) const {
-  for (const std::string &message : m_module_import_warnings)
-    process.PrintWarningCantLoadSwiftModule(module, message);
+  if (HasErrors() || HasClangImporterErrors())
+    process.PrintWarningCantLoadSwiftModule(module, GetAllErrors().AsCString());
 }
 
 /// Locate the swift-plugin-server for a plugin library,
@@ -1690,7 +1690,7 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
                                     m_description, errs, got_serialized_options,
                                     found_swift_modules)) {
       // Validation errors are not fatal for the context.
-      swift_ast_sp->m_module_import_warnings.push_back(std::string(error));
+      swift_ast_sp->AddDiagnostic(eDiagnosticSeverityWarning, errs.str());
     }
 
     llvm::StringRef serialized_triple =
@@ -2973,7 +2973,6 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer {
     return old;
   }
 
-  /// This is only used by ReconstructTypes.
   void AddDiagnostic(std::unique_ptr<Diagnostic> diagnostic) {
     if (diagnostic)
       m_diagnostics.push_back(std::move(diagnostic));
@@ -3062,19 +3061,16 @@ swift::ASTContext *SwiftASTContext::GetASTContext() {
 
       // Handle any errors.
       if (!clang_importer_ap || HasErrors()) {
-        std::string message;
-        if (!HasErrors()) {
-          message = "failed to create ClangImporter.";
-          m_module_import_warnings.push_back(message);
-        } else {
+        AddDiagnostic(eDiagnosticSeverityWarning,
+                        "failed to create ClangImporter");
+        if (GetLog(LLDBLog::Types)) {
           DiagnosticManager diagnostic_manager;
           PrintDiagnostics(diagnostic_manager, true);
           std::string underlying_error = diagnostic_manager.GetString();
-          message = "failed to initialize ClangImporter: ";
-          message += underlying_error;
-          m_module_import_warnings.push_back(underlying_error);
+          LOG_PRINTF(GetLog(LLDBLog::Types),
+                     "failed to initialize ClangImporter: %s",
+                     underlying_error.c_str());
         }
-        LOG_PRINTF(GetLog(LLDBLog::Types), "%s", message.c_str());
       }
       if (clang_importer_ap)
         moduleCachePath = swift::getModuleCachePathFromClang(
@@ -4127,10 +4123,9 @@ swift::TypeBase *
 SwiftASTContext::ReconstructType(ConstString mangled_typename) {
   Status error;
 
-  auto reconstructed_type = this->ReconstructType(mangled_typename, error);
-  if (!error.Success()) {
-    this->AddErrorStatusAsGenericDiagnostic(error);
-  }
+  auto reconstructed_type = ReconstructType(mangled_typename, error);
+  if (!error.Success())
+    AddDiagnostic(eDiagnosticSeverityWarning, error.AsCString());
   return reconstructed_type;
 }
 
@@ -4761,19 +4756,32 @@ uint32_t SwiftASTContext::GetPointerByteSize() {
   return m_pointer_byte_size;
 }
 
-bool SwiftASTContext::HasErrors() {
-  if (m_diagnostic_consumer_ap.get())
-    return (
-        static_cast<StoringDiagnosticConsumer *>(m_diagnostic_consumer_ap.get())
-            ->NumErrors() != 0);
-  return false;
+bool SwiftASTContext::HasErrors() const {
+  if (!m_diagnostic_consumer_ap)
+    return false;
+  return (
+      static_cast<StoringDiagnosticConsumer *>(m_diagnostic_consumer_ap.get())
+          ->NumErrors() != 0);
 }
-bool SwiftASTContext::HasClangImporterErrors() {
-  if (m_diagnostic_consumer_ap.get())
-    return (
-        static_cast<StoringDiagnosticConsumer *>(m_diagnostic_consumer_ap.get())
-            ->NumClangErrors() != 0);
-  return false;
+bool SwiftASTContext::HasClangImporterErrors() const {
+  if (!m_diagnostic_consumer_ap)
+    return false;
+  return (
+      static_cast<StoringDiagnosticConsumer *>(m_diagnostic_consumer_ap.get())
+          ->NumClangErrors() != 0);
+}
+
+void SwiftASTContext::AddDiagnostic(DiagnosticSeverity severity,
+                                    llvm::StringRef message) {
+  assert(m_diagnostic_consumer_ap.get());
+  HEALTH_LOG_PRINTF("%s", message.str().c_str());
+  if (!m_diagnostic_consumer_ap.get())
+    return;
+
+  auto diagnostic = std::make_unique<Diagnostic>(
+      message, severity, eDiagnosticOriginLLDB, LLDB_INVALID_COMPILER_ID);
+  static_cast<StoringDiagnosticConsumer *>(m_diagnostic_consumer_ap.get())
+      ->AddDiagnostic(std::move(diagnostic));
 }
 
 bool SwiftASTContext::HasFatalErrors(swift::ASTContext *ast_context) {
@@ -4795,17 +4803,6 @@ bool SwiftASTContext::SetColorizeDiagnostics(bool b) {
   return false;
 }
 
-void SwiftASTContext::AddErrorStatusAsGenericDiagnostic(Status error) {
-  assert(!error.Success() && "status should be in an error state");
-
-  auto diagnostic = std::make_unique<Diagnostic>(
-      error.AsCString(), eDiagnosticSeverityError, eDiagnosticOriginLLDB,
-      LLDB_INVALID_COMPILER_ID);
-  if (m_diagnostic_consumer_ap.get())
-    static_cast<StoringDiagnosticConsumer *>(m_diagnostic_consumer_ap.get())
-        ->AddDiagnostic(std::move(diagnostic));
-}
-
 void SwiftASTContext::PrintDiagnostics(DiagnosticManager &diagnostic_manager,
                                        uint32_t bufferID, uint32_t first_line,
                                        uint32_t last_line) const {

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

@@ -16,14 +16,19 @@
 #include "Plugins/ExpressionParser/Swift/SwiftPersistentExpressionState.h"
 #include "Plugins/TypeSystem/Swift/TypeSystemSwift.h"
 #include "Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.h"
-#include "swift/SymbolGraphGen/SymbolGraphOptions.h"
-#include "swift/Parse/ParseVersion.h"
+
 #include "lldb/Core/SwiftForward.h"
 #include "lldb/Core/ThreadSafeDenseSet.h"
+#include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Utility/Either.h"
+
+#include "swift/Parse/ParseVersion.h"
+#include "swift/SymbolGraphGen/SymbolGraphOptions.h"
+
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Target/TargetOptions.h"
+
 #include <memory>
 
 namespace swift {
@@ -425,9 +430,10 @@ class SwiftASTContext : public TypeSystemSwift {
 
   /// Error handling
   /// \{
-  bool HasErrors();
-  bool HasClangImporterErrors();
+  bool HasErrors() const;
+  bool HasClangImporterErrors() const;
 
+  void AddDiagnostic(DiagnosticSeverity severity, llvm::StringRef message);
   void RaiseFatalError(std::string msg) { m_fatal_errors.SetErrorString(msg); }
   static bool HasFatalErrors(swift::ASTContext *ast_context);
   bool HasFatalErrors() const {
@@ -445,7 +451,6 @@ class SwiftASTContext : public TypeSystemSwift {
   void ClearDiagnostics();
 
   bool SetColorizeDiagnostics(bool b);
-  void AddErrorStatusAsGenericDiagnostic(Status error);
 
   void PrintDiagnostics(DiagnosticManager &diagnostic_manager,
                         uint32_t bufferID = UINT32_MAX, uint32_t first_line = 0,
@@ -470,11 +475,6 @@ class SwiftASTContext : public TypeSystemSwift {
 
   const SwiftModuleMap &GetModuleCache() { return m_swift_module_cache; }
 
-  /// Return a list of warnings collected from ClangImporter.
-  const std::vector<std::string> &GetModuleImportWarnings() const {
-    return m_module_import_warnings;
-  }
-
   const swift::irgen::TypeInfo *
   GetSwiftTypeInfo(lldb::opaque_compiler_type_t type);
 
@@ -866,9 +866,6 @@ class SwiftASTContext : public TypeSystemSwift {
   llvm::once_flag m_ir_gen_module_once;
   std::unique_ptr<swift::DiagnosticConsumer> m_diagnostic_consumer_ap;
   std::unique_ptr<swift::DependencyTracker> m_dependency_tracker;
-  /// A collection of (not necessarily fatal) error messages that
-  /// should be printed by Process::PrintWarningCantLoadSwift().
-  std::vector<std::string> m_module_import_warnings;
   swift::ModuleDecl *m_scratch_module = nullptr;
   std::unique_ptr<swift::Lowering::TypeConverter> m_sil_types_ap;
   std::unique_ptr<swift::SILModule> m_sil_module_ap;