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

Raphael Isemann via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Mar 8 05:17:15 PST 2021

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.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list