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

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. You can't see it here, bug the word "error" is now also in
color, if so desired.

Depends on https://github.com/llvm/llvm-project/pull/106442.

24 files changed, 315 insertions, 51 deletions

diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 84ea9c0..6afa1c9 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -304,6 +304,8 @@ public:
 
   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 a72c259..1d5f2fc 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -364,6 +364,10 @@ public:
 
   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 62c6bce..b9a6421 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 20c4769..c5167e5 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -340,6 +340,13 @@ public:
       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 @@ protected:
   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 6b72994..47931f1 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 7711946..49f4421 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 a6cb951..e11aad2 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 9bdc5a3..e6b9eed 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 3731a20..7c67a0c 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 acd592c..d17aa6f 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 687962c..9b056ea 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -176,15 +176,22 @@ public:
     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 @@ public:
     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 @@ public:
       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 @@ public:
       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 @@ public:
           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 @@ public:
         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 1a034e8..06834edf 100644
--- a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
+++ b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
@@ -48,7 +48,7 @@ public:
   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 ddc1c35..1687b61 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -183,3 +183,54 @@ note: candidate function not viable: requires single argument 'x', but 2 argumen
         # 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 6a7995f..d0d9358 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 @@ class PersistentVariablesTestCase(TestBase):
         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 @@ class PersistentVariablesTestCase(TestBase):
         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 ea3aa6a..c734033 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 @@ class StaticInitializers(TestBase):
         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 71df90e..3be93de 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 @@ class TemplateFunctionsTestCase(TestBase):
                 "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 8b73254..1637d59 100644
--- a/lldb/test/API/lang/mixed/TestMixedLanguages.py
+++ b/lldb/test/API/lang/mixed/TestMixedLanguages.py
@@ -40,7 +40,7 @@ class MixedLanguagesTestCase(TestBase):
         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 0611171..19ca404 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 8537799..f8cad5b 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 a249057..8c16828 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 14371da..afb1a1f 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 4608b77..7e04d4b 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 54cea99..14a7d6c 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 0000000..b0cf22d
--- /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"));
+  }
+}