On 30 January 2013 19:15, Richard Smith <[email protected]> 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 <[email protected]> wrote: > > On 29 January 2013 18:58, Richard Smith <[email protected]> wrote: > >> > >> On Tue, Jan 29, 2013 at 6:50 PM, Nick Lewycky <[email protected]> > wrote: > >> > On 29 January 2013 14:16, Richard Smith <[email protected]> > 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 <[email protected]> > >> >> 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 > >> >> > [email protected] > >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >> >> > > >> > > >> > > > > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
