[PATCH] D127623: [Debug] [Coroutine] Adjust the scope and name for coroutine frame

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D127623#3608905 <https://reviews.llvm.org/D127623#3608905>, @dblaikie wrote:

> Looks like C locals
>
> In D127623#3595240 <https://reviews.llvm.org/D127623#3595240>, @ChuanqiXu wrote:
>
>> John gives the suggestion for the DIName of coroutine frame in C++ version in https://github.com/itanium-cxx-abi/cxx-abi/issues/142::
>>
>>   <local-name> ::= Z <function encoding> E <entity name> [<discriminator>]
>>
>> So that the frame name in the example would be `_ZZL9coro_taskiE13coro_frame_ty`. And both `C++filt` and `llvm-cxxfilt` would detangle it as we want as `coro_task(int)::coro_frame_ty`. I guess this is what @dblaikie want initially. Then John throws another problem that the solution above is not suit for C-style names. Although we're talking about C++ coroutine only, we could use `extern "C"` in C++ to prevent mangling actually. (It should be rare actually. Since the use of C++ coroutines generally require the involvement of coroutine types like 'lazy' or 'generator'. But such coroutine types are not C compatible generally.)
>>
>> Then @avogelsgesang and @palves throw another requirement to avoid to see mangled names in gdb. I feel like this might not be necessary at first since the users wouldn't see the linkage name from our use experience. Although @palves throws the solution for locals, but I found it doesn't work local scope types:
>>
>>   C++
>>   int main() {
>>     struct S {
>>       int a;
>>       bool b;
>>     };
>>     S s;
>>   }
>>
>>   (gdb) p main::s
>>   $2 = {a = -8000, b = 255}
>>   (gdb) p main::S
>>   A syntax error in expression, near `'.
>>   (gdb) p 'main'::S
>>   A syntax error in expression, near `'.
>>   (gdb) p 'main()'::S
>>   A syntax error in expression, near `'.
>>
>> I am not sure if it is unable to make.
>>
>> ---
>>
>> So there are 2 problems we saw:
>> (1) How do we handle unmangled names?
>> (2) Do we need to enable the user to print the type without knowing mangled names?
>>
>> For the first problem, I prefer to use '.' as the separator since @palves gives reason why I failed earlier. And for the second problem, my answer is no due to above reasons. But it would be good if we have solutions.
>
> Looks like at least clang (it's not part of the ABI so clang and GCC do things differently) uses "func.var" to encode local static variables in C, so if "func.var" has some precedent for C and works OK for C++, I think that's probably a reasonably generic solution, that doesn't depend on detecting mangled names and remangling them (which is awkward when there's different manglings, etc).

Yeah, this is implemented in current revision. I agree with it at least it is simpler from the perspective of both implementers and users. Would you like to give this a LGTM?


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

https://reviews.llvm.org/D127623