Re: RFC: PR libstdc++/80064 make heap algorithms work with function types

[Ville Voutilainen <ville dot voutilainen at gmail dot com>]

On 16 March 2017 at 15:02, Jonathan Wakely <jwakely@redhat.com> wrote:
> Richi reported a GCC 7 regression for a testcase from Cython that
> boils down to:
>
> void
> test01(int* first, int* last)
> {
>  extern bool cmp(int, int);
>  // PR libstdc++/80064
>  // This is undefined, because [alg.sorting] requires the template argument
>  // Compare to be a function object type, and bool(int, int) is not an
>  // object type. We previously accepted it by accident, so keep doing so.
>  std::make_heap<int*, bool(int, int)>(first, last, cmp);
> }
>
> I can restore support for this with the attached patch (changing the
> heap algos to use the decayed type of the cmp parameter instead of the
> original _Compare template argument type), but should we support it?
>
> Is this worth fixing, or should we just say it worked by accident
> before and the user code needs to be fixed?
>
> FWIW MSVC accepts the example, libc++ rejects it for similar reasons
> to GCC 7.


All in all it seems to some extent reasonable to me to allow such
code. It seems rather easy
to fail to provide a function pointer as the comparator type, but also
rather easy for the implementation
to not mind that. I do find it suspicious that any user code would
bother providing the comparator
type as a template argument, though; whenever users feel encouraged to
do that, trouble ensues.
At this point I'm gravitating towards fixing the regression now, and
possibly being more stringent in the next
release, meaning gcc 8.