[Lldb-commits] [lldb] 133c3ea - Streamline expression parser error messages.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2023-06-12T10:29:48-07:00
New Revision: 133c3eaac0a532380c3d6ad21a60da1490f51fb8

URL: https://github.com/llvm/llvm-project/commit/133c3eaac0a532380c3d6ad21a60da1490f51fb8
DIFF: https://github.com/llvm/llvm-project/commit/133c3eaac0a532380c3d6ad21a60da1490f51fb8.diff

LOG: Streamline expression parser error messages.

Currently the expression parser prints a mostly useless generic error before printing the compiler error:

  (lldb) p 1+x)
  error: expression failed to parse:
  error: <user expression 18>:1:3: use of undeclared identifier 'x'
  1+x)
     ^

This is distracting and as far as I can tell only exists to work
around the fact that the first "error: " is unconditionally injected
by CommandReturnObject. The solution is not very elegant, but the
result looks much better.

(Partially addresses rdar://110492710)

Differential Revision: https://reviews.llvm.org/D152590

Added: 
    

Modified: 
    lldb/source/Expression/UserExpression.cpp
    lldb/source/Interpreter/CommandReturnObject.cpp
    lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
    lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index b250177db9979..8e1cac7099dc2 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -330,11 +330,10 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
       std::string msg;
       {
         llvm::raw_string_ostream os(msg);
-        os << "expression failed to parse:\n";
         if (!diagnostic_manager.Diagnostics().empty())
           os << diagnostic_manager.GetString();
         else
-          os << "unknown error";
+          os << "expression failed to parse (no further compiler diagnostics)";
         if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
             !fixed_expression->empty())
           os << "\nfixed expression suggested:\n  " << *fixed_expression;

diff  --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index 4433c43ff6d46..0bc58124f3941 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -101,7 +101,10 @@ void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   SetStatus(eReturnStatusFailed);
   if (in_string.empty())
     return;
-  error(GetErrorStream()) << in_string.rtrim() << '\n';
+  // Workaround to deal with already fully formatted compiler diagnostics.
+  llvm::StringRef msg(in_string.rtrim());
+  msg.consume_front("error: ");
+  error(GetErrorStream()) << msg << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,

diff  --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index bf1fc8e4e0e0a..d64e9897a844f 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -151,8 +151,7 @@ def test_source_and_caret_printing(self):
         value = frame.EvaluateExpression("struct Redef { float y; };", top_level_opts)
         self.assertFalse(value.GetError().Success())
         self.assertIn(
-            """
-error: <user expression 9>:1:8: redefinition of 'Redef'
+            """error: <user expression 9>:1:8: redefinition of 'Redef'
     1 | struct Redef { float y; };
       |        ^
 <user expression 8>:1:8: previous definition is here

diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index 484c255e06860..3bdeb84b4e797 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -154,7 +154,6 @@ def test_with_multiple_retries(self):
         multiple_runs_options.SetRetriesWithFixIts(0)
         value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
         errmsg = value.GetError().GetCString()
-        self.assertIn("expression failed to parse", errmsg)
         self.assertIn("using declaration resolved to type without 'typename'", errmsg)
         self.assertIn("fixed expression suggested:", errmsg)
         self.assertIn("using typename T::TypeDef", errmsg)
@@ -166,7 +165,6 @@ def test_with_multiple_retries(self):
         multiple_runs_options.SetRetriesWithFixIts(1)
         value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
         errmsg = value.GetError().GetCString()
-        self.assertIn("expression failed to parse", errmsg)
         self.assertIn("fixed expression suggested:", errmsg)
         # Both our fixed expressions should be in the suggested expression.
         self.assertIn("using typename T::TypeDef", errmsg)