Re: [PATCH] Add qsort comparator consistency checking (PR71702)

[Alexander Monakov <amonakov at ispras dot ru>]

On Tue, 19 Jul 2016, Richard Biener wrote:
> Yes.  The other option is to enable this checking not with ENABLE_CHECKING
> but some new checking option, say ENABLE_CHECKING_ALGORITHMS, and
> do full checking in that case.

Thanks - I'm going to fold in this idea when redoing the patch (i.e. check a
subset of pairs under normal checking, all pairs under this option macro).

While the topic is fresh, I'd like to mention a small complication with
extending this checking to cover all qsort calls.  I mentioned in the opening
mail that I was going to do that with a '#define qsort(..) qsort_chk (..)' in
gcc/system.h, but I missed that vec::qsort would be subject to this macro
expansion as well.

I see two possible solutions.  The first is to use the argument counting trick
to disambiguate between libc qsort(base, n, sz, cmp) and vec::qsort(cmp) on the
preprocessor level.  I don't see a reason it wouldn't work, but in this context
I'd consider that a last-resort measure rather than an appropriate solution.

The second is to rename vec::qsort to vec::sort. While mass renaming is not very
nice, I hope it is acceptable in this case (I think formally vec::qsort
declaration in GCC is not portable, because it implicitly expects that stdlib.h
wouldn't shadow qsort with a function-like macro).


Actually, thinking about it more, instead of redirecting qsort in system.h, it
may be more appropriate to introduce gcc_qsort that wraps qsort and does
checking, add gcc_qsort_nochk as an escape hatch for cases where checking
shouldn't be done, and poison qsort in system.h (this again depends on doing the
vec::sort mass-rename).

Alexander