Revert "[lldb] Inline expression evaluator error visualization (#106470)"

This reverts commit 49372d1.

Author: adrian-prantl

lldb/include/lldb/API/SBDebugger.h

@@ -304,8 +304,6 @@ class LLDB_API SBDebugger {
 
   bool GetUseColor() const;
 
-  bool SetShowInlineDiagnostics(bool);
-
   bool SetUseSourceCache(bool use_source_cache);
 
   bool GetUseSourceCache() const;

lldb/include/lldb/Core/Debugger.h

@@ -364,10 +364,6 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   const std::string &GetInstanceName() { return m_instance_name; }
 
-  bool GetShowInlineDiagnostics() const;
-
-  bool SetShowInlineDiagnostics(bool);
-
   bool LoadPlugin(const FileSpec &spec, Status &error);
 
   void RunIOHandlers();

lldb/include/lldb/Expression/DiagnosticManager.h

@@ -39,7 +39,6 @@ struct DiagnosticDetail {
     unsigned line = 0;
     uint16_t column = 0;
     uint16_t length = 0;
-    bool hidden = false;
     bool in_user_input = false;
   };
   /// Contains {{}, 1, 3, 3, true} in the example above.

lldb/include/lldb/Interpreter/CommandObject.h

@@ -340,13 +340,6 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
       return false;
   }
 
-  /// Set the command input as it appeared in the terminal. This
-  /// is used to have errors refer directly to the original command.
-  void SetOriginalCommandString(std::string s) { m_original_command = s; }
-
-  /// \param offset_in_command is on what column \c args_string
-  /// appears, if applicable. This enables diagnostics that refer back
-  /// to the user input.
   virtual void Execute(const char *args_string,
                        CommandReturnObject &result) = 0;
 
@@ -411,7 +404,6 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
   std::string m_cmd_help_short;
   std::string m_cmd_help_long;
   std::string m_cmd_syntax;
-  std::string m_original_command;
   Flags m_flags;
   std::vector<CommandArgumentEntry> m_arguments;
   lldb::CommandOverrideCallback m_deprecated_command_override_callback;

lldb/source/API/SBDebugger.cpp

@@ -1483,12 +1483,6 @@ bool SBDebugger::GetUseColor() const {
   return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false);
 }
 
-bool SBDebugger::SetShowInlineDiagnostics(bool value) {
-  LLDB_INSTRUMENT_VA(this, value);
-
-  return (m_opaque_sp ? m_opaque_sp->SetShowInlineDiagnostics(value) : false);
-}
-
 bool SBDebugger::SetUseSourceCache(bool value) {
   LLDB_INSTRUMENT_VA(this, value);
 

lldb/source/Commands/CommandObjectExpression.cpp

@@ -10,7 +10,6 @@
 
 #include "CommandObjectExpression.h"
 #include "lldb/Core/Debugger.h"
-#include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
 #include "lldb/Expression/UserExpression.h"
@@ -399,122 +398,6 @@ CanBeUsedForElementCountPrinting(ValueObject &valobj) {
   return Status();
 }
 
-static llvm::raw_ostream &PrintSeverity(Stream &stream,
-                                        lldb::Severity severity) {
-  llvm::HighlightColor color;
-  llvm::StringRef text;
-  switch (severity) {
-  case eSeverityError:
-    color = llvm::HighlightColor::Error;
-    text = "error: ";
-    break;
-  case eSeverityWarning:
-    color = llvm::HighlightColor::Warning;
-    text = "warning: ";
-    break;
-  case eSeverityInfo:
-    color = llvm::HighlightColor::Remark;
-    text = "note: ";
-    break;
-  }
-  return llvm::WithColor(stream.AsRawOstream(), color, llvm::ColorMode::Enable)
-         << text;
-}
-
-namespace lldb_private {
-// Public for unittesting.
-void RenderDiagnosticDetails(Stream &stream,
-                             std::optional<uint16_t> offset_in_command,
-                             bool show_inline,
-                             llvm::ArrayRef<DiagnosticDetail> details) {
-  if (details.empty())
-    return;
-
-  if (!offset_in_command) {
-    for (const DiagnosticDetail &detail : details) {
-      PrintSeverity(stream, detail.severity);
-      stream << detail.rendered << '\n';
-    }
-    return;
-  }
-
-  // Print a line with caret indicator(s) below the lldb prompt + command.
-  const size_t padding = *offset_in_command;
-  stream << std::string(padding, ' ');
-
-  size_t offset = 1;
-  std::vector<DiagnosticDetail> remaining_details, other_details,
-      hidden_details;
-  for (const DiagnosticDetail &detail : details) {
-    if (!show_inline || !detail.source_location) {
-      other_details.push_back(detail);
-      continue;
-    }
-    if (detail.source_location->hidden) {
-      hidden_details.push_back(detail);
-      continue;
-    }
-    if (!detail.source_location->in_user_input) {
-      other_details.push_back(detail);
-      continue;
-    }
-
-    auto &loc = *detail.source_location;
-    remaining_details.push_back(detail);
-    if (offset > loc.column)
-      continue;
-    stream << std::string(loc.column - offset, ' ') << '^';
-    if (loc.length > 1)
-      stream << std::string(loc.length - 1, '~');
-    offset = loc.column + 1;
-  }
-  stream << '\n';
-
-  // Work through each detail in reverse order using the vector/stack.
-  bool did_print = false;
-  for (auto detail = remaining_details.rbegin();
-       detail != remaining_details.rend();
-       ++detail, remaining_details.pop_back()) {
-    // Get the information to print this detail and remove it from the stack.
-    // Print all the lines for all the other messages first.
-    stream << std::string(padding, ' ');
-    size_t offset = 1;
-    for (auto &remaining_detail :
-         llvm::ArrayRef(remaining_details).drop_back(1)) {
-      uint16_t column = remaining_detail.source_location->column;
-      stream << std::string(column - offset, ' ') << "│";
-      offset = column + 1;
-    }
-
-    // Print the line connecting the ^ with the error message.
-    uint16_t column = detail->source_location->column;
-    if (offset <= column)
-      stream << std::string(column - offset, ' ') << "╰─ ";
-
-    // Print a colorized string based on the message's severity type.
-    PrintSeverity(stream, detail->severity);
-
-    // Finally, print the message and start a new line.
-    stream << detail->message << '\n';
-    did_print = true;
-  }
-
-  // Print the non-located details.
-  for (const DiagnosticDetail &detail : other_details) {
-    PrintSeverity(stream, detail.severity);
-    stream << detail.rendered << '\n';
-    did_print = true;
-  }
-
-  // Print the hidden details as a last resort.
-  if (!did_print)
-    for (const DiagnosticDetail &detail : hidden_details) {
-      PrintSeverity(stream, detail.severity);
-      stream << detail.rendered << '\n';
-    }
-}
-} // namespace lldb_private
-
 bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
                                                  Stream &output_stream,
                                                  Stream &error_stream,
@@ -603,34 +486,19 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
 
         result.SetStatus(eReturnStatusSuccessFinishResult);
       } else {
-        // Retrieve the diagnostics.
-        std::vector<DiagnosticDetail> details;
-        llvm::consumeError(llvm::handleErrors(
-            result_valobj_sp->GetError().ToError(),
-            [&](ExpressionError &error) { details = error.GetDetails(); }));
-        // Find the position of the expression in the command.
-        std::optional<uint16_t> expr_pos;
-        size_t nchar = m_original_command.find(expr);
-        if (nchar != std::string::npos)
-          expr_pos = nchar + GetDebugger().GetPrompt().size();
-
-        if (!details.empty()) {
-          bool show_inline =
-              GetDebugger().GetShowInlineDiagnostics() && !expr.contains('\n');
-          RenderDiagnosticDetails(error_stream, expr_pos, show_inline, details);
+        const char *error_cstr = result_valobj_sp->GetError().AsCString();
+        if (error_cstr && error_cstr[0]) {
+          const size_t error_cstr_len = strlen(error_cstr);
+          const bool ends_with_newline = error_cstr[error_cstr_len - 1] == '\n';
+          if (strstr(error_cstr, "error:") != error_cstr)
+            error_stream.PutCString("error: ");
+          error_stream.Write(error_cstr, error_cstr_len);
+          if (!ends_with_newline)
+            error_stream.EOL();
         } else {
-          const char *error_cstr = result_valobj_sp->GetError().AsCString();
-          llvm::StringRef error(error_cstr);
-          if (!error.empty()) {
-            if (!error.starts_with("error:"))
-              error_stream << "error: ";
-            error_stream << error;
-            if (!error.ends_with('\n'))
-              error_stream.EOL();
-          } else {
-            error_stream << "error: unknown error\n";
-          }
+          error_stream.PutCString("error: unknown error\n");
         }
+
         result.SetStatus(eReturnStatusFailed);
       }
     }

lldb/source/Core/CoreProperties.td

@@ -225,8 +225,4 @@ let Definition = "debugger" in {
     DefaultEnumValue<"eDWIMPrintVerbosityNone">,
     EnumValues<"OptionEnumValues(g_dwim_print_verbosities)">,
     Desc<"The verbosity level used by dwim-print.">;
-  def ShowInlineDiagnostics: Property<"show-inline-diagnostics", "Boolean">,
-    Global,
-    DefaultFalse,
-    Desc<"Controls whether diagnostics can refer directly to the command input, drawing arrows to it. If false, diagnostics will echo the input.">;
 }

lldb/source/Core/Debugger.cpp

@@ -592,18 +592,7 @@ lldb::DWIMPrintVerbosity Debugger::GetDWIMPrintVerbosity() const {
   const uint32_t idx = ePropertyDWIMPrintVerbosity;
   return GetPropertyAtIndexAs<lldb::DWIMPrintVerbosity>(
       idx, static_cast<lldb::DWIMPrintVerbosity>(
-               g_debugger_properties[idx].default_uint_value != 0));
-}
-
-bool Debugger::GetShowInlineDiagnostics() const {
-  const uint32_t idx = ePropertyShowInlineDiagnostics;
-  return GetPropertyAtIndexAs<bool>(
-      idx, g_debugger_properties[idx].default_uint_value);
-}
-
-bool Debugger::SetShowInlineDiagnostics(bool b) {
-  const uint32_t idx = ePropertyShowInlineDiagnostics;
-  return SetPropertyAtIndex(idx, b);
+               g_debugger_properties[idx].default_uint_value));
 }
 
 #pragma mark Debugger

lldb/source/Expression/DiagnosticManager.cpp

@@ -37,7 +37,7 @@ ExpressionError::ExpressionError(lldb::ExpressionResults result,
     : ErrorInfo(std::error_code(result, expression_category())), m_message(msg),
       m_details(details) {}
 
-static llvm::StringRef StringForSeverity(lldb::Severity severity) {
+static const char *StringForSeverity(lldb::Severity severity) {
   switch (severity) {
   // this should be exhaustive
   case lldb::eSeverityError:

lldb/source/Interpreter/CommandInterpreter.cpp

@@ -1887,8 +1887,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
                                        CommandReturnObject &result,
                                        bool force_repeat_command) {
   std::string command_string(command_line);
-  std::string original_command_string(command_string);
-  std::string real_original_command_string(command_string);
+  std::string original_command_string(command_line);
 
   Log *log = GetLog(LLDBLog::Commands);
   llvm::PrettyStackTraceFormat stack_trace("HandleCommand(command = \"%s\")",
@@ -2077,7 +2076,6 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
     }
 
     ElapsedTime elapsed(execute_time);
-    cmd_obj->SetOriginalCommandString(real_original_command_string);
     cmd_obj->Execute(remainder.c_str(), result);
   }
 

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

@@ -176,22 +176,15 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
     m_manager = manager;
   }
 
-  /// Returns the last error ClangDiagnostic message that the
-  /// DiagnosticManager received or a nullptr.
+  /// Returns the last ClangDiagnostic message that the DiagnosticManager
+  /// received or a nullptr if the DiagnosticMangager hasn't seen any
+  /// Clang diagnostics yet.
   ClangDiagnostic *MaybeGetLastClangDiag() const {
     if (m_manager->Diagnostics().empty())
       return nullptr;
-    auto &diags = m_manager->Diagnostics();
-    for (auto it = diags.rbegin(); it != diags.rend(); it++) {
-      lldb_private::Diagnostic *diag = it->get();
-      if (ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag)) {
-        if (clang_diag->GetSeverity() == lldb::eSeverityWarning)
-          return nullptr;
-        if (clang_diag->GetSeverity() == lldb::eSeverityError)
-          return clang_diag;
-      }
-    }
-    return nullptr;
+    lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get();
+    ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag);
+    return clang_diag;
   }
 
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
@@ -221,6 +214,8 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
     m_passthrough->HandleDiagnostic(DiagLevel, Info);
 
     DiagnosticDetail detail;
+    bool make_new_diagnostic = true;
+
     switch (DiagLevel) {
     case DiagnosticsEngine::Level::Fatal:
     case DiagnosticsEngine::Level::Error:
@@ -234,6 +229,9 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
       detail.severity = lldb::eSeverityInfo;
       break;
     case DiagnosticsEngine::Level::Note:
+      m_manager->AppendMessageToDiagnostic(m_output);
+      make_new_diagnostic = false;
+
       // 'note:' diagnostics for errors and warnings can also contain Fix-Its.
       // We add these Fix-Its to the last error diagnostic to make sure
       // that we later have all Fix-Its related to an 'error' diagnostic when
@@ -251,6 +249,7 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
       AddAllFixIts(clang_diag, Info);
       break;
     }
+    if (make_new_diagnostic) {
       // ClangDiagnostic messages are expected to have no whitespace/newlines
       // around them.
       std::string stripped_output =
@@ -270,7 +269,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
           loc.line = fsloc.getSpellingLineNumber();
           loc.column = fsloc.getSpellingColumnNumber();
           loc.in_user_input = filename == m_filename;
-          loc.hidden = filename.starts_with("<lldb wrapper ");
 
           // Find the range of the primary location.
           for (const auto &range : Info.getRanges()) {
@@ -300,6 +298,7 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
         AddAllFixIts(new_diagnostic.get(), Info);
 
       m_manager->AddDiagnostic(std::move(new_diagnostic));
+    }
   }
 
   void BeginSourceFile(const LangOptions &LO, const Preprocessor *PP) override {

lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h

@@ -48,7 +48,7 @@ class CommandObjectThreadTraceExportCTF : public CommandObjectParsed {
   Options *GetOptions() override { return &m_options; }
 
 protected:
-  void DoExecute(Args &args, CommandReturnObject &result) override;
+  void DoExecute(Args &command, CommandReturnObject &result) override;
 
   CommandOptions m_options;
 };