[PATCH 6/6] qsort comparator consistency checking

Alexander Monakov amonakov@ispras.ru
Thu Aug 10 13:35:00 GMT 2017


On Wed, 9 Aug 2017, Jeff Law wrote:
> >> The _5th macro isn't that bad either, appart from using reserved namespace
> >> identifiers (it really should be something like qsort_5th and the arguments
> >> shouldn't start with underscores).
> > 
> > I didn't understand what Jeff found "ugly" about it; I wonder what epithets
> > would be applied then to more, ahem, unusual parts of GCC.
> I doubt anyone would want to hear what I might say about other code.
> I'm sure I wouldn't want my kids reading how I might refer to certain
> parts of GCC.

Imho it's no good to just say "ugly" in patch review without any further
elaboration, it only serves to provide a minor chilling effect, telling
the contributor they haven't done good enough (for your personal taste)
without informing them where/how they could have improved.

More importantly, am I correct in understanding that at this point all
remaining changes are reviewed and approved, and I can go ahead with
preparing vec::qsort -> vec::sort mass-renaming patch and rebasing the
others on top of it?  Or is the original approach with argument-counting
trick still under consideration?

Thanks.
Alexander



More information about the Gcc-patches mailing list