Also display the underlying error message when displaying a fixit

[aprantl]

Summary

When the user running LLDB with default settings sees the fixit notification it means that the auto-applied fixit didn't work. This patch shows the underlying error message instead of just the fixit to make it easier to understand what the error in the expression was.

[teemperor]

LGTM, thanks!

lldb/source/Expression/UserExpression.cpp

309

Nit: if (!diagnostic_manager.Diagnostics().empty()) {

[stella.stamenova]

It looks like this broke the Windows LLDB bot: https://lab.llvm.org/buildbot/#/builders/83/builds/6085

lldb/source/Expression/UserExpression.cpp

if (fixed_expression && !fixed_expression->empty() &&
             fixed_expression->clear();
             break;
           }
         }
       }
     }
 
     if (!parse_success) {
-      if (!fixed_expression->empty() && target->GetEnableNotifyAboutFixIts()) {
-        error.SetExpressionErrorWithFormat(
-            execution_results,
-            "expression failed to parse, fixed expression suggested:\n  %s",
-            fixed_expression->c_str());
-      } else {
-        if (!diagnostic_manager.Diagnostics().size())
-          error.SetExpressionError(execution_results,
-                                   "expression failed to parse, unknown error");
+      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
-          error.SetExpressionError(execution_results,
-                                   diagnostic_manager.GetString().c_str());
+          os << "unknown error";
+        if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
+            !fixed_expression->empty())
+          os << "\nfixed expression suggested:\n  " << *fixed_expression;
       }
+      error.SetExpressionError(execution_results, msg.c_str());
     }
   }
 
   if (parse_success) {
     lldb::ExpressionVariableSP expr_result;
 
     if (execution_policy == eExecutionPolicyNever &&
         !user_expression_sp->CanInterpret()) {

lldb/test/API/commands/expression/fixits/TestFixIts.py

def test_with_multiple_retries(self):
           S1<ClassWithTypeDef> s1;
           return s1.f();
         }
         """
 
         # Disable retries which will fail.
         multiple_runs_options.SetRetriesWithFixIts(0)
         value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
-        self.assertIn("expression failed to parse, fixed expression suggested:",
-                      value.GetError().GetCString())
-        self.assertIn("using typename T::TypeDef",
-                      value.GetError().GetCString())
+        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)
         # The second Fix-It shouldn't be suggested here as Clang should have
         # aborted the parsing process.
-        self.assertNotIn("i->m",
-                      value.GetError().GetCString())
+        self.assertNotIn("i->m", errmsg)
 
         # Retry once, but the expression needs two retries.
         multiple_runs_options.SetRetriesWithFixIts(1)
         value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
-        self.assertIn("expression failed to parse, fixed expression suggested:",
-                      value.GetError().GetCString())
+        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",
-                      value.GetError().GetCString())
-        self.assertIn("i->m",
-                      value.GetError().GetCString())
+        self.assertIn("using typename T::TypeDef", errmsg)
+        self.assertIn("i->m", errmsg)
 
         # Retry twice, which will get the expression working.
         multiple_runs_options.SetRetriesWithFixIts(2)
         value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
         # This error signals success for top level expressions.
         self.assertEquals(value.GetError().GetCString(), "error: No value")
 
         # Test that the code above compiles to the right thing.
         self.expect_expr("test_X(1)", result_type="int", result_value="123")