[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)

Adrian Prantl via lldb-commits lldb-commits at lists.llvm.org
Fri Sep 27 16:24:17 PDT 2024


https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/106470

>From 4f57669afd3d9e7475c0e44976eaa9b534883152 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Wed, 28 Aug 2024 16:19:21 -0700
Subject: [PATCH] [lldb] Inline expression evaluator error visualization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch is a reworking of Pete Lawrence's (@PortalPete) proposal
for better expression evaluator error messages:
https://github.com/llvm/llvm-project/pull/80938

Before:

```
$ lldb -o "expr a+b"
(lldb) expr a+b
error: <user expression 0>:1:1: use of undeclared identifier 'a'
a+b
^
error: <user expression 0>:1:3: use of undeclared identifier 'b'
a+b
  ^
```

After:

```
(lldb) expr a+b
            ^ ^
            │ ╰─ error: use of undeclared identifier 'b'
            ╰─ error: use of undeclared identifier 'a'
```

This eliminates the confusing `<user expression 0>:1:3` source
location and avoids echoing the expression to the console again, which
results in a cleaner presentation that makes it easier to grasp what's
going on.
---
 lldb/include/lldb/API/SBDebugger.h            |   2 +
 lldb/include/lldb/Core/Debugger.h             |   4 +
 .../lldb/Expression/DiagnosticManager.h       |   1 +
 lldb/include/lldb/Interpreter/CommandObject.h |   8 +
 lldb/source/API/SBDebugger.cpp                |   6 +
 .../Commands/CommandObjectExpression.cpp      | 154 ++++++++++++++++--
 lldb/source/Core/CoreProperties.td            |   4 +
 lldb/source/Core/Debugger.cpp                 |  13 +-
 lldb/source/Expression/DiagnosticManager.cpp  |   2 +-
 .../source/Interpreter/CommandInterpreter.cpp |   4 +-
 .../Clang/ClangExpressionParser.cpp           |  27 +--
 .../ctf/CommandObjectThreadTraceExportCTF.h   |   2 +-
 .../diagnostics/TestExprDiagnostics.py        |  51 ++++++
 .../TestPersistentVariables.py                |   4 +-
 .../TestStaticInitializers.py                 |   2 +-
 .../TestTemplateFunctions.py                  |   3 +-
 .../test/API/lang/mixed/TestMixedLanguages.py |   2 +-
 lldb/test/Shell/Expr/TestObjCIDCast.test      |   2 +-
 .../test/Shell/Expr/TestObjCInCXXContext.test |   2 +-
 .../NativePDB/incomplete-tag-type.cpp         |   4 +-
 lldb/tools/driver/Driver.cpp                  |   1 +
 .../Expression/DiagnosticManagerTest.cpp      |  33 ++--
 lldb/unittests/Interpreter/CMakeLists.txt     |   1 +
 .../TestCommandObjectExpression.cpp           |  34 ++++
 24 files changed, 315 insertions(+), 51 deletions(-)
 create mode 100644 lldb/unittests/Interpreter/TestCommandObjectExpression.cpp

diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 84ea9c0f772e16..6afa1c932ab050 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -304,6 +304,8 @@ class LLDB_API SBDebugger {
 
   bool GetUseColor() const;
 
+  bool SetShowInlineDiagnostics(bool);
+
   bool SetUseSourceCache(bool use_source_cache);
 
   bool GetUseSourceCache() const;
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index a72c2596cc2c5e..1d5f2fcc20626c 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -364,6 +364,10 @@ 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();
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index 62c6bcefe54110..b9a6421577781e 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -39,6 +39,7 @@ 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.
diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h
index 20c4769af90338..c5167e5e0ecb6a 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -340,6 +340,13 @@ 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;
 
@@ -404,6 +411,7 @@ 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;
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 6b72994fc96afb..47931f1c16f9a3 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1483,6 +1483,12 @@ 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);
 
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 771194638e1b65..49f4421fdb54f9 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -10,6 +10,7 @@
 
 #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"
@@ -398,6 +399,122 @@ 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,
@@ -486,19 +603,34 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
 
         result.SetStatus(eReturnStatusSuccessFinishResult);
       } else {
-        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();
+        // 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);
         } else {
-          error_stream.PutCString("error: unknown error\n");
+          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";
+          }
         }
-
         result.SetStatus(eReturnStatusFailed);
       }
     }
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index a6cb951187a040..e11aad2660b461 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -225,4 +225,8 @@ 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.">;
 }
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 9bdc5a3949751d..e6b9eedd89b4e3 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -592,7 +592,18 @@ 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));
+               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);
 }
 
 #pragma mark Debugger
diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index 3731a200c5e2f7..7c67a0ce4aa026 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/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 const char *StringForSeverity(lldb::Severity severity) {
+static llvm::StringRef StringForSeverity(lldb::Severity severity) {
   switch (severity) {
   // this should be exhaustive
   case lldb::eSeverityError:
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index acd592c3bd2dbc..d17aa6fec1f00e 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1887,7 +1887,8 @@ 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_line);
+  std::string original_command_string(command_string);
+  std::string real_original_command_string(command_string);
 
   Log *log = GetLog(LLDBLog::Commands);
   llvm::PrettyStackTraceFormat stack_trace("HandleCommand(command = \"%s\")",
@@ -2076,6 +2077,7 @@ 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);
   }
 
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 687962c700a30b..9b056ea73a77fb 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -176,15 +176,22 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
     m_manager = manager;
   }
 
-  /// Returns the last ClangDiagnostic message that the DiagnosticManager
-  /// received or a nullptr if the DiagnosticMangager hasn't seen any
-  /// Clang diagnostics yet.
+  /// Returns the last error ClangDiagnostic message that the
+  /// DiagnosticManager received or a nullptr.
   ClangDiagnostic *MaybeGetLastClangDiag() const {
     if (m_manager->Diagnostics().empty())
       return nullptr;
-    lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get();
-    ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag);
-    return clang_diag;
+    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;
   }
 
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
@@ -214,8 +221,6 @@ 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:
@@ -229,9 +234,6 @@ 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
@@ -249,7 +251,6 @@ 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 =
@@ -269,6 +270,7 @@ 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()) {
@@ -298,7 +300,6 @@ 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 {
diff --git a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
index 1a034e87cfb65b..06834edf14ea16 100644
--- a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
+++ b/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 &command, CommandReturnObject &result) override;
+  void DoExecute(Args &args, CommandReturnObject &result) override;
 
   CommandOptions m_options;
 };
diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index ddc1c3598480cf..1687b617350d92 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -183,3 +183,54 @@ def test_source_locations_from_objc_modules(self):
         # The NSLog definition source line should be printed. Return value and
         # the first argument are probably stable enough that this test can check for them.
         self.assertIn("void NSLog(NSString *format", value.GetError().GetCString())
+
+    def test_command_expr_formatting(self):
+        """Test that the source and caret positions LLDB prints are correct"""
+        self.build()
+
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "// Break here", self.main_source_spec
+        )
+        frame = thread.GetFrameAtIndex(0)
+        self.expect("settings set show-inline-diagnostics true")
+
+        def check(input_ref):
+            self.expect(input_ref[0], error=True, substrs=input_ref[1:])
+
+        check(
+            [
+                "expression -- a+b",
+                "              ^ ^",
+                "              │ ╰─ error: use of undeclared identifier 'b'",
+                "              ╰─ error: use of undeclared identifier 'a'",
+            ]
+        )
+
+        check(
+            [
+                "expr -- a",
+                "        ^",
+                "        ╰─ error: use of undeclared identifier 'a'",
+            ]
+        )
+        check(
+            [
+                "expr -i 0 -o 0 -- a",
+                "                  ^",
+                "                  ╰─ error: use of undeclared identifier 'a'",
+            ]
+        )
+
+        self.expect(
+            "expression --top-level -- template<typename T> T FOO(T x) { return x/2;}"
+        )
+        check(
+            [
+                'expression -- FOO("")',
+                "              ^",
+                "              ╰─ note: in instantiation of function template specialization 'FOO<const char *>' requested here",
+                "error: <user expression",
+                "invalid operands to binary expression",
+            ]
+        )
+        check(["expression --\na\n+\nb", "error: <user", "a", "error: <user", "b"])
diff --git a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
index 6a7995ff2a837e..d0d9358589a719 100644
--- a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
+++ b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
@@ -56,7 +56,7 @@ def test_persistent_variables(self):
         self.expect(
             "expr int $i = 123",
             error=True,
-            substrs=["error: redefinition of persistent variable '$i'"],
+            substrs=["redefinition of persistent variable '$i'"],
         )
         self.expect_expr("$i", result_type="int", result_value="5")
 
@@ -65,7 +65,7 @@ def test_persistent_variables(self):
         self.expect(
             "expr long $i = 123",
             error=True,
-            substrs=["error: redefinition of persistent variable '$i'"],
+            substrs=["redefinition of persistent variable '$i'"],
         )
         self.expect_expr("$i", result_type="int", result_value="5")
 
diff --git a/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py b/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py
index ea3aa6a4608c41..c734033bd6c028 100644
--- a/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py
+++ b/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py
@@ -35,7 +35,7 @@ def test_failing_init(self):
         self.expect(
             "expr -p -- struct Foo2 { Foo2() { do_abort(); } }; Foo2 f;",
             error=True,
-            substrs=["error: couldn't run static initializer:"],
+            substrs=["couldn't run static initializer:"],
         )
 
     def test_without_process(self):
diff --git a/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py b/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
index 71df90e6a6d161..3be93dedfd11df 100644
--- a/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
+++ b/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
@@ -21,7 +21,8 @@ def do_test_template_function(self, add_cast):
                 "expr b1 <=> b2",
                 error=True,
                 substrs=[
-                    "warning: <user expression 0>:1:4: '<=>' is a single token in C++20; add a space to avoid a change in behavior"
+                    "warning:",
+                    "'<=>' is a single token in C++20; add a space to avoid a change in behavior",
                 ],
             )
 
diff --git a/lldb/test/API/lang/mixed/TestMixedLanguages.py b/lldb/test/API/lang/mixed/TestMixedLanguages.py
index 8b73254cce4a93..1637d59a5edcba 100644
--- a/lldb/test/API/lang/mixed/TestMixedLanguages.py
+++ b/lldb/test/API/lang/mixed/TestMixedLanguages.py
@@ -40,7 +40,7 @@ def cleanup():
         self.runCmd("run")
         self.expect("thread backtrace", substrs=["`main", "lang=c"])
         # Make sure evaluation of C++11 fails.
-        self.expect("expr foo != nullptr", error=True, startstr="error")
+        self.expect("expr foo != nullptr", error=True, substrs=["error"])
 
         # Run to BP at foo (in foo.cpp) and test that the language is C++.
         self.runCmd("breakpoint set -n foo")
diff --git a/lldb/test/Shell/Expr/TestObjCIDCast.test b/lldb/test/Shell/Expr/TestObjCIDCast.test
index 0611171da09e2e..19ca404643c1d9 100644
--- a/lldb/test/Shell/Expr/TestObjCIDCast.test
+++ b/lldb/test/Shell/Expr/TestObjCIDCast.test
@@ -6,4 +6,4 @@
 // RUN:   2>&1 | FileCheck %s
 
 // CHECK: (lldb) expression --language objc -- *(id)0x1
-// CHECK: error: Couldn't apply expression side effects : Couldn't dematerialize a result variable: couldn't read its memory
+// CHECK: error:{{.*}}Couldn't apply expression side effects : Couldn't dematerialize a result variable: couldn't read its memory
diff --git a/lldb/test/Shell/Expr/TestObjCInCXXContext.test b/lldb/test/Shell/Expr/TestObjCInCXXContext.test
index 8537799bdeb674..f8cad5b58a1e53 100644
--- a/lldb/test/Shell/Expr/TestObjCInCXXContext.test
+++ b/lldb/test/Shell/Expr/TestObjCInCXXContext.test
@@ -18,4 +18,4 @@
 // CHECK-NEXT: (NSString *){{.*}}= nil
 
 // CHECK:      (lldb) expression NSString
-// CHECK-NEXT: error:{{.*}} use of undeclared identifier 'NSString'
+// CHECK:      error:{{.*}}use of undeclared identifier 'NSString'
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
index a249057282d890..8c168286903014 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
@@ -13,9 +13,7 @@
 // CHECK: (lldb) expression d
 // CHECK: (D) $1 = {}
 // CHECK: (lldb) expression static_e_ref
-// CHECK: error: {{.*}}incomplete type 'E' where a complete type is required
-// CHECK: static_e_ref
-// CHECK: ^
+// CHECK: error:{{.*}}incomplete type 'E' where a complete type is required
 
 // Complete base class.
 struct A { int x; A(); };
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 14371da64f2f2f..afb1a1ff95c3a1 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -442,6 +442,7 @@ int Driver::MainLoop() {
   m_debugger.SetInputFileHandle(stdin, false);
 
   m_debugger.SetUseExternalEditor(m_option_data.m_use_external_editor);
+  m_debugger.SetShowInlineDiagnostics(true);
 
   struct winsize window_size;
   if ((isatty(STDIN_FILENO) != 0) &&
diff --git a/lldb/unittests/Expression/DiagnosticManagerTest.cpp b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
index 4608b779f79a96..7e04d4b023e4c2 100644
--- a/lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -72,18 +72,25 @@ TEST(DiagnosticManagerTest, HasFixits) {
   EXPECT_TRUE(mgr.HasFixIts());
 }
 
+static std::string toString(DiagnosticManager &mgr) {
+  // The error code doesn't really matter since we just convert the
+  // diagnostics to a string.
+  auto result = lldb::eExpressionCompleted;
+  return llvm::toString(mgr.GetAsError(result));
+}
+
 TEST(DiagnosticManagerTest, GetStringNoDiags) {
   DiagnosticManager mgr;
-  EXPECT_EQ("", mgr.GetString());
+  EXPECT_EQ("", toString(mgr));
   std::unique_ptr<Diagnostic> empty;
   mgr.AddDiagnostic(std::move(empty));
-  EXPECT_EQ("", mgr.GetString());
+  EXPECT_EQ("", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, GetStringBasic) {
   DiagnosticManager mgr;
   mgr.AddDiagnostic(std::make_unique<TextDiag>("abc", lldb::eSeverityError));
-  EXPECT_EQ("error: abc\n", mgr.GetString());
+  EXPECT_EQ("error: abc\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, GetStringMultiline) {
@@ -91,15 +98,15 @@ TEST(DiagnosticManagerTest, GetStringMultiline) {
 
   // Multiline diagnostics should only get one severity label.
   mgr.AddDiagnostic(std::make_unique<TextDiag>("b\nc", lldb::eSeverityError));
-  EXPECT_EQ("error: b\nc\n", mgr.GetString());
+  EXPECT_EQ("error: b\nc\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, GetStringMultipleDiags) {
   DiagnosticManager mgr;
   mgr.AddDiagnostic(std::make_unique<TextDiag>("abc", lldb::eSeverityError));
-  EXPECT_EQ("error: abc\n", mgr.GetString());
+  EXPECT_EQ("error: abc\n", toString(mgr));
   mgr.AddDiagnostic(std::make_unique<TextDiag>("def", lldb::eSeverityError));
-  EXPECT_EQ("error: abc\nerror: def\n", mgr.GetString());
+  EXPECT_EQ("error: abc\nerror: def\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, GetStringSeverityLabels) {
@@ -110,7 +117,7 @@ TEST(DiagnosticManagerTest, GetStringSeverityLabels) {
   mgr.AddDiagnostic(std::make_unique<TextDiag>("bar", lldb::eSeverityWarning));
   // Remarks have no labels.
   mgr.AddDiagnostic(std::make_unique<TextDiag>("baz", lldb::eSeverityInfo));
-  EXPECT_EQ("error: foo\nwarning: bar\nbaz\n", mgr.GetString());
+  EXPECT_EQ("error: foo\nwarning: bar\nbaz\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, GetStringPreserveOrder) {
@@ -120,7 +127,7 @@ TEST(DiagnosticManagerTest, GetStringPreserveOrder) {
   mgr.AddDiagnostic(std::make_unique<TextDiag>("baz", lldb::eSeverityInfo));
   mgr.AddDiagnostic(std::make_unique<TextDiag>("bar", lldb::eSeverityWarning));
   mgr.AddDiagnostic(std::make_unique<TextDiag>("foo", lldb::eSeverityError));
-  EXPECT_EQ("baz\nwarning: bar\nerror: foo\n", mgr.GetString());
+  EXPECT_EQ("baz\nwarning: bar\nerror: foo\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, AppendMessageNoDiag) {
@@ -139,7 +146,7 @@ TEST(DiagnosticManagerTest, AppendMessageAttachToLastDiag) {
   // This should append to 'bar' and not to 'foo'.
   mgr.AppendMessageToDiagnostic("message text");
 
-  EXPECT_EQ("error: foo\nerror: bar\nmessage text\n", mgr.GetString());
+  EXPECT_EQ("error: foo\nerror: bar\nmessage text\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, AppendMessageSubsequentDiags) {
@@ -150,7 +157,7 @@ TEST(DiagnosticManagerTest, AppendMessageSubsequentDiags) {
   // Pushing another diag after the message should work fine.
   mgr.AddDiagnostic(std::make_unique<TextDiag>("foo", lldb::eSeverityError));
 
-  EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", mgr.GetString());
+  EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, PutString) {
@@ -159,7 +166,7 @@ TEST(DiagnosticManagerTest, PutString) {
   mgr.PutString(lldb::eSeverityError, "foo");
   EXPECT_EQ(1U, mgr.Diagnostics().size());
   EXPECT_EQ(eDiagnosticOriginLLDB, mgr.Diagnostics().front()->getKind());
-  EXPECT_EQ("error: foo\n", mgr.GetString());
+  EXPECT_EQ("error: foo\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, PutStringMultiple) {
@@ -169,7 +176,7 @@ TEST(DiagnosticManagerTest, PutStringMultiple) {
   mgr.PutString(lldb::eSeverityError, "foo");
   mgr.PutString(lldb::eSeverityError, "bar");
   EXPECT_EQ(2U, mgr.Diagnostics().size());
-  EXPECT_EQ("error: foo\nerror: bar\n", mgr.GetString());
+  EXPECT_EQ("error: foo\nerror: bar\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, PutStringSeverities) {
@@ -180,7 +187,7 @@ TEST(DiagnosticManagerTest, PutStringSeverities) {
   mgr.PutString(lldb::eSeverityError, "foo");
   mgr.PutString(lldb::eSeverityWarning, "bar");
   EXPECT_EQ(2U, mgr.Diagnostics().size());
-  EXPECT_EQ("error: foo\nwarning: bar\n", mgr.GetString());
+  EXPECT_EQ("error: foo\nwarning: bar\n", toString(mgr));
 }
 
 TEST(DiagnosticManagerTest, FixedExpression) {
diff --git a/lldb/unittests/Interpreter/CMakeLists.txt b/lldb/unittests/Interpreter/CMakeLists.txt
index 54cea995084d3d..14a7d6c5610388 100644
--- a/lldb/unittests/Interpreter/CMakeLists.txt
+++ b/lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(InterpreterTests
   TestCommandPaths.cpp
+  TestCommandObjectExpression.cpp
   TestCompletion.cpp
   TestOptionArgParser.cpp
   TestOptions.cpp
diff --git a/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp b/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
new file mode 100644
index 00000000000000..b0cf22d3cf0903
--- /dev/null
+++ b/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
@@ -0,0 +1,34 @@
+#include "lldb/Expression/DiagnosticManager.h"
+#include "lldb/Utility/StreamString.h"
+#include "gtest/gtest.h"
+
+namespace lldb_private {
+std::string RenderDiagnosticDetails(Stream &stream,
+                                    std::optional<uint16_t> offset_in_command,
+                                    bool multiline,
+                                    llvm::ArrayRef<DiagnosticDetail> details);
+}
+
+using namespace lldb_private;
+using namespace lldb;
+using llvm::StringRef;
+namespace {
+class ErrorDisplayTest : public ::testing::Test {};
+} // namespace
+
+static std::string Render(std::vector<DiagnosticDetail> details) {
+  StreamString stream;
+  RenderDiagnosticDetails(stream, 0, true, details);
+  return stream.GetData();
+}
+
+TEST_F(ErrorDisplayTest, RenderStatus) {
+  DiagnosticDetail::SourceLocation inline_loc;
+  inline_loc.in_user_input = true;
+  {
+    std::string result =
+        Render({DiagnosticDetail{inline_loc, eSeverityError, "foo", ""}});
+    ASSERT_TRUE(StringRef(result).contains("error:"));
+    ASSERT_TRUE(StringRef(result).contains("foo"));
+  }
+}



More information about the lldb-commits mailing list