[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

Adrian Prantl via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 23 10:12:21 PDT 2022


aprantl added inline comments.


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h:1
+//===-- Coroutines.h -------------------------------------------*- C++//-*-===//
+//
----------------
typo: `-*- C++ -*-`


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h:20
+
+// libc++, libstdc++ and MSVC STL: std::coroutine_handle<T>
+bool StdlibCoroutineHandleSummaryProvider(ValueObject &valobj, Stream &stream,
----------------
Can you convert this to a slightly more verbose Doxygen comment? It's not clear to me what exactly this comment means.


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h:21
+// libc++, libstdc++ and MSVC STL: std::coroutine_handle<T>
+bool StdlibCoroutineHandleSummaryProvider(ValueObject &valobj, Stream &stream,
+                                          const TypeSummaryOptions &options);
----------------
It would be nice to add Doxygen comments here, too. At least for the class itself.


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:15
+
+class TestCoroutineHandle(TestBase):
+    def do_test(self, stdlib_type):
----------------
Nice test!


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:44
+
+        lldbutil.run_to_source_breakpoint(self, '// Break after co_yield',
+                lldb.SBFileSpec("main.cpp", False))
----------------
Does this launch a new process? It might be faster to just set an additional breakpoint and run `process.Continue()`.


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp:35
+  gen.hdl.resume(); // Break after co_yield
+  // Break at final_suspend
+}
----------------
This looks risky, can you add a call to an empty function here, so we can be reasonably sure that there is code at this line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132415



More information about the cfe-commits mailing list