[Lldb-commits] [lldb] 8bad4ae - [lldb] Change dwim-print to default to disabled persistent results

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-03-15T13:34:19-07:00
New Revision: 8bad4ae679df6fc7dbd016dccbd3da34206e836b

URL: https://github.com/llvm/llvm-project/commit/8bad4ae679df6fc7dbd016dccbd3da34206e836b
DIFF: https://github.com/llvm/llvm-project/commit/8bad4ae679df6fc7dbd016dccbd3da34206e836b.diff

LOG: [lldb] Change dwim-print to default to disabled persistent results

Change `dwim-print` to now disable persistent results by default, unless requested by
the user with the `--persistent-result` flag.

Ex:

```
(lldb) dwim-print 1 + 1
(int) 2
(lldb) dwim-print --persistent-result on -- 1 + 1
(int) $0 = 2
```

Users who wish to enable persistent results can make and use an alias that includes
`--persistent-result on`.

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

Added: 
    

Modified: 
    lldb/source/Commands/CommandObjectDWIMPrint.cpp
    lldb/source/Commands/CommandObjectExpression.cpp
    lldb/source/Commands/CommandObjectExpression.h
    lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index d8bc7a1e89696..419a27acc8181 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -71,6 +71,10 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
     return false;
   }
 
+  // If the user has not specified, default to disabling persistent results.
+  if (m_expr_options.suppress_persistent_result == eLazyBoolCalculate)
+    m_expr_options.suppress_persistent_result = eLazyBoolYes;
+
   auto verbosity = GetDebugger().GetDWIMPrintVerbosity();
 
   Target *target_ptr = m_exe_ctx.GetTargetPtr();

diff  --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 63b92363369df..2658677085a24 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -151,7 +151,7 @@ Status CommandObjectExpression::CommandOptions::SetOptionValue(
     bool persist_result =
         OptionArgParser::ToBoolean(option_arg, true, &success);
     if (success)
-      suppress_persistent_result = !persist_result;
+      suppress_persistent_result = !persist_result ? eLazyBoolYes : eLazyBoolNo;
     else
       error.SetErrorStringWithFormat(
           "could not convert \"%s\" to a boolean value.",
@@ -187,7 +187,7 @@ void CommandObjectExpression::CommandOptions::OptionParsingStarting(
   auto_apply_fixits = eLazyBoolCalculate;
   top_level = false;
   allow_jit = true;
-  suppress_persistent_result = false;
+  suppress_persistent_result = eLazyBoolCalculate;
 }
 
 llvm::ArrayRef<OptionDefinition>
@@ -202,8 +202,9 @@ CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions(
   options.SetCoerceToId(display_opts.use_objc);
   // Explicitly disabling persistent results takes precedence over the
   // m_verbosity/use_objc logic.
-  if (suppress_persistent_result)
-    options.SetSuppressPersistentResult(true);
+  if (suppress_persistent_result != eLazyBoolCalculate)
+    options.SetSuppressPersistentResult(suppress_persistent_result ==
+                                        eLazyBoolYes);
   else if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
     options.SetSuppressPersistentResult(display_opts.use_objc);
   options.SetUnwindOnError(unwind_on_error);

diff  --git a/lldb/source/Commands/CommandObjectExpression.h b/lldb/source/Commands/CommandObjectExpression.h
index e381a4a5aaf92..d6a4bb19fd650 100644
--- a/lldb/source/Commands/CommandObjectExpression.h
+++ b/lldb/source/Commands/CommandObjectExpression.h
@@ -53,7 +53,7 @@ class CommandObjectExpression : public CommandObjectRaw,
     lldb::LanguageType language;
     LanguageRuntimeDescriptionDisplayVerbosity m_verbosity;
     LazyBool auto_apply_fixits;
-    bool suppress_persistent_result;
+    LazyBool suppress_persistent_result;
   };
 
   CommandObjectExpression(CommandInterpreter &interpreter);

diff  --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 705e2ef79ddeb..22d18f91d0a59 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,18 +16,16 @@ def _run_cmd(self, cmd: str) -> str:
         self.ci.HandleCommand(cmd, result)
         return result.GetOutput().rstrip()
 
-    VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
-    VAR_IDENT = re.compile(VAR_IDENT_RAW)
+    VAR_IDENT = re.compile(r"(?:\$\d+|\w+) = ")
 
-    def _mask_persistent_var(self, string: str) -> str:
+    def _strip_result_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.
+        Strip (persistent) result variables (ex '$0 = ', or 'someVar = ', etc).
+
+        This allows for using the output of `expression`/`frame variable`, to
+        compare it to `dwim-print` output, which disables result variables.
         """
-        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)
+        return self.VAR_IDENT.subn("", string, 1)[0]
 
     def _expect_cmd(
         self,
@@ -46,19 +44,16 @@ def _expect_cmd(
         if actual_cmd == "frame variable":
             resolved_cmd = resolved_cmd.replace(" -- ", " ", 1)
 
-        expected_output = self._run_cmd(resolved_cmd)
+        resolved_cmd_output = self._run_cmd(resolved_cmd)
+        dwim_cmd_output = self._strip_result_var(resolved_cmd_output)
 
         # Verify dwim-print chose the expected command.
         self.runCmd("settings set dwim-print-verbosity full")
-        substrs = [f"note: ran `{resolved_cmd}`"]
-        patterns = []
-
-        if self.VAR_IDENT.search(expected_output):
-            patterns.append(self._mask_persistent_var(expected_output))
-        else:
-            substrs.append(expected_output)
 
-        self.expect(dwim_cmd, substrs=substrs, patterns=patterns)
+        self.expect(dwim_cmd, substrs=[
+            f"note: ran `{resolved_cmd}`",
+            dwim_cmd_output,
+        ])
 
     def test_variables(self):
         """Test dwim-print with variables."""