[Lldb-commits] [PATCH] D129364: [LLDB][DataFormatter] Add data formatter for libcxx std::unordered_map iterator

[Adrian Prantl via Phabricator via lldb-commits]

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

Nice! Few minor stylistic comments inside.



================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:403
+
+  // this must be a ValueObject* because it is a child of the ValueObject we
+  // are producing children for it if were a ValueObjectSP, we would end up
----------------
Nit: `This`... and `.` at the end.


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:411
+                       ".__i_.__node_", nullptr, nullptr,
+                       ValueObject::GetValueForExpressionPathOptions()
+                           .DontCheckDotVsArrowSyntax()
----------------
Does this get more readable if we assign this subexpression to an auto helper variable?


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:448
+
+    if (addr && addr != LLDB_INVALID_ADDRESS) {
+      TypeSystemClang *ast_ctx =
----------------
maybe invert the condition and convert to early exit?


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py:13
+    @add_test_categories(['libc++'])
+    def test_with_run_command(self):
+        """Test that std::unordered_map related structures are formatted correctly when printed.
----------------
just curious: what does the _with_run_command suffix mean? Or was this copied from another test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129364