patch: remove entries from UndefinedInternals as functions are defined

Nick Lewycky nlewycky at google.com
Wed Jan 30 19:25:26 PST 2013


On 30 January 2013 19:15, Richard Smith <richard at metafoo.co.uk> wrote:

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

Doh, thanks. Clang-format was not involved.

Other than that, LGTM
>

Thanks!! Committed in r174034.

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
> >> >> >
> >> >
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130130/6c010d30/attachment.html>


More information about the cfe-commits mailing list