<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 19, 2015 at 10:50 AM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.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 class=""><div class="h5"><br>
> On 2015-Jan-19, at 10:28, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Fri, Dec 19, 2014 at 5:13 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
><br>
> > On 2014-Dec-19, at 15:10, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> ><br>
> ><br>
> ><br>
> > On Wed, Dec 3, 2014 at 2:18 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> ><br>
> ><br>
> > On Wed, Dec 3, 2014 at 2:08 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> ><br>
> > > On 2014-Dec-03, at 13:12, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> > ><br>
> > ><br>
> > ><br>
> > > On Thu, Nov 27, 2014 at 9:41 AM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> > >> Disabling uniquing for inlined locations seems like a pretty<br>
> > >> heavy hammer to me, and I worry that in contexts with heavy inlining it<br>
> > >> could cause an explosion.<br>
> > >><br>
> > > Any case where this creates more nodes due to inlining are cases that are buggy currently, so far as I know.<br>
> > ><br>
> > > The point of adding the column info was to make every call site distinct from every other call site - this uniquing just does the same thing but entirely consistently, as far as I understand it.<br>
> ><br>
> > Ah, okay.  This makes sense to me now.<br>
> ><br>
> > I made a few testcases of my own and ran with/without your patch and I<br>
> > better understand what you've done.  I'd imagined a bunch of interactions<br>
> > with `DebugLoc` that your approach avoids.<br>
> ><br>
> > >> Assuming disabling uniquing is the right goal, I really don't like this<br>
> > >> self-reference pattern.  I'm hoping to have a patch out early next week<br>
> > >> (I've been on holiday this week) that splits metadata and value -- and<br>
> > >> one of the first follow-ups will be a change to MDNode that allows<br>
> > >> "opting-out" of uniquing.  Do you think it makes sense to wait and use<br>
> > >> that, rather than introducing and pulling out an extra field?<br>
> > ><br>
> > > Another reason I sent this for review - I don't mind much waiting/how it gets done, just that it gets done.<br>
> ><br>
> > Let's wait then.  I'm hoping to get things moving along soon.<br>
> ><br>
> > Sure thing.<br>
> ><br>
> > Ping on this - but expect it'll be some time in the new year,<br>
><br>
> Leaving in 2 minutes for some holiday :).<br>
><br>
> > so I'll try to remember to check back in a month or so.<br>
><br>
> Please do!<br>
><br>
> Pinging just so I don't forget.<br>
><br>
> Now that we have 'distinct', I guess I just need to change the stuff that builds 'inlinedAt' location nodes to use that flag, perhaps?<br>
<br>
</div></div>Yeah, I think calling `MDLocation::getDistinct()` just does what you<br>
want here.</blockquote><div><br>Yep, seems to work basically the same. Committed in r226736.<br><br>Thanks!<br>- David </div></div><br></div></div>