patch: remove entries from UndefinedInternals as functions are defined

Nick Lewycky nlewycky at google.com
Wed Jan 30 18:43:21 PST 2013


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
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130130/28d8430e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: undefined-internal-erase-on-define-3.patch
Type: application/octet-stream
Size: 10343 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130130/28d8430e/attachment.obj>


More information about the cfe-commits mailing list