<div dir="ltr">On 29 January 2013 18:58, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Tue, Jan 29, 2013 at 6:50 PM, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>> wrote:<br>


> On 29 January 2013 14:16, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
>><br>
>> Hi Nick,<br>
>><br>
>> Your operator< might not be a total order -- it's possible for uses of<br>
>> two functions to have the same source location if at least one of them<br>
>> is implicitly called (as part of an implicit conversion, for<br>
>> instance). Perhaps you could break ties by comparing source locations<br>
>> of the NamedDecl?<br>
><br>
><br>
> Done. UndefinedInternal now holds a SourceManager* (the alternative being a<br>
> second FullSourceLoc for the decl).<br>
><br>
>> Also, please capitalize the added local variable names (I think these<br>
>> were introduced by reverting a previous change?), and remove the<br>
>> redundant braces in SemaDecl.cpp.<br>
><br>
><br>
> Done.<br>
><br>
> Updated patch attached<br>
<br>
</div>Looks fine, though it'd be nice to factor the common code out of the<br>
diagnostic and serialization paths.<br></blockquote><div><br></div><div style>Yep. Updated patch attached!</div><div style><br></div><div style>Nick</div><div style><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="HOEnZb"><div class="h5">
>> On Tue, Jan 29, 2013 at 4:04 AM, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>> wrote:<br>
>> > Currently UndefinedInternals grows an entry each time an internal<br>
>> > function<br>
>> > is ODR-used before it is defined, and this mapping is filtered only when<br>
>> > diagnostics are emitted.<br>
>> ><br>
>> > This patch shrinks that list by removing entries as functions are<br>
>> > defined.<br>
>> > Because that's a performance sensitive path, we test to see whether<br>
>> > there<br>
>> > even is a previous function declaration, and if so that it was ODR-used,<br>
>> > before doing the lookup that would remove the entry. Secondly, this<br>
>> > patch<br>
>> > applies the same filtering used when emitting diagnostics to emitting<br>
>> > PCH.<br>
>> ><br>
>> > A large chunk of this patch is reverting the switch to MapVector in<br>
>> > r173538,<br>
>> > as MapVector does not (and can not) support efficient removals.<br>
>> ><br>
>> > Please review!<br>
>> ><br>
>> > Nick<br>
>> ><br>
>> > _______________________________________________<br>
>> > cfe-commits mailing list<br>
>> > <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>