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