[Bug c++/113854] New: the expression 'is_invocable_v ... evaluated to 'false'

[Bug c++/113854] New: the expression 'is_invocable_v ... evaluated to 'false'

[f.heckenbach@fh-soft.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113854

            Bug ID: 113854
           Summary: the expression 'is_invocable_v ... evaluated to
                    'false'
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: f.heckenbach@fh-soft.de
  Target Milestone: ---

The following error output is less than helpful. It contains a long litany of
mostly library internals. The ultimate error is "the expression
'is_invocable_v<_Fn, _Args ...> [with _Fn = main::._anon_116&; _Args =
{std::unique_ptr<int, std::default_delete<int> >&}]' evaluated to 'false'"
which, while technically correct, is still not very helpful.

The actual error is trying to pass a move-only type by value rather than by
reference, but I can't infer this from the messages. So again, unfortunately, I
have to read gcc's message as "something's wrong, but I won't tell you what"
...

One of the promises of concepts was better error messages. That's obviously not
the case here, compared to the version with plain old std::find_if (below).

% cat test.cpp
#include <vector>
#include <memory>
#include <ranges>

int main ()
{
  std::vector <std::unique_ptr <int>> v;
  std::ranges::find_if (v, [] (auto i) { return !!i; });
}
% g++ -std=c++20 test.cpp
test.cpp: In function 'int main()':
test.cpp:8:24: error: no match for call to '(const std::ranges::__find_if_fn)
(std::vector<std::unique_ptr<int> >&, main()::<lambda(auto:20)>)'
    8 |   std::ranges::find_if (v, [] (auto i) { return !!i; });
      |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/12/ranges:47,
                 from test.cpp:3:
/usr/include/c++/12/bits/ranges_util.h:474:7: note: candidate: 'template<class
_Iter, class _Sent, class _Proj, class _Pred>  requires (input_iterator<_Iter>)
&& (sentinel_for<_Sent, _Iter>) && (indirect_unary_predicate<_Pred,
std::projected<_Iter, _Proj> >) constexpr _Iter
std::ranges::__find_if_fn::operator()(_Iter, _Sent, _Pred, _Proj) const'
  474 |       operator()(_Iter __first, _Sent __last,
      |       ^~~~~~~~
/usr/include/c++/12/bits/ranges_util.h:474:7: note:   template argument
deduction/substitution failed:
test.cpp:8:24: note:   candidate expects 4 arguments, 2 provided
    8 |   std::ranges::find_if (v, [] (auto i) { return !!i; });
      |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/12/bits/ranges_util.h:487:7: note: candidate: 'template<class
_Range, class _Proj, class _Pred>  requires (input_range<_Range>) &&
(indirect_unary_predicate<_Pred,
std::projected<decltype(std::ranges::__cust_access::__begin((declval<_Container&>)())),
_Proj> >) constexpr std::ranges::borrowed_iterator_t<_Range>
std::ranges::__find_if_fn::operator()(_Range&&, _Pred, _Proj) const'
  487 |       operator()(_Range&& __r, _Pred __pred, _Proj __proj = {}) const
      |       ^~~~~~~~
/usr/include/c++/12/bits/ranges_util.h:487:7: note:   template argument
deduction/substitution failed:
/usr/include/c++/12/bits/ranges_util.h:487:7: note: constraints not satisfied
In file included from /usr/include/c++/12/compare:39,
                 from /usr/include/c++/12/bits/stl_pair.h:65,
                 from /usr/include/c++/12/bits/stl_algobase.h:64,
                 from /usr/include/c++/12/vector:60,
                 from test.cpp:1:
/usr/include/c++/12/concepts: In substitution of 'template<class _Range, class
_Proj, class _Pred>  requires (input_range<_Range>) &&
(indirect_unary_predicate<_Pred,
std::projected<decltype(std::ranges::__cust_access::__begin((declval<_Container&>)())),
_Proj> >) constexpr std::ranges::borrowed_iterator_t<_Range>
std::ranges::__find_if_fn::operator()(_Range&&, _Pred, _Proj) const [with
_Range = std::vector<std::unique_ptr<int> >&; _Proj = std::identity; _Pred =
main()::<lambda(auto:20)>]':
test.cpp:8:24:   required from here
/usr/include/c++/12/concepts:336:13:   required for the satisfaction of
'invocable<_Fn, _Args ...>' [with _Fn = main::._anon_116&; _Args =
{std::unique_ptr<int, std::default_delete<int> >&}]
/usr/include/c++/12/concepts:340:13:   required for the satisfaction of
'regular_invocable<_Fn, _Args ...>' [with _Fn = main::._anon_116&; _Args =
{std::unique_ptr<int, std::default_delete<int> >&}]
/usr/include/c++/12/concepts:344:13:   required for the satisfaction of
'predicate<_Fn&, typename std::__detail::__iter_traits_impl<typename
std::remove_cvref<_Tp2>::type, std::indirectly_readable_traits<typename
std::remove_cvref<_Tp2>::type> >::type::value_type&>' [with _Fn =
main::._anon_116; _Iter =
std::projected<__gnu_cxx::__normal_iterator<std::unique_ptr<int,
std::default_delete<int> >*, std::vector<std::unique_ptr<int,
std::default_delete<int> >, std::allocator<std::unique_ptr<int,
std::default_delete<int> > > > >, std::identity>]
/usr/include/c++/12/bits/iterator_concepts.h:710:13:   required for the
satisfaction of 'indirect_unary_predicate<_Pred, std::projected<decltype
(std::ranges::__cust_access::__begin(declval<_Container&>())), _Proj> >' [with
_Pred = main::._anon_116; _Range = std::vector<std::unique_ptr<int,
std::default_delete<int> >, std::allocator<std::unique_ptr<int,
std::default_delete<int> > > >&; _Proj = std::identity]
/usr/include/c++/12/concepts:336:25: note: the expression 'is_invocable_v<_Fn,
_Args ...> [with _Fn = main::._anon_116&; _Args = {std::unique_ptr<int,
std::default_delete<int> >&}]' evaluated to 'false'
  336 |     concept invocable = is_invocable_v<_Fn, _Args...>;
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

% cat test2.cpp
#include <vector>
#include <memory>
#include <algorithm>

int main ()
{
  std::vector <std::unique_ptr <int>> v;
  std::find_if (v.begin (), v.end (), [] (auto i) { return !!i; });
}
% g++ -std=c++20 test2.cpp
In file included from /usr/include/c++/12/bits/stl_algobase.h:71,
                 from /usr/include/c++/12/vector:60,
                 from test2.cpp:1:
/usr/include/c++/12/bits/predefined_ops.h: In instantiation of 'constexpr bool
__gnu_cxx::__ops::_Iter_pred<_Predicate>::operator()(_Iterator) [with _Iterator
= __gnu_cxx::__normal_iterator<std::unique_ptr<int>*,
std::vector<std::unique_ptr<int> > >; _Predicate = main()::<lambda(auto:19)>]':
/usr/include/c++/12/bits/stl_algobase.h:2067:14:   required from 'constexpr
_RandomAccessIterator std::__find_if(_RandomAccessIterator,
_RandomAccessIterator, _Predicate, random_access_iterator_tag) [with
_RandomAccessIterator = __gnu_cxx::__normal_iterator<unique_ptr<int>*,
vector<unique_ptr<int> > >; _Predicate =
__gnu_cxx::__ops::_Iter_pred<main()::<lambda(auto:19)> >]'
/usr/include/c++/12/bits/stl_algobase.h:2112:23:   required from 'constexpr
_Iterator std::__find_if(_Iterator, _Iterator, _Predicate) [with _Iterator =
__gnu_cxx::__normal_iterator<unique_ptr<int>*, vector<unique_ptr<int> > >;
_Predicate = __gnu_cxx::__ops::_Iter_pred<main()::<lambda(auto:19)> >]'
/usr/include/c++/12/bits/stl_algo.h:3877:28:   required from 'constexpr _IIter
std::find_if(_IIter, _IIter, _Predicate) [with _IIter =
__gnu_cxx::__normal_iterator<unique_ptr<int>*, vector<unique_ptr<int> > >;
_Predicate = main()::<lambda(auto:19)>]'
test2.cpp:8:16:   required from here
/usr/include/c++/12/bits/predefined_ops.h:318:30: error: use of deleted
function 'std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp,
_Dp>&) [with _Tp = int; _Dp = std::default_delete<int>]'
  318 |         { return bool(_M_pred(*__it)); }
      |                       ~~~~~~~^~~~~~~
In file included from /usr/include/c++/12/memory:76,
                 from test2.cpp:2:
/usr/include/c++/12/bits/unique_ptr.h:514:7: note: declared here
  514 |       unique_ptr(const unique_ptr&) = delete;
      |       ^~~~~~~~~~
test2.cpp:8:39: note:   initializing argument 1 of 'main()::<lambda(auto:19)>
[with auto:19 = std::unique_ptr<int>]'
    8 |   std::find_if (v.begin (), v.end (), [] (auto i) { return !!i; });
      |                                       ^

[Bug c++/113854] the expression 'is_invocable_v ... evaluated to 'false'

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113854

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Anybody who promised concepts will always result in better errors was lying or
misinformed. They have the potential to help, but it takes real effort to make
it happen.

ranges::find_if is constrained to require the function to be callable with
those argument types, and that fails here. I'm not sure how we can do better.
If is_invocable is false, all we know is it's false. We can't poke at the
definition of is_invocable to see why.

A __builtin_is_invocable could _maybe_ capture more detail about the failure
which could then be recalled and displayed if there's a compilation failure due
to is_invocable not being satisfied (something which would be much more
difficult for enable_if-based constraints, but plausible with concepts, maybe).

I'm this specific case it's true that the error for std::find_if happens to be
quite clear, but I'm the general case it's better to be told "this function
requires is_invocable_v<F, A> and you didn't satisfy that" then to get errors
deep inside an instantiation stack.

[Bug c++/113854] the expression 'is_invocable_v ... evaluated to 'false'

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113854

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
And those are not library internals, they're the public concepts defined in the
standard. You can look them up to see what they mean, but the problem boils
down to what's shown at the end: your predicate can't be used with that
argument type.

I think that's actually a more accurate description of the problem than telling
you that a deleted copy constructor can't be used. We've just been conditioned
by years of bad errors to know what it means. But really what you're trying to
do is call a function, not copy a unique_ptr. The concept error tells you what
part of the API is not satisfied. The second error tells you about a symptom,
not the cause, and leaves to to infer the cause: it's trying to copy a move
only object because the lambda requires a copy.

There are plenty of cases where being told which concept failed is much clearer
than being told about some unintended symptom feel inside a template. The
existence of counterexamples doesn't mean concepts aren't an improvement.

[Bug c++/113854] the expression 'is_invocable_v ... evaluated to 'false'

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113854

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*unintended symptom deep inside a template

[Bug c++/113854] the expression 'is_invocable_v ... evaluated to 'false'

[f.heckenbach@fh-soft.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113854

--- Comment #4 from Frank Heckenbach <f.heckenbach@fh-soft.de> ---
(In reply to Jonathan Wakely from comment #1 and #2)

> [...] than to get errors deep inside an instantiation stack.

But they are deep in the stack; maybe not an instantiation stack, but a
requires stack or whatever you call it. Anyway, the text is much longer than
that for plain std::find_if.

> And those are not library internals, they're the public concepts defined in
> the standard.

is:invocable_v is standard, the rest seems to be full of __library _Internals.
Perhaps I wasn't clear, that's what I'm referring to. Apart from the last line
and a few others, most of the messages are not meaningful to a mere user.

> You can look them up to see what they mean, but the problem
> boils down to what's shown at the end: your predicate can't be used with
> that argument type.
> 
> I think that's actually a more accurate description of the problem than
> telling you that a deleted copy constructor can't be used. We've just been
> conditioned by years of bad errors to know what it means. But really what
> you're trying to do is call a function, not copy a unique_ptr.

Those messages may not be optimal, and perhaps we've actually been conditioned
in a way, but at least I've learned to understand what they mean. Deleted
copy-constructor, sure, it's about non-copyable types, I have something to go
on. Whereas the concepts message basically tells me I can't call it because I
can't invoke it.

In a simple case like this, it's clear why, but if i is not just a unique_ptr,
but a struct containing one among several other members (as was the case in my
actual code), it will be less obvious. And if the function takes more than one
argument, there will be no indication which one is the problem, whereas the
std::find_if message at least says "argument 1".

Also, the message "the expression 'is_invocable_v<_Fn, _Args ...> [with _Fn =
main::._anon_116&; _Args = {std::unique_ptr<int, std::default_delete<int>
>&}]'" IMHO reads more like a text puzzle. (This is a more general issue,
should I file another bug or two for it?)

a) It introduces 3 new identifiers, resolves 2 of them right after and the last
one not at all.

b) It lists default parameters the user doesn't really care about. I get that
gcc at this point probably doesn't know whether the parameters were defaulted
or specified (and happened to match the default, which seems unlikely). It
would be a big improvement if gcc could simplify that message to something like
this:

is_invocable_v<[] (auto), std::unique_ptr<int>> is required, but false

Please note: I'm not trying to be difficult here. I'm actually having trouble
understanding these kinds of messages. Finding the actual (see above) relevant
messages and puzzling them together now often takes more time than ignoring
them altogether and analyzing the code myself. That's why I said "something's
wrong here" would be as good a message. :(