[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe2039142f6b1: Some FormatEntity.cpp cleanup and unit testing (authored by nealsid, committed by teemperor). Repository: rG LLVM Github Monorepo C

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor accepted this revision. teemperor added a comment. Thanks for updating and thanks for the patch! I can land it right now. Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154 +TEST(FormatEntity, LookupAllEntriesInTree) { + for (const auto &testString : lookupStri

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 339059. nealsid added a comment. Small update of patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Cor

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked an inline comment as done. nealsid added a comment. Thanks for the review, and apologies for the delay in updating the diffs - seems like I haven't even had an hour to sit down at my dev machine for the past couple weeks, much less actually do any coding :) I don't have commit ac

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked an inline comment as done. nealsid added inline comments. Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154 +TEST(FormatEntity, LookupAllEntriesInTree) { + for (const auto &testString : lookupStrings) { +Entry e; teemperor wrote: > teem

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 337334. nealsid marked an inline comment as done. nealsid added a comment. Update type in for loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/includ

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 337331. nealsid added a comment. Fixed comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/Format

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 337329. nealsid added a comment. CR feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/FormatEnt

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks for the patch (and especially the unit test)! Some two nits left but feel free to fix those when merging. If you don't have commit access I can also do that, just let me kno

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 333507. nealsid added a comment. Thanks for the review - addressed your comments here (I had to rely on Doxygen picking up the type reference automatically because I couldn't get the \see syntax to work correctly, though) Repository: rG LLVM Github Monor

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Sorry for the delay! > It seems like the pre-merge check failures are because I incorrectly set the > project repo when I first created the revision. I noticed in the build >

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid added a comment. In D98153#2610953 , @teemperor wrote: > Thanks for cleaning this up! Are the `if (!sc) ...` stuff are missing nullptr > checks that affect a normal LLDB session (not sure if we can ever have no > SymbolContext) or is this just fo

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 330884. nealsid added a comment. Rebased on HEAD Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/Forma

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 329834. nealsid added a comment. Fix regression introduced by removing return statement in case where we were able to format a function name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://re

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid planned changes to this revision. nealsid added inline comments. Comment at: lldb/source/Core/FormatEntity.cpp:1560 } -return true; } Sorry, just caught this. Am uploading a new diff shortly. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked 2 inline comments as done. nealsid added a comment. In D98153#2610953 , @teemperor wrote: > Thanks for cleaning this up! Are the `if (!sc) ...` stuff are missing nullptr > checks that affect a normal LLDB session (not sure if we can ever ha

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked 3 inline comments as done. nealsid added inline comments. Comment at: lldb/include/lldb/Core/FormatEntity.h:144 +num_children(num_children), children(children), +keep_separator(keep_separator) {} }; teemperor wrote: > I

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 329266. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/FormatEntity.cpp lldb/unittests/Core/CMakeList

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid added a comment. Thanks - all the comments sound good. I'll upload a new patch. It seems like the pre-merge check failures are because I incorrectly set the project repo when I first created the revision. I noticed in the build commands here: https://buildkite.com/llvm-project/premer

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Thanks for cleaning this up! Are the `if (!sc) ...` stuff are missing nullptr checks that affect a normal LLDB session (not sure if we can ever have no SymbolContext) or is thi

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 328917. nealsid added a comment. Herald added a subscriber: JDevlieghere. Reuploading patch after I incorrectly set project repo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.or