patch: remove entries from UndefinedInternals as functions are defined

Richard Smith richard at metafoo.co.uk
Tue Jan 29 18:58:49 PST 2013


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.

>> 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
>> >
>
>



More information about the cfe-commits mailing list