[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d4fcd411b3d: [lldb] Default can_create to true in 
GetChildMemberWithName (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -604,7 +604,7 @@
   valobj_sp = GetValueObjectForFrameVariable(variable_sp, use_dynamic);
   if (!valobj_sp)
 return valobj_sp;
-  valobj_sp = valobj_sp->GetChildMemberWithName(name_const_string, true);
+  valobj_sp = valobj_sp->GetChildMemberWithName(name_const_string);
   if (valobj_sp)
 break;
 }
@@ -705,13 +705,13 @@
   return ValueObjectSP();
 }
   }
-  child_valobj_sp = valobj_sp->GetChildMemberWithName(child_name, true);
+  child_valobj_sp = valobj_sp->GetChildMemberWithName(child_name);
   if (!child_valobj_sp) {
 if (!no_synth_child) {
   child_valobj_sp = valobj_sp->GetSyntheticValue();
   if (child_valobj_sp)
 child_valobj_sp =
-child_valobj_sp->GetChildMemberWithName(child_name, true);
+child_valobj_sp->GetChildMemberWithName(child_name);
 }
 
 if (no_synth_child || !child_valobj_sp) {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -514,7 +514,7 @@
 ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
 lldb::ValueObjectSP exception_sp) {
   ValueObjectSP reserved_dict =
-  exception_sp->GetChildMemberWithName("reserved", true);
+  exception_sp->GetChildMemberWithName("reserved");
   if (!reserved_dict)
 return FailExceptionParsing("Failed to get 'reserved' member.");
 
@@ -567,15 +567,15 @@
 
   if (!return_addresses)
 return FailExceptionParsing("Failed to get return addresses.");
-  auto frames_value = return_addresses->GetChildMemberWithName("_frames", true);
+  auto frames_value = return_addresses->GetChildMemberWithName("_frames");
   if (!frames_value)
 return FailExceptionParsing("Failed to get frames_value.");
   addr_t frames_addr = frames_value->GetValueAsUnsigned(0);
-  auto count_value = return_addresses->GetChildMemberWithName("_cnt", true);
+  auto count_value = return_addresses->GetChildMemberWithName("_cnt");
   if (!count_value)
 return FailExceptionParsing("Failed to get count_value.");
   size_t count = count_value->GetValueAsUnsigned(0);
-  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore", true);
+  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore");
   if (!ignore_value)
 return FailExceptionParsing("Failed to get ignore_value.");
   size_t ignore = ignore_value->GetValueAsUnsigned(0);
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -138,11 

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.

I'm alright with this if Jim is. I think we should remove this parameter since 
it's never set to anything other than `true` from what I understand, but that 
can be done in a follow-up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

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

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

It's OK to retain this as the default, and as you say, taking it out would be a 
trivial patch after this work.  The control does allow you to do "Have I 
already made this child" before setting about to make it, though I can't 
currently see anywhere where we could use this information.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

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

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

To expand the conversation, I have also opened D152031 
 which makes the same change to 
`GetChildAtIndex`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

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

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

correction: there's still an overload of `GetChildAtNamePath`, which takes a 
can_create value and passes it through to `GetChildMemberWithName`. However 
that function isn't used, and could be deleted. To delete the parameter, that 
overload of `GetChildMemberWithName` would need to be deleted too. In general, 
I'm still in favor of the conservative choice of giving it a default. Deleting 
it altogether is an easy follow up, if so desired.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

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

Re: [Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Dave Lee via lldb-commits]

That's an option too, which I considered. I went with this as a more
conservative change. If there's agreement to remove it then I can change it.

On Thursday, June 1, 2023, Alex Langford via Phabricator <
revi...@reviews.llvm.org> wrote:

> bulbazord added a comment.
>
> If every caller sets this to true, why not remove the argument altogether?
> It looks like `ValueObjectRegister::GetChildMemberWithName` doesn't use
> the argument, `ValueObject::GetChildMemberWithName` and
> `ValueObjectSynthetic::GetChildMemberWithName` just pass it along to
> `GetChildAtIndex`.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D151966/new/
>
> https://reviews.llvm.org/D151966
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

If every caller sets this to true, why not remove the argument altogether? It 
looks like `ValueObjectRegister::GetChildMemberWithName` doesn't use the 
argument, `ValueObject::GetChildMemberWithName` and 
`ValueObjectSynthetic::GetChildMemberWithName` just pass it along to 
`GetChildAtIndex`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

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

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: jingham, bulbazord.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It turns out all existing callers of `GetChildMemberWithName` pass true for 
`can_create`.
This change makes `true` the default value, callers don't have to pass an 
opaque true.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151966

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -604,7 +604,7 @@
   valobj_sp = GetValueObjectForFrameVariable(variable_sp, use_dynamic);
   if (!valobj_sp)
 return valobj_sp;
-  valobj_sp = valobj_sp->GetChildMemberWithName(name_const_string, true);
+  valobj_sp = valobj_sp->GetChildMemberWithName(name_const_string);
   if (valobj_sp)
 break;
 }
@@ -705,13 +705,13 @@
   return ValueObjectSP();
 }
   }
-  child_valobj_sp = valobj_sp->GetChildMemberWithName(child_name, true);
+  child_valobj_sp = valobj_sp->GetChildMemberWithName(child_name);
   if (!child_valobj_sp) {
 if (!no_synth_child) {
   child_valobj_sp = valobj_sp->GetSyntheticValue();
   if (child_valobj_sp)
 child_valobj_sp =
-child_valobj_sp->GetChildMemberWithName(child_name, true);
+child_valobj_sp->GetChildMemberWithName(child_name);
 }
 
 if (no_synth_child || !child_valobj_sp) {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -514,7 +514,7 @@
 ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
 lldb::ValueObjectSP exception_sp) {
   ValueObjectSP reserved_dict =
-  exception_sp->GetChildMemberWithName("reserved", true);
+  exception_sp->GetChildMemberWithName("reserved");
   if (!reserved_dict)
 return FailExceptionParsing("Failed to get 'reserved' member.");
 
@@ -567,15 +567,15 @@
 
   if (!return_addresses)
 return FailExceptionParsing("Failed to get return addresses.");
-  auto frames_value = return_addresses->GetChildMemberWithName("_frames", true);
+  auto frames_value = return_addresses->GetChildMemberWithName("_frames");
   if (!frames_value)
 return FailExceptionParsing("Failed to get frames_value.");
   addr_t frames_addr = frames_value->GetValueAsUnsigned(0);
-  auto count_value = return_addresses->GetChildMemberWithName("_cnt", true);
+  auto count_value = return_addresses->GetChildMemberWithName("_cnt");
   if (!count_value)
 return FailExceptionParsing("Failed to get count_value.");
   size_t count = count_value->GetValueAsUnsigned(0);
-  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore", true);
+  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore");
   if (!ignore_value)
 return FailExceptionParsing("Failed to get ignore_value.");
   size_t ignore = ignore_value->GetValueAsUnsigned(0);
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
---