<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 16, 2013 at 12:38 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="im">On Wed, Oct 16, 2013 at 11:54 AM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div>On Wed, Oct 16, 2013 at 11:10 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div>On Wed, Oct 16, 2013 at 10:54 AM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div>On Tue, Oct 15, 2013 at 5:59 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">
<div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote"><div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote"><div>In beginModule, we construct the CUs, but not all the DIEs that belong to the CU.</div><div>In endFunction, we started to construct the scope DIEs. So in some sense, we are adding things to prior CUs.</div>
<div><br></div><div>Looking at void CompileUnit::addDIEEntry(DIE *Die, uint16_t Attribute, DIE *Entry), we can possibly have 3 CUs, this CU,</div><div>the CU Die belongs to (i.e. the CU the parent chain points to), the CU Entry belongs to. </div>
</div></div></div></blockquote><div><br></div></div></div><div>When does "this CU" differ from both the Die and the Entry?</div></div></div></div></blockquote><div><br></div></div></div><div>The signature of addDIEEntry didn't say that "this CU" should be the same as either Die's CU or Entry's CU.</div>
</div></div></div></blockquote><div><br></div></div><div>No, but the code prior to your patch that's the only really sane thing for it to do, right? Why would one CompileUnit be dealing with some other CU's DIEs?<br>
<br>
This is why we should carefully think about the new semantics we're introducing by doing cross-CU DIE references.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div>If you think it should not happen, we should put a comment in so other developers will not violate it.</div><div>But that is irrelevant if we are going to make a single assumption:</div><div> "in addDIEEntry, if a DIE is without a parent, it belongs to the CU we call addDIEEntry on".<br>
</div></div></div></div></blockquote><div><br></div></div><div>I'm not entirely sure that will be true. In the case of implicit special members, such as the copy ctor, what could happen is that a later CU might want to add a member function to an earlier CU. That member function DIE will be being constructed from within the latter CompileUnit, but its parent will be the former CU. Walking the parent chain will find the correct (older CU) because we build the context for a member function and attach the member function to that context before we build any of its parameters, template arguments, etc.</div>
<div><br></div></div></div></div></blockquote></div><div>Exactly, so it is possible that a later CU can construct a DIE that belongs to another CU.</div><div> </div><div>I was rephrasing the statement in your earlier email and trying to be sure what assumption we are going to make.</div>
<div>
<div> <span style="color:rgb(80,0,80)">The claim Eric and I are making/discussed is that we should assume any DIE without a parent is in the current CU and/or that it possibly should never come up (I haven't thought through the codepaths carefully here - if we add CUs to their parent before creating their members, I think it should never come up).</span></div>
<div><br></div></div><div>I know that the assumption may not be true with the current code base, but we need to either</div><div>1> keep the worklist or</div><div>2> remove the worklist and modify the code base to make the assumption true, right?</div>
</div></div></div></blockquote><div><br></div></div><div>I'm OK assuming it to be true until we show it to not be true and treating those cases as bugs and fixing them on an as-found basis.<br></div></div></div></div>
</blockquote><div><br></div></div><div>Okay, I attached a patch removing the worklist. Please review,</div></div></div></div></blockquote><div><br></div></div><div>Will do.</div></div></div></div></blockquote><div><br>patch looks fairly obvious/trivial<br>
<br>Have you tried any of the test cases I've described (special members, nested types, and member templates - all used across CUs so an earlier CU has the type definition and a latter one has one of these extra members)? I'd like to see test results for these before we progress. <br>
</div></div></div></div>