[Lldb-commits] [lldb] 63c77bf - [lldb] Make persisting result variables configurable

Dave Lee via lldb-commits lldb-commits at lists.llvm.org
Fri Feb 17 17:50:53 PST 2023


Author: Dave Lee
Date: 2023-02-17T17:50:43-08:00
New Revision: 63c77bf71d80b24df377fc45c80bfa1904ee849e

URL: https://github.com/llvm/llvm-project/commit/63c77bf71d80b24df377fc45c80bfa1904ee849e
DIFF: https://github.com/llvm/llvm-project/commit/63c77bf71d80b24df377fc45c80bfa1904ee849e.diff

LOG: [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

Added: 
    lldb/test/API/commands/expression/persistent_result/Makefile
    lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
    lldb/test/API/commands/expression/persistent_result/main.c

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 28c78af73dcec..a348f416af22f 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -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;

diff  --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index f576729b76d97..63b92363369df 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/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());
 

diff  --git a/lldb/source/Commands/CommandObjectExpression.h b/lldb/source/Commands/CommandObjectExpression.h
index bcd76f8512b18..e381a4a5aaf92 100644
--- a/lldb/source/Commands/CommandObjectExpression.h
+++ b/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);

diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 6aee2ac228049..f11c95e5660e2 100644
--- a/lldb/source/Commands/Options.td
+++ b/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 {

diff  --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 6fd6308ded5d9..c4221a5ac7cc0 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/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;
         }
       }

diff  --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 89d40097070b0..705e2ef79ddeb 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,8 @@ 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:
         """
@@ -24,8 +25,9 @@ def _mask_persistent_var(self, string: str) -> str:
         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)

diff  --git a/lldb/test/API/commands/expression/persistent_result/Makefile b/lldb/test/API/commands/expression/persistent_result/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/commands/expression/persistent_result/Makefile
@@ -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
index 0000000000000..10eb100bac37b
--- /dev/null
+++ b/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"])

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
index 0000000000000..490e78f7f6195
--- /dev/null
+++ b/lldb/test/API/commands/expression/persistent_result/main.c
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}


        


More information about the lldb-commits mailing list