[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:996
+      TypeSummaryImplSP(new StringSummaryFormat(
+          eTypeOptionHideChildren | eTypeOptionHideValue, "${var.__rep_}")));
 }
----------------
Nice!


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/chrono/TestDataFormatterLibcxxChrono.py:23
+        # clean slate for the next test case.
+        def cleanup():
+            self.runCmd("type format clear", check=False)
----------------
You probably copied & pasted this — this is no longer needed since every test function is now running in its own instance.


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/chrono/TestDataFormatterLibcxxChrono.py:33
+
+        # empty vectors (and storage pointers SHOULD BOTH BE NULL..)
+        self.expect("frame variable ns", substrs=["ns = 0"])
----------------
This is not guaranteed behavior and is highly specific on the actual compiler. I would probably just remove this first sets of tests.


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/chrono/main.cpp:17
+
+  return 0; // break here
+}
----------------
can you change this to a function call like
```
std::cout()<<"break here\n";
```

It's not guaranteed that all compilers actually generate code for this line that results in a breakpoint opportunity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159127