[Lldb-commits] [PATCH] D131335: [lldb] abi_tag support 3/3 - Use mangle tree API to determine approximate mangled matches

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131335#3733117 <https://reviews.llvm.org/D131335#3733117>, @Michael137 wrote:

> In D131335#3732877 <https://reviews.llvm.org/D131335#3732877>, @labath wrote:
>
>> FWIW, I would be all for attaching the abi tags to the clang declarations if they would be in some easily accessible form (e.g. a DWARF attribute). Parsing them out of the mangled name is somewhat dubious, but I am not entirely against that, if it is necessary for some use case. However, even if we did that, I'd still say that attaching the asm attribute is a good idea.
>
> We're actually thinking of maybe getting rid off the fallback logic for the C++ plugin entirely. Only a handful of API tests seem to call into it (and some of them don't even expect the symbol to be found). But maybe there are some critical corner cases that require this that aren't covered by the tests.

I expect that this logic exists mostly for the benefit of low-quality or "minimal" debug info, at least that's what it looks like for me when I see the kinds of fallbacks that are used. For example the `char`->`signed char` substitution exists probably to work around the fact that DWARF does not really recognize `char` as a separate type (not in the same way that C does). So the type `char` could end up having using DW_ATE_signed_char (same as `signed char`) or DW_ATE_unsigned_char (same as `unsigned char`), depending on whether the type actually is signed or not. Makes sense if all you want is to provide a description of the type, but it does not sufficient for reconstructing mangled names. This is why our DWARF parsing code (TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize) needs to use string comparisons to get this right. One could say the same for int->long substitutions, etc.

That means that these things will only show up when using debug info produced by third-party compilers (I think even gcc should be fine), which is not something that we test regularly. That said, I don't think this is a good reason to keep this code around, and I'd hope that the asm labels will fix most of these issues (obviously, you won't be able to call the right overload based on the signedness of a char variable, but you couldn't do that before either). If it breaks anyones use case, we can figure out how to fix this in a better way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131335