Force is_invocable template parameters to be complete types

[zoecarver]

Summary

This patch adds __complete_type (along with some helpers). It also updates is_invocable to assert that it's template parameters are complete types, void, or an unbounded array.

The patch introduces a lot of helper structs which create some bloat. If you decide that it would be better if it were more condensed, that is fine with me. I decided to start with maximum verbosity though.

[zoecarver]

include/__tree

973

Adding a check using __complete_type might help fix this issue (41360).

include/type_traits

4451

Is there a better way to include function types?

4504

I am not sure if it is better to static_assert this or to make it affect value.

[EricWF]

This patch isn't correct. The is_flexable_invocable trait is an ODR violation. And I don't think the bug this is solution to PR41360.

include/type_traits

4437

This is an ODR violation in waiting.

include/utility

1596

The types have to be complete here. It's an ODR violation otherwise.

[zoecarver]

Your right, PR41360 is only a wording issue. I also agree __flexible_invokable is bad, but I don't see another way to check invokability while allowing incomplete types (which I think is necessary for containers).

As for __invokable_r, I think the two options are either ignore the UB or implement some version of __complete_type. I was just going off what the comment said needed to be done.

[EricWF]

In D60393#1460727, @zoecarver wrote:

Your right, PR41360 is only a wording issue. I also agree __flexible_invokable is bad, but I don't see another way to check invokability while allowing incomplete types (which I think is necessary for containers).

I'm not sure it's necessary, but the way libstdc++ addressed this seems preferable. They moved the triggering of the diagnostic into the destructor. Which meant the types couldn't be incomplete when it was evaluated.
This used to be what we did, but @ldionne reverted it in r348529 to improve the diagnostic message. I think we should revert that commit.

As for __invokable_r, I think the two options are either ignore the UB or implement some version of __complete_type. I was just going off what the comment said needed to be done.

We had a __check_complete implementation previously. I removed it because it's near impossible to correctly check that *all* the types are complete in a call expression. See llvm.org/PR37407 and r332040.

[ldionne]

In D60393#1460732, @EricWF wrote:

In D60393#1460727, @zoecarver wrote:

Your right, PR41360 is only a wording issue. I also agree __flexible_invokable is bad, but I don't see another way to check invokability while allowing incomplete types (which I think is necessary for containers).

I'm not sure it's necessary, but the way libstdc++ addressed this seems preferable. They moved the triggering of the diagnostic into the destructor. Which meant the types couldn't be incomplete when it was evaluated.
This used to be what we did, but @ldionne reverted it in r348529 to improve the diagnostic message. I think we should revert that commit.

I think we should move my check back to the destructor, yes. However, reverting the commit as a whole results in worse error messages. You can retain slightly better error messages AND avoid crazy forward declarations AND fix the issue discussed in PR41360. I'll submit a patch.

[zoecarver]

Thanks @ldionne. Let me know if you want me to help at all.

[zoecarver]

include/type_traits

4419

This is now the only issue we still have to resolve. The standard says all these types must "be a complete type, (possibly cv-qualified) void, or an array of unknown bound. Otherwise, the behavior is undefined." Which (I think) means that we should either put an error/warning here or just let it be UB and get rid of the comment.

[EricWF]

include/type_traits

4419

We should keep the comment or replace it with a bug, but implementing the naive library implementation isn't what we want.

Also, it's worth noting checking for completeness doesn't address PR41360. Foo* is a complete type even if Foo isn't. For example, the return type, function type, and argument types are all complete in the following code:

struct Base {};
struct Derived;
void sink(Base*);
void test(Derived* p) { std::invoke(&sink, p); }
struct Derived : Base {};

And there are many variations of this example, such as:

namespace NS { struct X; }
void baz(...) = delete;
void test2(const NS::X &x) { baz(x); }
struct NS::X { friend void baz(const X&); };

Additionally, the current wording is grossly underconstrained. It allows invoke_result_t<Func, Incomplete&&>, but forbids invoke_result_t<Func, Incomplete> even though they're functionally equivalent (declval<T>() and declval<T&&>() return the same type).

Also the cost if instantiating all the templates required to check for completeness is non-trivial.

Perhaps we should open a bug about this, file a standard issue, and see if we can add a builtin type-trait to clang which handles this checking for us.

[zoecarver]

include/type_traits

4419

Also, it's worth noting checking for completeness doesn't address PR41360. Foo* is a complete type even if Foo isn't. For example, the return type, function type, and argument types are all complete in the following code:

Do you mean that it should not be treated as a complete type or that it is not being treated as a complete type?

Anyway, I completely agree with your last point. How hard do you expect it would be to add a builtin? I have been looking for a relatively easy one to start with.

include/__hash_table

#ifndef _LIBCPP_CXX03_LANG
     static_assert(is_copy_constructible<_Equal>::value,
     "the specified comparator is required to be copy constructible");
 #endif
     typedef int type;
 };
 
 template <class _Key, class _Hash, class _Equal>
 #ifndef _LIBCPP_CXX03_LANG
-    _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Equal const&, _Key const&, _Key const&>::value,
+    _LIBCPP_DIAGNOSE_WARNING(!__flexible_invokable<_Equal const&, _Key const&, _Key const&>::value,
     "the specified comparator type does not provide a const call operator")
-    _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Hash const&, _Key const&>::value,
+    _LIBCPP_DIAGNOSE_WARNING(!__flexible_invokable<_Hash const&, _Key const&>::value,
     "the specified hash functor does not provide a const call operator")
 #endif
 typename __enforce_unordered_container_requirements<_Key, _Hash, _Equal>::type
 __diagnose_unordered_container_requirements(int);
 
 // This dummy overload is used so that the compiler won't emit a spurious
 // "no matching function for call to __diagnose_unordered_xxx" diagnostic
 // when the overload above causes a hard error.

include/__tree

private:
     template <class, class, class> friend class _LIBCPP_TEMPLATE_VIS set;
     template <class, class, class> friend class _LIBCPP_TEMPLATE_VIS multiset;
     template <class> friend class _LIBCPP_TEMPLATE_VIS __map_const_iterator;
 
 };
 
 template<class _Tp, class _Compare>
 #ifndef _LIBCPP_CXX03_LANG
-    _LIBCPP_DIAGNOSE_WARNING(!std::__invokable<_Compare const&, _Tp const&, _Tp const&>::value,
+    _LIBCPP_DIAGNOSE_WARNING(!std::__flexible_invokable<_Compare const&, _Tp const&, _Tp const&>::value,
         "the specified comparator type does not provide a const call operator")
 #endif
 int __diagnose_non_const_comparator();
 
 template <class _Tp, class _Compare, class _Allocator>
 class __tree
 {
 public:

include/type_traits

template <class _Tp> struct _LIBCPP_TEMPLATE_VIS remove_all_extents<_Tp[]>
     {typedef typename remove_all_extents<_Tp>::type type;};
 template <class _Tp, size_t _Np> struct _LIBCPP_TEMPLATE_VIS remove_all_extents<_Tp[_Np]>
     {typedef typename remove_all_extents<_Tp>::type type;};
 
 #if _LIBCPP_STD_VER > 11
 template <class _Tp> using remove_all_extents_t = typename remove_all_extents<_Tp>::type;
 #endif
 
+template <class>     struct _LIBCPP_TEMPLATE_VIS __is_unbounded_array        : false_type {};
+template <class _Tp> struct _LIBCPP_TEMPLATE_VIS __is_unbounded_array<_Tp[]> : true_type {};
+
 #if _LIBCPP_STD_VER > 17
 // is_bounded_array
 
 template <class>                 struct _LIBCPP_TEMPLATE_VIS is_bounded_array           : false_type {};
 template <class _Tp, size_t _Np> struct _LIBCPP_TEMPLATE_VIS is_bounded_array<_Tp[_Np]> : true_type {};
 
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR
 bool is_bounded_array_v  = is_bounded_array<_Tp>::value;
 
 // is_unbounded_array
 
-template <class>     struct _LIBCPP_TEMPLATE_VIS is_unbounded_array        : false_type {};
-template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_unbounded_array<_Tp[]> : true_type {};
+template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_unbounded_array :
+    __is_unbounded_array<_Tp> {};
 
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR
 bool is_unbounded_array_v  = is_unbounded_array<_Tp>::value;
 #endif
 
 // decay
 

 template <class _Fp, class ..._Args>
 inline _LIBCPP_INLINE_VISIBILITY
 _LIBCPP_CONSTEXPR auto
 __invoke_constexpr(_Fp&& __f, _Args&& ...__args)
 _LIBCPP_INVOKE_RETURN(_VSTD::forward<_Fp>(__f)(_VSTD::forward<_Args>(__args)...))
 
 #undef _LIBCPP_INVOKE_RETURN
 
+template<class _Tp>
+struct __remove_all_bound_extents
+{ using type = _Tp; };
+
+template<class _Tp>
+struct __remove_all_bound_extents<_Tp[]>
+{ using type = _Tp[]; };
+
+template<class _Tp, size_t __n>
+struct __remove_all_bound_extents<_Tp[__n]>
+{ using type = _Tp; };
+
+template<class _Tp>
+struct __make_pure
+{
+    using type = typename remove_reference<
+                    typename remove_pointer<
+                        typename __remove_all_bound_extents<_Tp>::type
+                    >::type
+                 >::type;
+};
+
+template<class... _Args> struct __complete_type : true_type { };
+
+template<class _Tp>
+struct __complete_type<_Tp>
+{
+    template<class _T2, size_t = sizeof(_T2)>
+    static true_type __test(int);
+
+    template<class _T2>
+    static false_type __test(...);
+
+    template<class _T2>
+    using __test_validity = decltype(__test<_T2>(0));
+
+    using __pure_type = typename __make_pure<_Tp>::type;
+
+    using type = typename conditional<
+        is_function<__pure_type>::value,
+        true_type,
+        __test_validity<__pure_type>
+    >::type;
+
+    static const bool value = type::value;
+};
+
+template<class _Tp, class _T2, class... _Args>
+struct __complete_type<_Tp, _T2, _Args...>
+{
+    static const bool value = __complete_type<_Tp>::value &&
+                              __complete_type<_T2>::value &&
+                              __complete_type<_Args...>::value;
+};
+
+template<class... _Args> struct __valid_invokable_type : true_type { };
+
+template<class _Tp>
+struct __valid_invokable_type<_Tp>
+{
+    using __pure_type = typename __make_pure<_Tp>::type;
+
+    static const bool value = __complete_type<_Tp>::value || // already makes type pure
+                              is_void<__pure_type>::value ||
+                              __is_unbounded_array<__pure_type>::value;
+};
+
+template<class _Tp, class _T2, class... _Args>
+struct __valid_invokable_type<_Tp, _T2, _Args...>
+{
+    static const bool value = __valid_invokable_type<_Tp>::value &&
+                              __valid_invokable_type<_T2>::value &&
+                              __valid_invokable_type<_Args...>::value;
+};
+
+template <class _Fp, class ..._Args>
+struct __flexible_invokable
+{
+    template<class _XFp, class... _XArgs>
+    static auto __try_call(int) -> decltype(__invoke(declval<_XFp>(),
+                                                     declval<_XArgs>()...));
+    template<class _XFp, class... _XArgs>
+    static __nat __try_call(...);
+
+    using _Result = decltype(__try_call<_Fp, _Args...>(0));
+    static const bool value = !is_same<_Result, __nat>::value;
+};
+
 // __invokable
 template <class _Ret, class _Fp, class ..._Args>
 struct __invokable_r
 {
+  static_assert(__valid_invokable_type<_Ret, _Fp, _Args...>::value,
+                "_Ret, _Fp and all _Args must be complete types, void, or unbounded arrays");
+
   template <class _XFp, class ..._XArgs>
-  static auto __try_call(int) -> decltype(
-    _VSTD::__invoke(_VSTD::declval<_XFp>(), _VSTD::declval<_XArgs>()...));
+  static auto __try_call(int) -> decltype(__invoke(declval<_XFp>(),
+                                                   declval<_XArgs>()...));
   template <class _XFp, class ..._XArgs>
   static __nat __try_call(...);
 
-  // FIXME: Check that _Ret, _Fp, and _Args... are all complete types, cv void,
-  // or incomplete array types as required by the standard.
   using _Result = decltype(__try_call<_Fp, _Args...>(0));
 
   using type =
   typename conditional<
       !is_same<_Result, __nat>::value,
       typename conditional<
           is_void<_Ret>::value,
           true_type,
           is_convertible<_Result, _Ret>
       >::type,
       false_type
   >::type;
   static const bool value = type::value;
 };
+
 template <class _Fp, class ..._Args>
 using __invokable = __invokable_r<void, _Fp, _Args...>;
 
 template <bool _IsInvokable, bool _IsCVVoid, class _Ret, class _Fp, class ..._Args>
 struct __nothrow_invokable_r_imp {
   static const bool value = false;
 };
 

include/utility

 };
 #endif
 
 #ifndef _LIBCPP_CXX03_LANG
 template <class _Key, class _Hash>
 using __check_hash_requirements = integral_constant<bool,
     is_copy_constructible<_Hash>::value &&
     is_move_constructible<_Hash>::value &&
-    __invokable_r<size_t, _Hash, _Key const&>::value
+    __flexible_invokable<_Hash, _Key const&>::value
 >;
 
 template <class _Key, class _Hash = std::hash<_Key> >
 using __has_enabled_hash = integral_constant<bool,
     __check_hash_requirements<_Key, _Hash>::value &&
     is_default_constructible<_Hash>::value
 >;
 

test/libcxx/type_traits/invokable.fail.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+
+#include<type_traits>
+
+// Testing internal templates
+// __invokable_r tests if a function may be called using a given
+// set of parameter types and a return type (or void).
+// It is expected to fail (with a static assertion failure) when it
+// is given an incomplete type
+// (with the exception of void or an unbounded array).
+
+// Not defined
+class Foo;
+struct Bar;
+
+int main()
+{
+    static_assert(!std::__invokable_r<int, Foo, int>::value);
+    static_assert(!std::__invokable_r<int, Bar, int>::value);
+    static_assert(!std::__invokable_r<void, Foo>::value);
+    static_assert(!std::__invokable_r<void, Bar>::value);
+
+    return 0;
+}
 No newline at end of file

test/libcxx/type_traits/invokable.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+
+#include<type_traits>
+
+// Testing internal templates
+// __complete_type checks if type is "complete".
+// __invokable_r tests if a function may be called using a given
+//      set of parameter types and a return type (or void).
+
+struct Foo {
+    int operator()(int a, int b) { return a + b; }
+};
+
+struct Baz {
+    int operator() (int const & a, int const & b) const { return a + b; }
+};
+
+struct Bar; // Not defined
+
+int func(int) { return true; }
+
+void simple () { }
+
+int main()
+{
+    const int arr[4][4][4] = {{{}}};
+    const int (*i)[4];
+
+    static_assert( std::__complete_type<Foo>::value);
+    static_assert( std::__complete_type<int>::value);
+    static_assert( std::__complete_type<decltype(i)>::value);
+    static_assert( std::__complete_type<decltype(arr)>::value);
+    static_assert(!std::__complete_type<Bar>::value);
+    static_assert(!std::__complete_type<Bar*>::value);
+    static_assert(!std::__complete_type<int[]>::value);
+    static_assert(!std::__complete_type<void*>::value);
+
+    static_assert( std::__invokable_r<int, Foo, int, int>::value);
+    static_assert( std::__invokable_r<void, decltype(simple)>::value);
+    static_assert( std::__invokable_r<void, decltype(func), int>::value);
+    static_assert( std::__invokable_r<int, decltype(func), int>::value);
+    static_assert( std::__invokable_r<int, decltype(func) &, int>::value);
+    static_assert( std::__invokable_r<int, Baz const &, int const &, int const &>::value);
+    static_assert(!std::__invokable_r<int, Foo const &, int const &, int const &>::value);
+    static_assert(!std::__invokable_r<int, Foo, int>::value);
+
+    return 0;
+}
 No newline at end of file