Author: Adrian Prantl
Date: 2024-09-27T16:32:35-07:00
New Revision: 49372d1cccf50f404d52d40ae4b663db5604eb2c

URL: 
https://github.com/llvm/llvm-project/commit/49372d1cccf50f404d52d40ae4b663db5604eb2c
DIFF: 
https://github.com/llvm/llvm-project/commit/49372d1cccf50f404d52d40ae4b663db5604eb2c.diff

LOG: [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.

Added: 
    lldb/unittests/Interpreter/TestCommandObjectExpression.cpp

Modified: 
    lldb/include/lldb/API/SBDebugger.h
    lldb/include/lldb/Core/Debugger.h
    lldb/include/lldb/Expression/DiagnosticManager.h
    lldb/include/lldb/Interpreter/CommandObject.h
    lldb/source/API/SBDebugger.cpp
    lldb/source/Commands/CommandObjectExpression.cpp
    lldb/source/Core/CoreProperties.td
    lldb/source/Core/Debugger.cpp
    lldb/source/Expression/DiagnosticManager.cpp
    lldb/source/Interpreter/CommandInterpreter.cpp
    lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
    lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
    lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
    
lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
    
lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py
    lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
    lldb/test/API/lang/mixed/TestMixedLanguages.py
    lldb/test/Shell/Expr/TestObjCIDCast.test
    lldb/test/Shell/Expr/TestObjCInCXXContext.test
    lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
    lldb/tools/driver/Driver.cpp
    lldb/unittests/Expression/DiagnosticManagerTest.cpp
    lldb/unittests/Interpreter/CMakeLists.txt

Removed: 
    


################################################################################
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"));
+  }
+}


        
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to