[PATCH] D143397: [WIP][llvm][DebugInfo] Add DW_TAG_imported_declaration to accelerator tables

[Michael Buch via Phabricator via llvm-commits]

Michael137 added a comment.

In D143397#4107560 <https://reviews.llvm.org/D143397#4107560>, @dblaikie wrote:

> In D143397#4107156 <https://reviews.llvm.org/D143397#4107156>, @Michael137 wrote:
>
>> In D143397#4107079 <https://reviews.llvm.org/D143397#4107079>, @dblaikie wrote:
>>
>>> It'd be good if this issue were submitted to the DWARF committee as a fix for the .debug_names accelerator tables.
>>>
>>> I don't /think/ you need unnamed entries in the index - since these entries are only for finding named entities by unqualified name, so an unnamed using namespace doesn't introduce any new unqualified names?
>>>
>>> I guess if the user writes "x::y" (and the code has "z::y" defined and "using namespace z" in namespace x) then you need some breadcrumb that when you do unqualified lookup for "y" and unqualified lookup for "x" you can see that they're connected/actually would resolve to that name? How does this work currently for "z::y"? You do lookup for "y" then check its parent DIE and see that it's "z" and so it satisfies the lookup?
>>
>>
>>
>>> I don't /think/ you need unnamed entries in the index - since these entries are only for finding named entities by unqualified name, so an unnamed using namespace doesn't introduce any new unqualified names?
>>
>> Actually I'm not quite sure how we can get empty import declarations (since `using namespace Foo` is represented by `DW_TAG_imported_module`)
>
> Oh, right, sorry, didn't notice the distinction between imported module and imported declaration. Ah, you're talking about code like this:
>
>   namespace ns1 {
>   struct t1 { };
>   };
>   namespace ns2 {
>   using ns1::t1;
>   }
>   namespace ns3 {
>   using t2 = ns1::t1;
>   }
>
> So the DWARF for the first `DW_TAG_imported_declaration` (in `ns2`) won't have a name - it inherits the name from the entity.
>
> Oh... seems clang just doesn't emit DWARF for the `ns2::t1` case - if you try to use that type it resolves to `ns1::t1`, which is a (minor) bug. I implemented the imported declaration stuff years ago, and it's... not great (Clang doesn't track usage of using namespaces especially - and I guess unnamed using decls - makes it hard to emit them only when needed, and the usual DWARF-reachability that helps trim DWARF during IR optimization/linking can't kick in because nothing references the imported namespaces especially)
>
>>> I guess if the user writes "x::y" (and the code has "z::y" defined and "using namespace z" in namespace x) then you need some breadcrumb that when you do unqualified lookup for "y" and unqualified lookup for "x" you can see that they're connected/actually would resolve to that name?
>>
>> Good point, currently this isn't supported. And this patch wouldn't help with that. To support `x::y` in LLDB we'd have to add the `DW_TAG_imported_module` into the accelerator table too, under the name of the module it's importing (?) Though I'd be fine with not adding support for this for now
>
> Ah, right, fair enough.
>
>>> How does this work currently for "z::y"? You do lookup for "y" then check its parent DIE and see that it's "z" and so it satisfies the lookup?
>>
>> The order of operations is:
>>
>> 1. Lookup `z`. Finds namespace
>> 2. Lookup `y`. Finds namespace
>> 3. Check that `y` "is contained in" `z` (accounts for inline namespaces between the two). This is purely based on walking through the parent DIEs of `y`
>
> yeah, easy for the canonical/original namespace - doing that for imported namespaces/entities would be tricky. I guess, yeah, for named imported entities it's possible to put the name in the index, pointing to the imported entity - then you can do the walk up.
>
> But that doesn't work with unnamed using decls where all the names of the other namespace are brought in. :/



> yeah, easy for the canonical/original namespace - doing that for imported namespaces/entities would be tricky. I guess, yeah, for named imported entities it's possible to put the name in the index, pointing to the imported entity - then you can do the walk up.

Yup that's exactly how I've prototyped it in LLDB: https://reviews.llvm.org/D143398

> But that doesn't work with unnamed using decls where all the names of the other namespace are brought in. :/

True, I think I'll just proceed with not supporting the unnamed (and `using namespace ...`) cases for now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143397