[lldb] Make persisting result variables configurable
authorDave Lee <davelee.com@gmail.com>
Thu, 16 Feb 2023 23:39:09 +0000 (15:39 -0800)
committerDave Lee <davelee.com@gmail.com>
Sat, 18 Feb 2023 01:50:43 +0000 (17:50 -0800)
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

lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectExpression.h
lldb/source/Commands/Options.td
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/test/API/commands/dwim-print/TestDWIMPrint.py
lldb/test/API/commands/expression/persistent_result/Makefile [new file with mode: 0644]
lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py [new file with mode: 0644]
lldb/test/API/commands/expression/persistent_result/main.c [new file with mode: 0644]

index 28c78af..a348f41 100644 (file)
@@ -62,13 +62,24 @@ 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())
@@ -76,6 +87,7 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
         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;
index f576729..63b9236 100644 (file)
@@ -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());
 
index bcd76f8..e381a4a 100644 (file)
@@ -53,6 +53,7 @@ public:
     lldb::LanguageType language;
     LanguageRuntimeDescriptionDisplayVerbosity m_verbosity;
     LazyBool auto_apply_fixits;
+    bool suppress_persistent_result;
   };
 
   CommandObjectExpression(CommandInterpreter &interpreter);
index 6aee2ac..f11c95e 100644 (file)
@@ -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 {
index 6fd6308..c4221a5 100644 (file)
@@ -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;
         }
       }
index 89d4009..705e2ef 100644 (file)
@@ -16,7 +16,8 @@ class TestCase(TestBase):
         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:
         """
@@ -24,8 +25,9 @@ class TestCase(TestBase):
         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 @@ class TestCase(TestBase):
         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)
diff --git a/lldb/test/API/commands/expression/persistent_result/Makefile b/lldb/test/API/commands/expression/persistent_result/Makefile
new file mode 100644 (file)
index 0000000..1049594
--- /dev/null
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py b/lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
new file mode 100644 (file)
index 0000000..10eb100
--- /dev/null
@@ -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"])
diff --git a/lldb/test/API/commands/expression/persistent_result/main.c b/lldb/test/API/commands/expression/persistent_result/main.c
new file mode 100644 (file)
index 0000000..490e78f
--- /dev/null
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}