[Lldb-commits] [lldb] [lldb] Add Status::Detail to store expression evaluator diagnostics [… (PR #106442)

[Med Ismail Bennani via lldb-commits]

================
@@ -328,18 +328,19 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
     }
 
     if (!parse_success) {
-      std::string msg;
-      {
-        llvm::raw_string_ostream os(msg);
-        if (!diagnostic_manager.Diagnostics().empty())
-          os << diagnostic_manager.GetString();
-        else
-          os << "expression failed to parse (no further compiler diagnostics)";
-        if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
-            !fixed_expression->empty())
-          os << "\nfixed expression suggested:\n  " << *fixed_expression;
+      if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
+          !fixed_expression->empty()) {
+        std::string fixit =
+            "fixed expression suggested:\n  " + *fixed_expression;
+        diagnostic_manager.AddDiagnostic(fixit, lldb::eSeverityInfo,
+                                         eDiagnosticOriginLLDB);
       }
-      error = Status::FromExpressionError(execution_results, msg);
+      if (!diagnostic_manager.Diagnostics().size())
+        error = Status::FromExpressionError(
+            execution_results,
+            "expression failed to parse (no further compiler diagnostics)");
+      else
+        error = diagnostic_manager.GetAsStatus(execution_results);
----------------
medismailben wrote:

Assuming `AddDiagnostic` never fails to append a new fixit diagnostic, we will find ourselves in this `else` branch. However it looks like `DiagnosticManager::GetAsStatus` will always create a status object with a `lldb::eErrorTypeExpression` error type.

I'm wondering if fixits should actually be reported as errors ...

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