[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)
Dave Lee via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 15 15:00:49 PDT 2024
https://github.com/kastiglione updated https://github.com/llvm/llvm-project/pull/85152
>From 970cf82fa3d64c5a4e1b3929c110b42974ef13cd Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Wed, 13 Mar 2024 14:49:23 -0700
Subject: [PATCH 1/3] [lldb] Fix dwim-print to not delete non-result persistent
variables
---
lldb/source/Commands/CommandObjectDWIMPrint.cpp | 12 ++++++++++--
lldb/test/API/commands/dwim-print/TestDWIMPrint.py | 12 ++++++++++++
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index b183cb423111fb..5c043dfd101be6 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -170,6 +170,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
ExpressionResults expr_result = target.EvaluateExpression(
expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
+ auto persistent_name = valobj_sp->GetName();
+ // EvaluateExpression doesn't generate a new persistent result (`$0`) when
+ // the expression is already just a persistent variable (`$var`). Instead,
+ // the same persistent variable is reused. Take note of when a persistent
+ // result is created, to prevent unintentional deletion of a user's
+ // persistent variable.
+ bool did_persist_result = persistent_name != expr;
+
// Only mention Fix-Its if the expression evaluator applied them.
// Compiler errors refer to the final expression after applying Fix-It(s).
if (!fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
@@ -199,9 +207,9 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
}
}
- if (suppress_result)
+ if (did_persist_result && suppress_result)
if (auto result_var_sp =
- target.GetPersistentVariable(valobj_sp->GetName())) {
+ target.GetPersistentVariable(persistent_name)) {
auto language = valobj_sp->GetPreferredDisplayLanguage();
if (auto *persistent_state =
target.GetPersistentExpressionStateForLanguage(language))
diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 040632096c70e7..c650b1e3533e08 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -146,3 +146,15 @@ def test_void_result(self):
self, "// break here", lldb.SBFileSpec("main.c")
)
self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"])
+
+ def test_preserves_persistent_variables(self):
+ """Test dwim-print does not delete persistent variables."""
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "// break here", lldb.SBFileSpec("main.c")
+ )
+ self.expect("dwim-print int $i = 15")
+ # Run the same expression twice and verify success. This ensures the
+ # first command does not delete the persistent variable.
+ for _ in range(2):
+ self.expect("dwim-print $i", startstr="(int) 15")
>From 6503e22c894d9ed3f0d94d30a2165cf7f1914e47 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Fri, 15 Mar 2024 11:02:24 -0700
Subject: [PATCH 2/3] Try expr as a persistent variable
---
.../Commands/CommandObjectDWIMPrint.cpp | 24 +++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 5c043dfd101be6..9db0c5a1e73ea1 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -23,7 +23,6 @@
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/FormatVariadic.h"
#include <regex>
@@ -161,7 +160,16 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
}
}
- // Second, also lastly, try `expr` as a source expression to evaluate.
+ // Second, try `expr` as a persistent variable.
+ if (expr.starts_with("$"))
+ if (auto var_sp = target.GetPersistentVariable(ConstString(expr)))
+ if (auto valobj_sp = var_sp->GetValueObject()) {
+ valobj_sp->Dump(result.GetOutputStream(), dump_options);
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ return;
+ }
+
+ // Third, and lastly, try `expr` as a source expression to evaluate.
{
auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
ValueObjectSP valobj_sp;
@@ -170,14 +178,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
ExpressionResults expr_result = target.EvaluateExpression(
expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
- auto persistent_name = valobj_sp->GetName();
- // EvaluateExpression doesn't generate a new persistent result (`$0`) when
- // the expression is already just a persistent variable (`$var`). Instead,
- // the same persistent variable is reused. Take note of when a persistent
- // result is created, to prevent unintentional deletion of a user's
- // persistent variable.
- bool did_persist_result = persistent_name != expr;
-
// Only mention Fix-Its if the expression evaluator applied them.
// Compiler errors refer to the final expression after applying Fix-It(s).
if (!fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
@@ -207,9 +207,9 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
}
}
- if (did_persist_result && suppress_result)
+ if (suppress_result)
if (auto result_var_sp =
- target.GetPersistentVariable(persistent_name)) {
+ target.GetPersistentVariable(valobj_sp->GetName())) {
auto language = valobj_sp->GetPreferredDisplayLanguage();
if (auto *persistent_state =
target.GetPersistentExpressionStateForLanguage(language))
>From 9a19a7f9ce8b993e60a369a482458f8ca6d665ea Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Fri, 15 Mar 2024 15:00:25 -0700
Subject: [PATCH 3/3] Scope persistent var lookup to expected language
---
lldb/source/Commands/CommandObjectDWIMPrint.cpp | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 9db0c5a1e73ea1..a88da986bb384f 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -162,12 +162,13 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Second, try `expr` as a persistent variable.
if (expr.starts_with("$"))
- if (auto var_sp = target.GetPersistentVariable(ConstString(expr)))
- if (auto valobj_sp = var_sp->GetValueObject()) {
- valobj_sp->Dump(result.GetOutputStream(), dump_options);
- result.SetStatus(eReturnStatusSuccessFinishResult);
- return;
- }
+ if (auto *state = target.GetPersistentExpressionStateForLanguage(language))
+ if (auto var_sp = state->GetVariable(expr))
+ if (auto valobj_sp = var_sp->GetValueObject()) {
+ valobj_sp->Dump(result.GetOutputStream(), dump_options);
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ return;
+ }
// Third, and lastly, try `expr` as a source expression to evaluate.
{
More information about the lldb-commits
mailing list