patch: remove entries from UndefinedInternals as functions are defined

[Richard Smith]

Strange indentation in SortUndefinedInternals::operator(). (I assume
this was clang-format obstinately refusing to indent anything within a
namespace?)

Other than that, LGTM

On Wed, Jan 30, 2013 at 6:43 PM, Nick Lewycky <nlewycky at google.com> wrote:
> On 29 January 2013 18:58, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>> On Tue, Jan 29, 2013 at 6:50 PM, Nick Lewycky <nlewycky at google.com> wrote:
>> > On 29 January 2013 14:16, Richard Smith <richard at metafoo.co.uk> wrote:
>> >>
>> >> Hi Nick,
>> >>
>> >> Your operator< might not be a total order -- it's possible for uses of
>> >> two functions to have the same source location if at least one of them
>> >> is implicitly called (as part of an implicit conversion, for
>> >> instance). Perhaps you could break ties by comparing source locations
>> >> of the NamedDecl?
>> >
>> >
>> > Done. UndefinedInternal now holds a SourceManager* (the alternative
>> > being a
>> > second FullSourceLoc for the decl).
>> >
>> >> Also, please capitalize the added local variable names (I think these
>> >> were introduced by reverting a previous change?), and remove the
>> >> redundant braces in SemaDecl.cpp.
>> >
>> >
>> > Done.
>> >
>> > Updated patch attached
>>
>> Looks fine, though it'd be nice to factor the common code out of the
>> diagnostic and serialization paths.
>
>
> Yep. Updated patch attached!
>
> Nick
>
>> >> On Tue, Jan 29, 2013 at 4:04 AM, Nick Lewycky <nlewycky at google.com>
>> >> wrote:
>> >> > Currently UndefinedInternals grows an entry each time an internal
>> >> > function
>> >> > is ODR-used before it is defined, and this mapping is filtered only
>> >> > when
>> >> > diagnostics are emitted.
>> >> >
>> >> > This patch shrinks that list by removing entries as functions are
>> >> > defined.
>> >> > Because that's a performance sensitive path, we test to see whether
>> >> > there
>> >> > even is a previous function declaration, and if so that it was
>> >> > ODR-used,
>> >> > before doing the lookup that would remove the entry. Secondly, this
>> >> > patch
>> >> > applies the same filtering used when emitting diagnostics to emitting
>> >> > PCH.
>> >> >
>> >> > A large chunk of this patch is reverting the switch to MapVector in
>> >> > r173538,
>> >> > as MapVector does not (and can not) support efficient removals.
>> >> >
>> >> > Please review!
>> >> >
>> >> > Nick
>> >> >
>> >> > _______________________________________________
>> >> > cfe-commits mailing list
>> >> > cfe-commits at cs.uiuc.edu
>> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >> >
>> >
>> >
>
>