[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D132624#3749263 <https://reviews.llvm.org/D132624#3749263>, @avogelsgesang wrote:

>> leads me to believe that the data formatter is creating the synthetic children as non pointer objects (e.g. by automatically dereferencing any nested pointers), even though the structures clearly contain some pointers inside
>
> Yes, that is correct. The formatter (introduced recently in https://reviews.llvm.org/D132415) does dereference the pointer internally. It replaces the
>
>   (std::coroutine_handle<>) hdl = {
>     __handle_ = 0x55555555b2a0
>   }
>
> by
>
>   (std::coroutine_handle<>) hdl = coro frame = 0x55555555b2a0 {
>       resume = 0x0000555555555a10 (a.out`coro_task(int, int) at llvm-example.cpp:36)
>       destroy = 0x0000555555556090 (a.out`coro_task(int, int) at llvm-example.cpp:36)
>   }

The question isn't so much about the dereferencing of actual physical pointers, but more of a any "logical" pointers in the data structures and how they're being presented by the formatters. Most of our formatters dereference some kinds of pointers. For example, a std::set formatter may need to dereference hundreds of pointers to display all of the members. That is OK(*), because those elements are logically "inside" that set. Just like you cannot have a `struct S` hold itself as a member (only a pointer to `S`), you also cannot have a `std::set` contain itself (it would either have to be a pointer, or a set of a different type). And, I think, neither should a coroutine "contain" another coroutine. I don't know much about coroutines, but it seems like your goal is to format them like a linked list, so may the (synthetic) object which represents the "next" coroutine (promise?) in the list should be of a pointer type, forcing the lldb (and user) to do an actual dereference operation before viewing the next element. Maybe that could be achieved by changing the type of `__promise.continuation.promise` (I'm using the example from your patch description) from `std::...::promise_type` to `std::...::promise_type *`  ?

(*) However, the formatter should be wary of loops due to corrupted data structures. That's why our std::list formatter has an internal loop detection to prevent it from getting stuck on corrupted lists. AFAIK, our map formatter does not have that, probably because noone ran into that problem yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132624