[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/85669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/85669

>From baa85a25548546679bf2b54fb723b51a2fb50e82 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 18 Mar 2024 10:14:49 -0700
Subject: [PATCH 1/2] [lldb][nfc] Factor out repeated code in DWIM Print

The code that prints ValueObjects is duplicated across two different cases of
the dwim-print command, and a subsequent commit will add a third case. As such,
this commit factors out the common code into a lambda. A free function was
considered, but there is too much function-local context required in that.

We also reword some of the comments so that they stop counting cases, making it
easier to add other cases later.
---
 .../Commands/CommandObjectDWIMPrint.cpp   | 36 +--
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index a88da986bb384f..933e5af3b69ff7 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -129,6 +129,19 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   };
 
+  // Dump `valobj` according to whether `po` was requested or not.
+  auto dump_val_object = [&](ValueObject ) {
+if (is_po) {
+  StreamString temp_result_stream;
+  valobj.Dump(temp_result_stream, dump_options);
+  llvm::StringRef output = temp_result_stream.GetString();
+  maybe_add_hint(output);
+  result.GetOutputStream() << output;
+} else {
+  valobj.Dump(result.GetOutputStream(), dump_options);
+}
+  };
+
   // First, try `expr` as the name of a frame variable.
   if (frame) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
@@ -146,15 +159,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 flags, expr);
   }
 
-  if (is_po) {
-StreamString temp_result_stream;
-valobj_sp->Dump(temp_result_stream, dump_options);
-llvm::StringRef output = temp_result_stream.GetString();
-maybe_add_hint(output);
-result.GetOutputStream() << output;
-  } else {
-valobj_sp->Dump(result.GetOutputStream(), dump_options);
-  }
+  dump_val_object(*valobj_sp);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return;
 }
@@ -196,17 +201,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 expr);
   }
 
-  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult) {
-if (is_po) {
-  StreamString temp_result_stream;
-  valobj_sp->Dump(temp_result_stream, dump_options);
-  llvm::StringRef output = temp_result_stream.GetString();
-  maybe_add_hint(output);
-  result.GetOutputStream() << output;
-} else {
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
-}
-  }
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+dump_val_object(*valobj_sp);
 
   if (suppress_result)
 if (auto result_var_sp =

>From d6d212093f58324c1778b73360456738944c2773 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 18 Mar 2024 14:08:04 -0700
Subject: [PATCH 2/2] fixup! also use new lambda on persisent variable case

---
 lldb/source/Commands/CommandObjectDWIMPrint.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 933e5af3b69ff7..e1255f37d9bc58 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -170,7 +170,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 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);
+  dump_val_object(*valobj_sp);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return;
 }

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Dave Lee via lldb-commits 

https://github.com/kastiglione approved this pull request.

one request, and then looks good, thanks.

https://github.com/llvm/llvm-project/pull/85669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Dave Lee via lldb-commits 

https://github.com/kastiglione edited 
https://github.com/llvm/llvm-project/pull/85669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Dave Lee via lldb-commits 


@@ -129,6 +129,19 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   };
 
+  // Dump `valobj` according to whether `po` was requested or not.
+  auto dump_val_object = [&](ValueObject ) {

kastiglione wrote:

Would you mind also applying this function to the block that handles persistent 
variables (I just added it in #85152). Thanks.

https://github.com/llvm/llvm-project/pull/85669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

felipepiovezan wrote:

Rebased, address review comments

https://github.com/llvm/llvm-project/pull/85669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/85669

>From baa85a25548546679bf2b54fb723b51a2fb50e82 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 18 Mar 2024 10:14:49 -0700
Subject: [PATCH] [lldb][nfc] Factor out repeated code in DWIM Print

The code that prints ValueObjects is duplicated across two different cases of
the dwim-print command, and a subsequent commit will add a third case. As such,
this commit factors out the common code into a lambda. A free function was
considered, but there is too much function-local context required in that.

We also reword some of the comments so that they stop counting cases, making it
easier to add other cases later.
---
 .../Commands/CommandObjectDWIMPrint.cpp   | 36 +--
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index a88da986bb384f..933e5af3b69ff7 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -129,6 +129,19 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   };
 
+  // Dump `valobj` according to whether `po` was requested or not.
+  auto dump_val_object = [&](ValueObject ) {
+if (is_po) {
+  StreamString temp_result_stream;
+  valobj.Dump(temp_result_stream, dump_options);
+  llvm::StringRef output = temp_result_stream.GetString();
+  maybe_add_hint(output);
+  result.GetOutputStream() << output;
+} else {
+  valobj.Dump(result.GetOutputStream(), dump_options);
+}
+  };
+
   // First, try `expr` as the name of a frame variable.
   if (frame) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
@@ -146,15 +159,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 flags, expr);
   }
 
-  if (is_po) {
-StreamString temp_result_stream;
-valobj_sp->Dump(temp_result_stream, dump_options);
-llvm::StringRef output = temp_result_stream.GetString();
-maybe_add_hint(output);
-result.GetOutputStream() << output;
-  } else {
-valobj_sp->Dump(result.GetOutputStream(), dump_options);
-  }
+  dump_val_object(*valobj_sp);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return;
 }
@@ -196,17 +201,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 expr);
   }
 
-  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult) {
-if (is_po) {
-  StreamString temp_result_stream;
-  valobj_sp->Dump(temp_result_stream, dump_options);
-  llvm::StringRef output = temp_result_stream.GetString();
-  maybe_add_hint(output);
-  result.GetOutputStream() << output;
-} else {
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
-}
-  }
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+dump_val_object(*valobj_sp);
 
   if (suppress_result)
 if (auto result_var_sp =

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/85669

>From ddc9c22765623f88051ce1f2407603a9065c7cd1 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 18 Mar 2024 10:14:49 -0700
Subject: [PATCH] [lldb][nfc] Factor out repeated code in DWIM Print

The code that prints ValueObjects is duplicated across two different cases of
the dwim-print command, and a subsequent commit will add a third case. As such,
this commit factors out the common code into a lambda. A free function was
considered, but there is too much function-local context required in that.

We also reword some of the comments so that they stop counting cases, making it
easier to add other cases later.
---
 .../Commands/CommandObjectDWIMPrint.cpp   | 37 +--
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index a88da986bb384f..3ed54a0f43615d 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -129,6 +129,19 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   };
 
+  // Dump `valobj` according to whether `po` was requested or not.
+  auto dump_val_object = [&](ValueObject ) {
+if (is_po) {
+  StreamString temp_result_stream;
+  valobj.Dump(temp_result_stream, dump_options);
+  llvm::StringRef output = temp_result_stream.GetString();
+  maybe_add_hint(output);
+  result.GetOutputStream() << output;
+} else {
+  valobj.Dump(result.GetOutputStream(), dump_options);
+}
+  };
+
   // First, try `expr` as the name of a frame variable.
   if (frame) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
@@ -146,15 +159,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 flags, expr);
   }
 
-  if (is_po) {
-StreamString temp_result_stream;
-valobj_sp->Dump(temp_result_stream, dump_options);
-llvm::StringRef output = temp_result_stream.GetString();
-maybe_add_hint(output);
-result.GetOutputStream() << output;
-  } else {
-valobj_sp->Dump(result.GetOutputStream(), dump_options);
-  }
+  dump_val_object(*valobj_sp);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return;
 }
@@ -170,6 +175,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
   return;
 }
 
+
   // Third, and lastly, try `expr` as a source expression to evaluate.
   {
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
@@ -196,17 +202,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 expr);
   }
 
-  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult) {
-if (is_po) {
-  StreamString temp_result_stream;
-  valobj_sp->Dump(temp_result_stream, dump_options);
-  llvm::StringRef output = temp_result_stream.GetString();
-  maybe_add_hint(output);
-  result.GetOutputStream() << output;
-} else {
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
-}
-  }
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+dump_val_object(*valobj_sp);
 
   if (suppress_result)
 if (auto result_var_sp =

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Dave Lee via lldb-commits 


@@ -130,7 +130,20 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   };
 
-  // First, try `expr` as the name of a frame variable.

kastiglione wrote:

1. I'd prefer to leave the "first", "second", "third" comments. I want to be 
explicit that there's an order of attempts.
2. You'll need to rebase, I made a change last week that introduce some 
comments and code that you may need to incorporate.

https://github.com/llvm/llvm-project/pull/85669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)


Changes

The code that prints ValueObjects is duplicated across two different cases of 
the dwim-print command, and a subsequent commit will add a third case. As such, 
this commit factors out the common code into a lambda. A free function was 
considered, but there is too much function-local context required in that.

We also reword some of the comments so that they stop counting cases, making it 
easier to add other cases later.

---
Full diff: https://github.com/llvm/llvm-project/pull/85669.diff


1 Files Affected:

- (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+18-22) 


``diff
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index b183cb423111fb..300647a98b6b95 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -130,7 +130,20 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   };
 
-  // First, try `expr` as the name of a frame variable.
+  // Dump `valobj` according to whether `po` was requested or not.
+  auto dump_val_object = [&](ValueObject ) {
+if (is_po) {
+  StreamString temp_result_stream;
+  valobj.Dump(temp_result_stream, dump_options);
+  llvm::StringRef output = temp_result_stream.GetString();
+  maybe_add_hint(output);
+  result.GetOutputStream() << output;
+} else {
+  valobj.Dump(result.GetOutputStream(), dump_options);
+}
+  };
+
+  // Try `expr` as the name of a frame variable.
   if (frame) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
 if (valobj_sp && valobj_sp->GetError().Success()) {
@@ -147,21 +160,13 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 flags, expr);
   }
 
-  if (is_po) {
-StreamString temp_result_stream;
-valobj_sp->Dump(temp_result_stream, dump_options);
-llvm::StringRef output = temp_result_stream.GetString();
-maybe_add_hint(output);
-result.GetOutputStream() << output;
-  } else {
-valobj_sp->Dump(result.GetOutputStream(), dump_options);
-  }
+  dump_val_object(*valobj_sp);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return;
 }
   }
 
-  // Second, also lastly, try `expr` as a source expression to evaluate.
+  // Try `expr` as a source expression to evaluate.
   {
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
 ValueObjectSP valobj_sp;
@@ -187,17 +192,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 expr);
   }
 
-  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult) {
-if (is_po) {
-  StreamString temp_result_stream;
-  valobj_sp->Dump(temp_result_stream, dump_options);
-  llvm::StringRef output = temp_result_stream.GetString();
-  maybe_add_hint(output);
-  result.GetOutputStream() << output;
-} else {
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
-}
-  }
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+dump_val_object(*valobj_sp);
 
   if (suppress_result)
 if (auto result_var_sp =

``




https://github.com/llvm/llvm-project/pull/85669
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Factor out repeated code in DWIM Print (PR #85669)

2024-03-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/85669

The code that prints ValueObjects is duplicated across two different cases of 
the dwim-print command, and a subsequent commit will add a third case. As such, 
this commit factors out the common code into a lambda. A free function was 
considered, but there is too much function-local context required in that.

We also reword some of the comments so that they stop counting cases, making it 
easier to add other cases later.

>From 0480765c0c1feef6c93e6ba90f8367d30fec33bf Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 18 Mar 2024 10:14:49 -0700
Subject: [PATCH] [lldb][nfc] Factor out repeated code in DWIM Print

The code that prints ValueObjects is duplicated across two different cases of
the dwim-print command, and a subsequent commit will add a third case. As such,
this commit factors out the common code into a lambda. A free function was
considered, but there is too much function-local context required in that.

We also reword some of the comments so that they stop counting cases, making it
easier to add other cases later.
---
 .../Commands/CommandObjectDWIMPrint.cpp   | 40 +--
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index b183cb423111fb..300647a98b6b95 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -130,7 +130,20 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   };
 
-  // First, try `expr` as the name of a frame variable.
+  // Dump `valobj` according to whether `po` was requested or not.
+  auto dump_val_object = [&](ValueObject ) {
+if (is_po) {
+  StreamString temp_result_stream;
+  valobj.Dump(temp_result_stream, dump_options);
+  llvm::StringRef output = temp_result_stream.GetString();
+  maybe_add_hint(output);
+  result.GetOutputStream() << output;
+} else {
+  valobj.Dump(result.GetOutputStream(), dump_options);
+}
+  };
+
+  // Try `expr` as the name of a frame variable.
   if (frame) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
 if (valobj_sp && valobj_sp->GetError().Success()) {
@@ -147,21 +160,13 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 flags, expr);
   }
 
-  if (is_po) {
-StreamString temp_result_stream;
-valobj_sp->Dump(temp_result_stream, dump_options);
-llvm::StringRef output = temp_result_stream.GetString();
-maybe_add_hint(output);
-result.GetOutputStream() << output;
-  } else {
-valobj_sp->Dump(result.GetOutputStream(), dump_options);
-  }
+  dump_val_object(*valobj_sp);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return;
 }
   }
 
-  // Second, also lastly, try `expr` as a source expression to evaluate.
+  // Try `expr` as a source expression to evaluate.
   {
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
 ValueObjectSP valobj_sp;
@@ -187,17 +192,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 expr);
   }
 
-  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult) {
-if (is_po) {
-  StreamString temp_result_stream;
-  valobj_sp->Dump(temp_result_stream, dump_options);
-  llvm::StringRef output = temp_result_stream.GetString();
-  maybe_add_hint(output);
-  result.GetOutputStream() << output;
-} else {
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
-}
-  }
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+dump_val_object(*valobj_sp);
 
   if (suppress_result)
 if (auto result_var_sp =

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits