Undefined uses of std::is_invocable in <concepts> and <ranges> tests

[jwakely]

Bugzilla Link	50715
Version	12.0
OS	All
CC	@mclow

Extended Description

The tests for the std::invocable concept use std::is_invocable with an argument of incomplete type, which is undefined (it violates the precondition for std::is_invocable):

In file included from /home/jwakely/gcc/12/include/c++/12.0.0/ratio:39,
                 from /home/jwakely/gcc/12/include/c++/12.0.0/chrono:39,
                 from std/concepts/concepts.callable/concept.invocable/invocable.compile.pass.cpp:15:
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits: In instantiation of 'struct std::is_invocable<int S::*, S>':
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits:3001:73:   required from 'constexpr const bool std::is_invocable_v<int S::*, S>'
/home/jwakely/gcc/12/include/c++/12.0.0/concepts:338:25:   required from 'constexpr bool pointer_to_member_functions::check_member_is_invocable() [with Member = int S::*; T = S; Args = {}]'
std/concepts/concepts.callable/concept.invocable/invocable.compile.pass.cpp:259:53:   required from here
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits:2942:7: error: static assertion failed: each argument type must be a complete class or an unbounded array

Similarly for std::regular_invocable:

In file included from /home/jwakely/gcc/12/include/c++/12.0.0/ratio:39,
                 from /home/jwakely/gcc/12/include/c++/12.0.0/chrono:39,
                 from std/concepts/concepts.callable/concept.regularinvocable/regular_invocable.compile.pass.cpp:15:
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits: In instantiation of 'struct std::is_invocable<int S::*, S>':
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits:3001:73:   required from 'constexpr const bool std::is_invocable_v<int S::*, S>'
/home/jwakely/gcc/12/include/c++/12.0.0/concepts:338:25:   required from 'constexpr bool pointer_to_member_functions::check_member_is_invocable() [with Member = int S::*; T = S; Args = {}]'
std/concepts/concepts.callable/concept.regularinvocable/regular_invocable.compile.pass.cpp:285:53:   required from here
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits:2942:7: error: static assertion failed: each argument type must be a complete class or an unbounded array

The tests for std::ranges::data use std::is_invocable_v to check that an array of incomplete type can't be used, but that's undefined as stated above:

In file included from /home/jwakely/gcc/12/include/c++/12.0.0/concepts:44,
                 from /home/jwakely/gcc/12/include/c++/12.0.0/ranges:37,
                 from std/ranges/range.access/range.prim/data.pass.cpp:15:
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits: In instantiation of 'struct std::is_invocable<const std::ranges::__cust_access::_Data, Incomplete [2]>':
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits:3001:73:   required from 'constexpr const bool std::is_invocable_v<const std::ranges::__cust_access::_Data, Incomplete [2]>'
std/ranges/range.access/range.prim/data.pass.cpp:28:21:   required from here
/home/jwakely/gcc/12/include/c++/12.0.0/type_traits:2942:7: error: static assertion failed: each argument type must be a complete class or an unbounded array

Shouldn't those first three static assertions in std/ranges/range.access/range.prim/data.pass.cpp be using a reference to an array anyway? An array rvalue is not a borrowed_range, so it's ill-formed. If they are supposed to be testing the array of incomplete type condition, they aren't doing that, they're testing the "lvalue or borrowed range" condition.

Also, that test is not portable (should it be guarded by a preprocessor check?) because the standard doesn't require a diagnostic for trying to use ranges::data on an array of incomplete type. With libstdc++ you do get an error, but not in the immediate context (so it's not SFINAE-friendly and you can't check for it with is_invocable).

[Quuxplusone]

Is the right action here "grep for places that use std::is_invocable{,_v} on incomplete types, and make them use std::invocable instead"? If so, that's easy, I can do that.

Orthogonally, I agree that

static_assert(!std::is_invocable_v<RangeDataT, Incomplete[]>);

was likely intended to be

static_assert(!std::invocable<RangeDataT, Incomplete[]>);  // OK, portable
static_assert(!std::invocable<RangeDataT, Incomplete(&)[]>);  // move this IFNDR line into libcxx/test/libcxx

All these tests are messes. D115607 is somewhat related.

[jwakely]

I don't think std::invocable helps. That has a constraint that uses std::is_invocable_v so still has the complete type precondition.

If you don't care about violating that precondition (because you know your implementation doesn't diagnose it, and you don't care about the tests being portable) then using is_invocable is OK.

I can just #if these away in my local clone, to avoid the failures due to libstdc++ having different (but still conforming) behaviour for these tests.

[Quuxplusone]

Aw, gross, you're right: std::invocable merely tests the well-formedness of std::invoke(...), but std::invoke has a constraint that mentions std::is_invocable_v, and std::is_invocable_v is IFNDR for incomplete types (other than arrays of unknown bound), so ultimately std::invocable can be IFNDR too.

FWIW, I think std::is_invocable_v<RangeDataT, Incomplete[]> is actually fine, because is_invocable's wording specifically permits arrays of unknown bound even though they're incomplete. But in invocable.compile.pass.cpp, we have

static_assert(check_member_is_invocable<int S::*, S>());

which invokes std::invocable<int S::*, S>, which is definitely IFNDR.

[Quuxplusone]

Relevant to libc++ strategy here: "Completeness preconditions considered harmful" — I encourage libstdc++ to adopt the same strategy. :)
But as long as this remains IFNDR, we should be perfectly willing to remove IFNDRisms from libcxx/test/std/; file a new bug or reopen this one if you discover we missed any.