[PATCH] D99179: [RFC] [Coroutines] Enable printing coroutine frame in debugger if program is compiled with -g

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D99179#2703941 <https://reviews.llvm.org/D99179#2703941>, @ChuanqiXu wrote:

> In D99179#2703807 <https://reviews.llvm.org/D99179#2703807>, @dblaikie wrote:
>
>> In D99179#2688242 <https://reviews.llvm.org/D99179#2688242>, @ChuanqiXu wrote:
>>
>>> In D99179#2688049 <https://reviews.llvm.org/D99179#2688049>, @dblaikie wrote:
>>>
>>>> Something @probinson mentioned in the llvm-dev thread is that any debug info this creates should be sure that it doesn't produce duplicate unique entities - if it has to create any support types. Does the code correctly only produce one of them, even if there are multiple coroutines being lowered?
>>>
>>> Thanks for reminding! I don't know why I don't get the responding from @probinson even in the spam list. And I find it here: https://groups.google.com/g/llvm-dev/c/hfZ6z_MJFOw.
>>> It looks like it wouldn't create redundant base types. But it would still create one coroutine frame type for each coroutine. It makes sense because the layout of coroutine is different with each other (Although it could be the same, as the updated test case tells).
>>> And I didn't do type cache in the module level actually. So I guess it is handled by the infrastructure of the DI system.
>>
>> Hmm - does the test case cover this situation where there are two coroutine frames independently constructed/lowered, but they share basic types? My first look at some of the basic types created only shows them being used in one frame.
>>
>> Also, can this end up creating multiple types and variables with the same name in the same scope? If so, maybe those types and variables (the coro_frame types and variables specifically, from what I can see) should be unnamed instead of duplicately named? (or their names should somehow be uniqued, but I imagine that's non-trivial, having to look to see what coroutines have been lowered already/what names are already used)
>
> Thanks for the detailed review!
>
>> Hmm - does the test case cover this situation where there are two coroutine frames independently constructed/lowered, but they share basic types? My first look at some of the basic types created only shows them being used in one frame.
>
> Yes,  two coroutines would share basic types like `i32` and `double`. I updated the test case to cover that.
>
>> Also, can this end up creating multiple types and variables with the same name in the same scope? If so, maybe those types and variables (the coro_frame types and variables specifically, from what I can see) should be unnamed instead of duplicately named? (or their names should somehow be uniqued, but I imagine that's non-trivial, having to look to see what coroutines have been lowered already/what names are already used)
>
> Maybe I don't get your point correctly. I think it is necessary to create name for `__coro_frame` (the created variables) and member types including (`__resume_fn`, `__destroy_fn` and `__coro_index`). Since the users of debugger want to see coroutine frame by `p __coro_frame` in the debugger, I worries if we don't create names, then the end user can't print the variable or they can't get the meaning. It may be OK different coroutines uses the same name since their scopes are not the same. I think it is a little bit like:
>
>   void foo() {
>       strcut {
>         // ... whatever
>       } local_type;
>       local_type my_var;
>   }
>   void bar() {
>       strcut {
>         // ... whatever
>       } local_type;
>       local_type my_var;
>   }
>
> Both `foo` and `bar` have `local_type` and `my_var`. It wouldn't be confusing since we can know the context in the debugger.

Ah, if the types and variables are always local to independent functions (please confirm that two of these entities can't appear in the same scope), that's fine by me.


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

https://reviews.llvm.org/D99179