[lldb] Make persisting result variables configurable

Context: The `expression` command uses artificial variables to store the expression
result. This result variable is unconditionally kept around after the expression command
has completed. These variables are known as persistent results. These are the variables
`$0`, `$1`, etc, that are displayed when running `p` or `expression`.

This change allows users to control whether result variables are persisted, by
introducing a `--persistent-result` flag.

This change keeps the current default behavior, persistent results are created by
default. This change gives users the ability to opt-out by re-aliasing `p`. For example:

```
command unalias p
command alias p expression --persistent-result false --
```

For consistency, this flag is also adopted by `dwim-print`. Of note, if asked,
`dwim-print` will create a persistent result even for frame variables.

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

Author: kastiglione

lldb/source/Commands/CommandObjectDWIMPrint.cpp

@@ -62,20 +62,32 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 
   auto verbosity = GetDebugger().GetDWIMPrintVerbosity();
 
+  Target *target_ptr = m_exe_ctx.GetTargetPtr();
+  // Fallback to the dummy target, which can allow for expression evaluation.
+  Target &target = target_ptr ? *target_ptr : GetDummyTarget();
+
+  const EvaluateExpressionOptions eval_options =
+      m_expr_options.GetEvaluateExpressionOptions(target, m_varobj_options);
+
   DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
       m_expr_options.m_verbosity, m_format_options.GetFormat());
+  dump_options.SetHideName(eval_options.GetSuppressPersistentResult());
 
   // First, try `expr` as the name of a frame variable.
   if (StackFrame *frame = m_exe_ctx.GetFramePtr()) {
     auto valobj_sp = frame->FindVariable(ConstString(expr));
     if (valobj_sp && valobj_sp->GetError().Success()) {
+      if (!eval_options.GetSuppressPersistentResult())
+        valobj_sp = valobj_sp->Persist();
+
       if (verbosity == eDWIMPrintVerbosityFull) {
         StringRef flags;
         if (args.HasArgs())
           flags = args.GetArgString();
         result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
                                         flags, expr);
       }
+
       valobj_sp->Dump(result.GetOutputStream(), dump_options);
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
@@ -84,13 +96,7 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 
   // Second, also lastly, try `expr` as a source expression to evaluate.
   {
-    Target *target_ptr = m_exe_ctx.GetTargetPtr();
-    // Fallback to the dummy target, which can allow for expression evaluation.
-    Target &target = target_ptr ? *target_ptr : GetDummyTarget();
-
     auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
-    const EvaluateExpressionOptions eval_options =
-        m_expr_options.GetEvaluateExpressionOptions(target, m_varobj_options);
     ValueObjectSP valobj_sp;
     ExpressionResults expr_result =
         target.EvaluateExpression(expr, exe_scope, valobj_sp, eval_options);
@@ -102,6 +108,7 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
         result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
                                         expr);
       }
+
       valobj_sp->Dump(result.GetOutputStream(), dump_options);
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;

lldb/source/Commands/CommandObjectExpression.cpp

@@ -146,6 +146,19 @@ Status CommandObjectExpression::CommandOptions::SetOptionValue(
     break;
   }
 
+  case '\x01': {
+    bool success;
+    bool persist_result =
+        OptionArgParser::ToBoolean(option_arg, true, &success);
+    if (success)
+      suppress_persistent_result = !persist_result;
+    else
+      error.SetErrorStringWithFormat(
+          "could not convert \"%s\" to a boolean value.",
+          option_arg.str().c_str());
+    break;
+  }
+
   default:
     llvm_unreachable("Unimplemented option");
   }
@@ -174,6 +187,7 @@ void CommandObjectExpression::CommandOptions::OptionParsingStarting(
   auto_apply_fixits = eLazyBoolCalculate;
   top_level = false;
   allow_jit = true;
+  suppress_persistent_result = false;
 }
 
 llvm::ArrayRef<OptionDefinition>
@@ -186,7 +200,11 @@ CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions(
     const Target &target, const OptionGroupValueObjectDisplay &display_opts) {
   EvaluateExpressionOptions options;
   options.SetCoerceToId(display_opts.use_objc);
-  if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
+  // Explicitly disabling persistent results takes precedence over the
+  // m_verbosity/use_objc logic.
+  if (suppress_persistent_result)
+    options.SetSuppressPersistentResult(true);
+  else if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
     options.SetSuppressPersistentResult(display_opts.use_objc);
   options.SetUnwindOnError(unwind_on_error);
   options.SetIgnoreBreakpoints(ignore_breakpoints);
@@ -405,10 +423,10 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
     return false;
   }
 
-  const EvaluateExpressionOptions options =
+  const EvaluateExpressionOptions eval_options =
       m_command_options.GetEvaluateExpressionOptions(target, m_varobj_options);
   ExpressionResults success = target.EvaluateExpression(
-      expr, frame, result_valobj_sp, options, &m_fixed_expression);
+      expr, frame, result_valobj_sp, eval_options, &m_fixed_expression);
 
   // We only tell you about the FixIt if we applied it.  The compiler errors
   // will suggest the FixIt if it parsed.
@@ -437,6 +455,7 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
 
         DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
             m_command_options.m_verbosity, format));
+        options.SetHideName(eval_options.GetSuppressPersistentResult());
         options.SetVariableFormatDisplayLanguage(
             result_valobj_sp->GetPreferredDisplayLanguage());
 

lldb/source/Commands/CommandObjectExpression.h

@@ -53,6 +53,7 @@ class CommandObjectExpression : public CommandObjectRaw,
     lldb::LanguageType language;
     LanguageRuntimeDescriptionDisplayVerbosity m_verbosity;
     LazyBool auto_apply_fixits;
+    bool suppress_persistent_result;
   };
 
   CommandObjectExpression(CommandInterpreter &interpreter);

lldb/source/Commands/Options.td

@@ -386,6 +386,11 @@ let Command = "expression" in {
     Arg<"Boolean">,
     Desc<"Controls whether the expression can fall back to being JITted if it's "
     "not supported by the interpreter (defaults to true).">;
+  def persistent_result : Option<"persistent-result", "\\x01">, Groups<[1,2]>,
+    Arg<"Boolean">,
+    Desc<"Persist expression result in a variable for subsequent use. "
+    "Expression results will be labeled with $-prefixed variables, e.g. $0, "
+    "$1, etc. Defaults to true.">;
 }
 
 let Command = "frame diag" in {

lldb/source/DataFormatters/ValueObjectPrinter.cpp

@@ -425,7 +425,9 @@ bool ValueObjectPrinter::PrintValueAndSummaryIfNeeded(bool &value_printed,
         if (m_options.m_hide_pointer_value &&
             IsPointerValue(m_valobj->GetCompilerType())) {
         } else {
-          m_stream->Printf(" %s", m_value.c_str());
+          if (!m_options.m_hide_name)
+            m_stream->PutChar(' ');
+          m_stream->PutCString(m_value);
           value_printed = true;
         }
       }

lldb/test/API/commands/dwim-print/TestDWIMPrint.py

@@ -16,16 +16,18 @@ def _run_cmd(self, cmd: str) -> str:
         self.ci.HandleCommand(cmd, result)
         return result.GetOutput().rstrip()
 
-    PERSISTENT_VAR = re.compile(r"\$\d+")
+    VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
+    VAR_IDENT = re.compile(VAR_IDENT_RAW)
 
     def _mask_persistent_var(self, string: str) -> str:
         """
         Replace persistent result variables (ex '$0', '$1', etc) with a regex
         that matches any persistent result (r'\$\d+'). The returned string can
         be matched against other `expression` results.
         """
-        before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
-        return re.escape(before) + r"\$\d+" + re.escape(after)
+        before, after = self.VAR_IDENT.split(string, maxsplit=1)
+        # Support either a frame variable (\w+) or a persistent result (\$\d+).
+        return re.escape(before) + self.VAR_IDENT_RAW + re.escape(after)
 
     def _expect_cmd(
         self,
@@ -51,7 +53,7 @@ def _expect_cmd(
         substrs = [f"note: ran `{resolved_cmd}`"]
         patterns = []
 
-        if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+        if self.VAR_IDENT.search(expected_output):
             patterns.append(self._mask_persistent_var(expected_output))
         else:
             substrs.append(expected_output)

lldb/test/API/commands/expression/persistent_result/Makefile

@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py

@@ -0,0 +1,37 @@
+"""
+Test controlling `expression` result variables are persistent.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+    def setUp(self):
+        TestBase.setUp(self)
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+
+    def test_enable_persistent_result(self):
+        """Test explicitly enabling result variables persistence."""
+        self.expect("expression --persistent-result on -- i", substrs=["(int) $0 = 30"])
+        # Verify the lifetime of $0 extends beyond the `expression` it was created in.
+        self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+    def test_disable_persistent_result(self):
+        """Test explicitly disabling persistent result variables."""
+        self.expect("expression --persistent-result off -- i", substrs=["(int) 30"])
+        # Verify a persistent result was not silently created.
+        self.expect("expression $0", error=True)
+
+    def test_expression_persists_result(self):
+        """Test `expression`'s default behavior is to persist a result variable."""
+        self.expect("expression i", substrs=["(int) $0 = 30"])
+        self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+    def test_p_persists_result(self):
+        """Test `p` does persist a result variable."""
+        self.expect("p i", substrs=["(int) $0 = 30"])
+        self.expect("p $0", substrs=["(int) $0 = 30"])

lldb/test/API/commands/expression/persistent_result/main.c

@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}