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 this just for the unit test?
Anyway, I have some small inline comments but otherwise this looks pretty good
to me.
================
Comment at: lldb/include/lldb/Core/FormatEntity.h:113
+ const Definition
+ *children; // An array of "num_children" Definition entries,
+ const bool keep_separator;
----------------
Can you doxygenify this when you anyway touch this code (`/// An array of
"num_children" Definition entries,` before the member)?
================
Comment at: lldb/include/lldb/Core/FormatEntity.h:144
+ num_children(num_children), children(children),
+ keep_separator(keep_separator) {}
};
----------------
I believe you could make this far less verbose with delegating constructors and
using C++11 default member initializers.
================
Comment at: lldb/source/Core/FormatEntity.cpp:1759
+ return false;
+ }
const char *name = nullptr;
----------------
One-line if's don't have `{}` around the body according to LLVM code style (I
know this is wrong in a lot of LLDB code, but newer code should follow that
style). Just commenting this once but the same thing is happening above/below
this change too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98153/new/
https://reviews.llvm.org/D98153
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
