<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 17, 2014 at 7:45 AM, Robinson, Paul <span dir="ltr"><<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
> From: David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>]<br>
<div><div class="h5">> On Sat, Sep 13, 2014 at 6:54 PM, Robinson, Paul <<a href="mailto:Paul_Robinson@playstation.sony.com">Paul_Robinson@playstation.sony.com</a>> wrote:<br>
> > > > On 09 Sep 2014, at 00:01, <a href="mailto:jingham@apple.com">jingham@apple.com</a> wrote:<br>
> > > > ><br>
> > > > > From the debugger's standpoint, the functional concern is that if you do<br>
> > > > something more real, like:<br>
> > > > ><br>
> > > > > typedef int A;<br>
> > > > > template <typename T><br>
> > > > > struct S<br>
> > > > > {<br>
> > > > > T my_t;<br>
> > > > > };<br>
> > > > ><br>
> > > > > I want to make sure that the type of my_t is given as "A" not as "int".<br>
> > > > The reason for that is that it is not uncommon to have data formatters<br>
> > > > that trigger off the typedef name. This happens when you use some common<br>
> > > > underlying type like "int" but the value has some special meaning when it<br>
> > > > is formally an "A", and you want to use the data formatters to give it an<br>
> > > > appropriate presentation. Since the data formatters work by matching type<br>
> > > > name, starting from the most specific on down, it is important that the<br>
> > > > typedef name be preserved.<br>
> > > > ><br>
> > > > > However, it would be really odd to see:<br>
> > > > ><br>
> > > > > (lldb) expr -T -- my_s<br>
> > > > > (S<int>) $1 = {<br>
> > > > > (A) my_t = 5<br>
> > > > > }<br>
> > > > ><br>
> > > > > instead of:<br>
> > > > ><br>
> > > > > (lldb) expr -T -- my_s<br>
> > > > > (S<A>) $1 = {<br>
> > > > > (A) my_t = 5<br>
> > > > > }<br>
> > > > ><br>
> > > > > so I am in favor of presenting the template parameter type with the most<br>
> > > > specific name it was given in the overall template type name.<br>
> > > ><br>
> > > > OK, we get this wrong today. I’ll try to look into it.<br>
> > > ><br>
> > > > What’s your take on the debug info representation for the templated class<br>
> > > > type? The tentative patch introduces a typedef that declares S<A> as a<br>
> > > > typedef for S<int>. The typedef doesn’t exist in the code, thus I find it<br>
> > > > a bit of a lie to the debugger. I was more in favour of something like :<br>
> > > ><br>
> > > > DW_TAG_variable<br>
> > > > DW_AT_type: -> DW_TAG_structure_type<br>
> > > > DW_AT_name: S<A><br>
> > > > DW_AT_specification: -> DW_TAG_structure_type<br>
> > > > DW_AT_name: S<int><br>
> > > ><br>
> > > > This way the canonical type is kept in the debug information, and the<br>
> > > > declaration type is a real class type aliasing the canonical type. But I’m<br>
> > > > not sure debuggers can digest this kind of aliasing.<br>
> > > ><br>
> > > > Fred<br>
> > ><br>
> > > Why introduce the extra typedef? S<A> should have a template parameter<br>
> > > entry pointing to A which points to int. The info should all be there<br>
> > > without any extra stuff. Or if you think something is missing, please<br>
> > > provide a more complete example.<br>
> > My immediate concern here would be either loss of information or bloat<br>
> > when using that with type units (either bloat because each instantiation<br>
> > with differently spelled (but identical) parameters is treated as a separate<br>
> > type - or loss when the types are considered the same and all but one are<br>
> > dropped at link time)<br>
> You'll need to unpack that more because I'm not following the concern.<br>
> If the typedefs are spelled differently, don't they count as different types?<br>
> DWARF wants to describe the program as-written, and there's no S<int> written<br>
> in the program.<br>
><br>
> Maybe not in this TU, but possibly in another TU? Or by the user.<br>
><br>
> void func(S<int>);<br>
> ...<br>
> typedef int A;<br>
> S<A> s;<br>
> func(s); // calls the same function<br>
><br>
> The user probably wants to be able to call void func with S<int> or S<A><br>
<br>
</div></div>Sure.<br>
<span class=""><br>
> (and, actually, in theory, with S<B> where B is another typedef of int, but<br>
> that'll /really/ require DWARF consumer support and/or new DWARF wording).<br>
<br>
</span>Not DWARF wording. DWARF doesn't say when you can and can't call something;<br>
that's a debugger feature and therefore a debugger decision.<br></blockquote><div><br>What I mean is we'd need some new DWARF to help explain which types are equivalent (or the debugger would have to do a lot of spelunking to try to find structurally equivalent types - "S<B>" and "S<A>", go look through their DW_TAG_template_type_params, see if they are typedefs to the same underlying type, etc... )</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> We can't emit these as completely independent types - it would be verbose<br>
> (every instantiation with different typedefs would be a whole separate type<br>
> in the DWARF, not deduplicated by type units, etc) and wrong<br>
<br>
</span>Yes, "typedef int A;" creates a synonym/alias not a new type, so S<A> and S<int><br>
describe the same type from the C++ perspective, so you don't want two complete<br>
descriptions with different names, because that really would be describing them<br>
as separate types. What wrinkles my brow is having S<int> be the "real"<br>
description even though it isn't instantiated that way in the program. I wonder<br>
if it should be marked artificial... but if you do instantiate S<int> in another<br>
TU then you don't want that. Huh. It also seems weird to have this:<br>
DW_TAG_typedef<br>
DW_AT_name "S<A>"<br>
DW_AT_type -> S<int><br>
but I seem to be coming around to thinking that's the most viable way to have<br>
a single actual instantiated type, and still have the correct names of things<br></blockquote><div><br></div><div>Yep - it's the only way I can think of giving this information in a way that's likely to work with existing consumers. It would probably be harmless to add DW_AT_artificial to the DW_TAG_typedef, if that's any help to any debug info consumer.<br><br>That said, I'm not opposed to proposing something to DWARF to define some more 'proper' way to describe this.<br><br>One other open question is then, when, if ever, to reference the DW_TAG_typedef rather than the underlying type? Do we just reference it whenever the user writes using that name?<br><br> void f(S<A>);<br> ...<br> void f(S<B>) { ... }<br><br>etc... (this would be just as possible/we could maybe treat it the same as if the user wrote "void f(A); ... void f(B) { ... }")</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
(because DWARF is all about the name "as it appears in the source program.")<br>
<span class="im HOEnZb"><br>
> (the debugger wouldn't know these are actually the same type so wouldn't<br>
> allow function calls, etc).<br>
><br>
> - David<br>
><br>
> ><br>
> ><br>
</span><div class="HOEnZb"><div class="h5">> > > Jim<br>
> > ><br>
> > >> On Sep 8, 2014, at 12:38 PM, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
> > >><br>
> > >><br>
> > >>> On 08 Sep 2014, at 19:31, Greg Clayton <<a href="mailto:gclayton@apple.com">gclayton@apple.com</a>> wrote:<br>
> > >>><br>
> > >>> This means you will see "S<A>" as the type for your variables in the<br>
> > debugger when you view variables or children of structs/unions/classes. I<br>
> > think this is not what the user would want to see. I would rather see<br>
> > "S<int>" as the type for my variable than see "S<A>”.<br>
> > >><br>
> > >> I find it more accurate for the debugger to report what has actually<br>
> > been put in the code. Moreover when a typedef is used, it’s usually to<br>
> > make things more readable not to hide information, thus I guess it would<br>
> > usually be as informative while being more compact. The debugger needs to<br>
> > have a way to describe the real type behind the abbreviated name though,<br>
> > we must not have less information compared to what we have today.<br>
> > >><br>
> > >> Another point: this allows the debugger to know what S<A> actually is.<br>
> > Without it, the debugger only knows the canonical type. This means that<br>
> > currently you can’t copy/paste a piece of code that references that kind<br>
> > of template names and have it parse correctly. I /think/ that having this<br>
> > information in the debug info will allow more of this to work.<br>
> > >><br>
> > >> But we can agree to disagree :-) It would be great to have more people<br>
> > chime and give their opinion.<br>
> > >><br>
> > >> Fred<br>
> > >><br>
> > >>>> On Sep 5, 2014, at 4:00 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br>
> > >>>><br>
> > >>>><br>
> > >>>>> On Sep 5, 2014, at 3:49 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>><br>
> > wrote:<br>
> > >>>>><br>
> > >>>>><br>
> > >>>>><br>
> > >>>>><br>
> > >>>>> On Fri, Sep 5, 2014 at 3:43 PM, Duncan P. N. Exon Smith<br>
> > <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> > >>>>><br>
> > >>>>>> On 2014 Sep 5, at 16:01, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
> > >>>>>><br>
> > >>>>>> I couldn’t even find a subject expressing exactly what this patch<br>
> > is about… First of all, it’s meant to start a discussion, and I’m not<br>
> > proposing it for inclusion right now.<br>
> > >>>>>><br>
> > >>>>>> The issue I’m trying to address is that template types are always<br>
> > canonicalized when emitted in the debug information (this is the desugar()<br>
> > call in UnwrapTypeForDebugInformation).<br>
> > >>>>>><br>
> > >>>>>> This means that if the developer writes:<br>
> > >>>>>><br>
> > >>>>>> typedef int A;<br>
> > >>>>>> template <typename T><br>
> > >>>>>> struct S {};<br>
> > >>>>>><br>
> > >>>>>> S<A> var;<br>
> > >>>>>><br>
> > >>>>>> The variable var will have type S<int> and not S<A>. In this simple<br>
> > example, it’s not that much of an issue, but for heavily templated code,<br>
> > the full expansion might be really different from the original<br>
> > declaration.<br>
> > >>>>>><br>
> > >>>>>> The attached patch makes us emit an intermediate typedef for the<br>
> > variable’s type:<br>
> > >>>>>><br>
> > >>>>>> 0x0000002a: DW_TAG_variable [2]<br>
> > >>>>>> DW_AT_name [DW_FORM_strp] (<br>
> > .debug_str[0x00000032] = “var")<br>
> > >>>>>> DW_AT_type [DW_FORM_ref4] (cu + 0x0040 =><br>
> > {0x00000040})<br>
> > >>>>>> DW_AT_external [DW_FORM_flag] (0x01)<br>
> > >>>>>> DW_AT_decl_file [DW_FORM_data1] (0x01)<br>
> > >>>>>> DW_AT_decl_line [DW_FORM_data1] (8)<br>
> > >>>>>> DW_AT_location [DW_FORM_block1] (<0x09> 03 70 6c 00 00<br>
> > 00 00 00 00 )<br>
> > >>>>>><br>
> > >>>>>> 0x00000040: DW_TAG_typedef [3]<br>
> > >>>>>> DW_AT_type [DW_FORM_ref4] (cu + 0x004b =><br>
> > {0x0000004b})<br>
> > >>>>>> DW_AT_name [DW_FORM_strp] (<br>
> > .debug_str[0x00000035] = “S<A>")<br>
> > >>>>>> DW_AT_decl_file [DW_FORM_data1] (0x01)<br>
> > >>>>>> DW_AT_decl_line [DW_FORM_data1] (6)<br>
> > >>>>>><br>
> > >>>>>> 0x0000004b: DW_TAG_structure_type [4] *<br>
> > >>>>>> DW_AT_name [DW_FORM_strp] (<br>
> > .debug_str[0x0000003e] = “S<int>")<br>
> > >>>>>> DW_AT_byte_size [DW_FORM_data1] (0x01)<br>
> > >>>>>> DW_AT_decl_file [DW_FORM_data1] (0x01)<br>
> > >>>>>> DW_AT_decl_line [DW_FORM_data1] (6)<br>
> > >>>>>><br>
> > >>>>>><br>
> > >>>>>> Which basically is what I want, although I don’t like that it<br>
> > introduces a typedef where there is none in the code. I’d prefer that to<br>
> > be:<br>
> > >>>>>><br>
> > >>>>>> DW_TAG_variable<br>
> > >>>>>> DW_AT_type: -> DW_TAG_structure_type<br>
> > >>>>>> DW_AT_name: S<A><br>
> > >>>>>> DW_AT_specification: -> DW_TAG_structure_type<br>
> > >>>>>> DW_AT_name: S<int><br>
> > >>>>>> …<br>
> > >>>>>><br>
> > >>>>>> The patch also has the nice property of omitting the defaulted<br>
> > template arguments in the first level typedef. For example you get<br>
> > vector<A> instead of vector<int, std::__1::allocator<int> >.<br>
> > >>>>><br>
> > >>>>> If you specify `vector<int>` in C++ do you get that instead of<br>
> > >>>>> `vector<int, std::__1::allocator<int>>`?<br>
> > >>>>><br>
> > >>>>> Yeah, I mentioned this as possibly causing problems with debuggers<br>
> > or other consumers, but I don't have any proof past "ooooo scary!”.<br>
> > >>>><br>
> > >>>> Well, [+lldb-dev], could this confuse debuggers? :-)<br>
> > >>>><br>
> > >>>> -- adrian<br>
> > >>>>><br>
> > >>>>> That said, I like the idea personally :)<br>
> > >>>>><br>
> > >>>>> -eric<br>
> > >>>>><br>
> > >>>>><br>
> > >>>>>> Now there is one thing I really don’t like about the patch. In<br>
> > order not to emit typedefs for types that don’t need it, I use string<br>
> > comparison between the desugared and the original type. I haven’t<br>
> > quantified anything, but doing the construction of the type name for every<br>
> > template type and then comparing it to decide to use it or not seems like<br>
> > a big waste.<br>
> > >>>>><br>
> > >>>>> Maybe someone on cfe-dev knows a better way.<br>
> > >>>>><br>
> > >>>>>><br>
> > >>>>>> Thoughts?<br>
> > >>>>>><br>
> > >>>>>> <template-arg-typedefs.diff><br>
> > >>>>>><br>
> > >>>>>> Fred<br>
> > >>>>>><br>
> > >>>>>><br>
> > >>>>>><br>
> > >>>>>><br>
> > >>>>>><br>
> > >>>>>> _______________________________________________<br>
> > >>>>>> llvm-commits mailing list<br>
> > >>>>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > >>>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
> > >>>>><br>
> > >>>>><br>
> > >>>><br>
> > >>>> _______________________________________________<br>
> > >>>> lldb-dev mailing list<br>
> > >>>> <a href="mailto:lldb-dev@cs.uiuc.edu">lldb-dev@cs.uiuc.edu</a><br>
> > >>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</a><br>
> > >>><br>
> > >><br>
> > >><br>
> > >> _______________________________________________<br>
> > >> lldb-dev mailing list<br>
> > >> <a href="mailto:lldb-dev@cs.uiuc.edu">lldb-dev@cs.uiuc.edu</a><br>
> > >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</a><br>
> > ><br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>