<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 18, 2016 at 8:33 AM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On Apr 18, 2016, at 8:08 AM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> <snip><br>
<div><div class="h5">><br>
> I'm also concerned about the semantics of a particular case that I<br>
> learned debugging correctness issues.  If a type is defined inside a<br>
> function, we create a DICompositeType (CT) whose "scope:" is the<br>
> DISubprogram (SP) for that function's definition.  This SP is not part of<br>
> the type graph, it's the actual definition that eventually describes the<br>
> code.<br>
><br>
> However, the CT has an ODR identifier, which means it will get uniqued<br>
> by the DITypeMap.  This seems wrong.  If the function is defined in a<br>
> header you can end up with two copies of this type, and their scope is<br>
> completely unrelated.  Here it is without using the type map:<br>
> --<br>
> !0 = distinct !DICompileUnit()<br>
> !1 = distinct !DISubprogram(name: "foo", unit: !0)<br>
> !2 = distinct !DICompositeType(name: "T", identifier: "foo_T", scope: !1)<br>
> !3 = distinct !DICompileUnit()<br>
> !4 = distinct !DISubprogram(name: "foo", unit: !3)<br>
> !5 = distinct !DICompositeType(name: "T", identifier: "foo_T", scope: !4)<br>
> --<br>
><br>
> I had thought that when !DICompositeType had an 'identifier:' (followed<br>
> ODR-rules) then its 'scope:' must also have an 'identifier:', or at least<br>
> follow ODR-rules.  That's clearly not always the case right now.  There<br>
> are two simple ways to make it so:<br>
><br>
>  1. Drop the 'identifier:' from composite types local to a subprogram.<br>
><br>
>  2. Change the 'scope:' to point at a *declaration* of the subprogram<br>
>     (which lives in the type graph) and unique them there (and disallow<br>
>     having 'identifier:' types point at subprogram definitions).<br>
<br>
</div></div>Intuitively option (2) seems to be the better solution — we want to separate out the type hierarchy from the function definitions. In the  spirit of PR27352[1], could we even stop DISubprograms from being DIScopes in the class hierarchy and use uniqueable DIPrototypes as scopes instead?<br></blockquote><div><br></div><div>I think that's going to produce some pretty weird DWARF I wouldn't expect existing debuggers to handle well - I doubt they're necessarily equipped to expect to see things like that in subprogram declarations & to merge them appropriately with the contents of the definition. (and what if the type is  in a local lexical scope in the function definition - do we put lexical scopes in the declaration? (how do we then describe which part of the function that lexical scope applies to? (which instruction range is the type name valid for)))<br><br>Removing uniquing from these wouldn't work (imagine a local type in an inline function in a header - lots of definitions of the function and its associated type) - if the type was only used locally, maybe - but the type could leak out into a template instantiation, and we could pick the wrong one (we could pick the outer function from TU1 and the template instantiation from TU2) potentially? Thus ending up with both/multiple instances of the type, or deduplicating in a problematic way that leads to both versions of the outer function (the one from TU1 and TU2)<br><br>I'm not sure yet what a good answer might be here</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-- adrian<br>
<br>
[1] <a href="https://llvm.org/bugs/show_bug.cgi?id=27352" rel="noreferrer" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=27352</a><br>
<div class="HOEnZb"><div class="h5">><br>
> I'd appreciate feedback on whether either of these is a good idea.<br>
><br>
> (Another way of looking at this, is that I'd imagined there were two<br>
> parts of the debug info graph: the "types" part, and the "codegen" part<br>
> (by which I mean stuff that describes the actual emitted object code);<br>
> and I'd imagined that the "types" part never referenced the "codegen"<br>
> part.  However it does in this case.)<br>
<br>
</div></div></blockquote></div><br></div></div>