[PATCH] D36993: [llvm-dwarfdump] Print type names in DW_AT_type DIEs

Fri Sep 1 14:32:48 PDT 2017

On Fri, Sep 1, 2017 at 2:04 PM Adrian Prantl <aprantl at apple.com> wrote:

> On Sep 1, 2017, at 1:56 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Sep 1, 2017 at 1:50 PM Adrian Prantl <aprantl at apple.com> wrote:
>
>> On Sep 1, 2017, at 1:46 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Fri, Sep 1, 2017 at 1:41 PM Adrian Prantl <aprantl at apple.com> wrote:
>>
>>> On Sep 1, 2017, at 1:36 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>> Could we get reasonable test cases for them up-front, to see where
>>> things are?
>>>
>>>
>>> You mean for the cases that do work as expected? That seems reasonable.
>>>
>>
>> All the cases, actually - so we can see what they are, whether they need
>> improvement, track what those improvements are, etc.
>>
>>
>> That seems excessive to me, since you'd have to create them manually
>> (unless yaml-obj makes that trivial?). What about all C(++) types that can
>> be produced by clang?
>>
>
> Doesn't seem too hideously expensive to produce by hand, are they? maybe
> 10-15 lines of assembly (even including the abbreviation) per thing? (It
> can be really simple DWARF - like a bunch of DW_TAG_variables that have
> only a DW_AT_type (no name, no other attributes), etc)
>
>
> Perhaps, but you'll also need to dig through the specification to
> understand how each is supposed to be used (for example, a variable with a
> DW_AT_type(DW_TAG_thrown_type <>) doesn't actually make sense). I don't
> know, maybe I'm overestimating how much work this would be.
>

That seems good, though - I'm mostly interested in the sensible cases for
each thing. If syntactically valid but nonsense DWARF gets printed weirdly,
I wouldn't be worried.

- Dave

>
>
>
>
>>
>> -- adrian
>>
>>
>>
>>>
>>> -- adrian
>>>
>>>
>>> (I still find it a bit weird to get const/volatile falling out through
>>> this process, but yeah, if there's a whole bunch of other cases that fall
>>> through this way for now, guess it makes  sense)
>>>
>>> On Fri, Sep 1, 2017 at 1:24 PM Adrian Prantl via Phabricator <
>>> reviews at reviews.llvm.org> wrote:
>>>
>>>> aprantl added a comment.
>>>>
>>>> In https://reviews.llvm.org/D36993#859034, @dblaikie wrote:
>>>>
>>>> > In https://reviews.llvm.org/D36993#858768, @JDevlieghere wrote:
>>>> >
>>>> > > In https://reviews.llvm.org/D36993#858121, @dblaikie wrote:
>>>> > >
>>>> > > > In https://reviews.llvm.org/D36993#858093, @JDevlieghere wrote:
>>>> > > >
>>>> > > > > David, apologies for missing your e-mail. I really hate that it
>>>> doesn't automatically show up in Phabricator! 🙁
>>>> > > > >
>>>> > > > > If the tag doesn't have a name attribute, everything will go
>>>> through this function except: `DW_TAG_pointer_type`,
>>>> `DW_TAG_ptr_to_member_type`, `DW_TAG_reference_type`,
>>>> `DW_TAG_rvalue_reference_type`. The first part explains why `class` and
>>>> `struct` don't show up. I prefer this approach because it's guaranteed to
>>>> be robust. Every `DW_TAG_*_type` encountered without a name will have
>>>> something meaningful printed.
>>>> > > > >
>>>> > > > > IIRC, the original switch had between 20 and 25 cases.
>>>> > > >
>>>> > > >
>>>> > > > I'm curious what those 20-25 cases were - do you have a
>>>> copy/roughly describe their contents? Because while 'const' does print
>>>> nicely, (& volatile would be similar) I'm not sure what the other 10 or so
>>>> cases might be and whether that's a reasonable way to print them.
>>>> > >
>>>> > >
>>>> > > Here's the list of cases I had originally:
>>>> > >
>>>> > >   case DW_TAG_array_type:
>>>> > >   case DW_TAG_base_type:
>>>> > >   case DW_TAG_class_type:
>>>> > >   case DW_TAG_const_type:
>>>> > >   case DW_TAG_enumeration_type:
>>>> > >   case DW_TAG_file_type:
>>>> > >   case DW_TAG_interface_type:
>>>> > >   case DW_TAG_packed_type:
>>>> > >   case DW_TAG_pointer_type:
>>>> > >   case DW_TAG_ptr_to_member_type:
>>>> > >   case DW_TAG_reference_type:
>>>> > >   case DW_TAG_restrict_type:
>>>> > >   case DW_TAG_set_type:
>>>> > >   case DW_TAG_shared_type:
>>>> > >   case DW_TAG_string_type
>>>> > >   case DW_TAG_structure_type:
>>>> > >   case DW_TAG_subrange_type:
>>>> > >   case DW_TAG_subroutine_type:
>>>> > >   case DW_TAG_thrown_type:
>>>> > >   case DW_TAG_union_type:
>>>> > >   case DW_TAG_unspecified_type:
>>>> > >   case DW_TAG_volatile_type:
>>>> > >
>>>> >
>>>> >
>>>> > Ah, thanks!
>>>> >
>>>> > I feel like maybe this should be examined more closely (an example of
>>>> how each of these would be printed would be ideal, though that might be a
>>>> bit much) - for example I don't think it makes sense to print out
>>>> subroutine types like "int subroutine" (rather than "int(float, double)",
>>>> say) which I /think/ is how they might look based on the current code)
>>>>
>>>>
>>>> Generally agreed, but I think it might make sense to improve this in
>>>> separate follow-on patches on a case-by-case basis. Getting the
>>>> pretty-printing entirely right would mean that we would have to implement
>>>> different pretty-printers for each DW_LANG_foo, since e.g., a C function
>>>> type would have to be rendered very differently from the same DWARF
>>>> type-representation in an. e.g., Swift or Fortran context. And even if we
>>>> choose to always render types as C types it is unclear what to do with
>>>> types such as DW_TAG_set_type.
>>>>
>>>>
>>>> Repository:
>>>>   rL LLVM
>>>>
>>>> https://reviews.llvm.org/D36993
>>>>
>>>>
>>>>
>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170901/f47a9039/attachment.html>