[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

This revision was automatically updated to reflect the committed changes. Closed by commit rG83bd2c4a0680: Prevent GetNumChildren from transitively walking pointer chains (authored by jarin, committed by teemperor). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin added a comment. Raphael, could you land this for me? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

clayborg accepted this revision. clayborg added a comment. Major changes not required. Thanks for finding and fixing this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

teemperor accepted this revision. teemperor added a comment. I think the refactoring is a good idea, but IMHO this patch can go in as-is. It's not adding any more debt to the existing approach (it even deletes code) and it adds tests, so I don't see a drawback of merging this. Also having this

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin added a comment. In D80254#2047982 , @clayborg wrote: > This looks good, thanks for subscribing me. We need to have GetNumChildren > and GetChildAtIndex agreeing on things and we definitely shouldn't be walking > more than on pointer recursively.

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

clayborg added a comment. This looks good, thanks for subscribing me. We need to have GetNumChildren and GetChildAtIndex agreeing on things and we definitely shouldn't be walking more than on pointer recursively. My only question is do we need helper functions added to TypeSystemClang to avoid

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

shafik added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:5215 +uint32_t num_pointee_children = 0; +if (pointee_clang_type.IsAggregateType()) + num_pointee_children = jarin wrote: > shafik wrote: > > I am

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin updated this revision to Diff 265237. jarin marked 3 inline comments as done. jarin added a comment. Merged the ObjC pointer case with the reference case, simplified the test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 Files:

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin added a comment. In D80254#2046275 , @labath wrote: > Looks fine to me too. The way this test is phrased, it would probably make > more sense under `test/API/python_api/value`, than here. (I mean, it's not > technically wrong because everything is

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

labath added a comment. Looks fine to me too. The way this test is phrased, it would probably make more sense under `test/API/python_api/value`, than here. (I mean, it's not technically wrong because everything is a "functionality", but i'd like to avoid putting stuff here precisely because

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

teemperor accepted this revision. teemperor added subscribers: labath, clayborg. teemperor added a comment. This revision is now accepted and ready to land. I think this looks good beside some minor nit-picks. Maybe @labath should take a second look too. Comment at:

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin marked an inline comment as not done. jarin added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:5215 +uint32_t num_pointee_children = 0; +if (pointee_clang_type.IsAggregateType()) + num_pointee_children =

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin updated this revision to Diff 265141. jarin marked 2 inline comments as done. jarin added a comment. Added more assertions, reformatted the test code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 Files:

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

shafik added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:5215 +uint32_t num_pointee_children = 0; +if (pointee_clang_type.IsAggregateType()) + num_pointee_children = I am curious what cases are pointers

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin created this revision. jarin added reviewers: teemperor, jingham. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. This is an attempt to fix https://bugs.llvm.org/show_bug.cgi?id=45988, where SBValue::GetNumChildren returns 2, but SBValue::GetChildAtIndex(1) returns an