slint icon indicating copy to clipboard operation
slint copied to clipboard

C++: Use concepts for the invokables

Open ogoffart opened this issue 2 years ago • 4 comments

I don't think this is a breaking change actually.

One thing that this doesn't constraint is the return type. But i don't know if there is a standard concept for that.

ogoffart avatar Jan 24 '22 16:01 ogoffart

Looks like the clang on the macosx CI still don't support what we need here. So maybe not worth merging?

ogoffart avatar Jan 24 '22 17:01 ogoffart

we could have a

#if clang
#define SIXTYFPS_CONCEPT(X) typename
#else
#define SIXTYFPS_CONCEPT(X) X
#endif

and some other tweaks

ogoffart avatar Jan 24 '22 17:01 ogoffart

(it looks like the problem is that the concepts header was only added in libc++ 13

ogoffart avatar Jan 24 '22 17:01 ogoffart

With Xcode 13.3 (LLVM 13 based) the concepts header exists. Perhaps it works now.

lukas-jung avatar Apr 07 '22 13:04 lukas-jung

For comparison calling on_ with a lambda that has one argument too many

Before this patch:

In file included from /usr/include/c++/12/bits/unique_ptr.h:36,
                 from /usr/include/c++/12/memory:76,
                 from /home/olivier/slint/api/cpp/include/slint.h:12,
                 from /tmp/.tmp42a7Y9.cpp:5:
/usr/include/c++/12/tuple: In instantiation of ‘constexpr decltype(auto) std::__apply_impl(_Fn&&, _Tuple&&, index_sequence<_Idx ...>) [with _Fn = main()::<lambda(int, int)>&; _Tuple = const tuple<int>&; long unsigned int ..._Idx = {0}; index_sequence<_Idx ...> = integer_sequence<long unsigned int, 0>]’:
/usr/include/c++/12/tuple:1863:31:   required from ‘constexpr decltype(auto) std::apply(_Fn&&, _Tuple&&) [with _Fn = main()::<lambda(int, int)>&; _Tuple = const tuple<int>&]’
/home/olivier/slint/api/cpp/include/slint_callbacks.h:37:39:   required from ‘void slint::private_api::Callback<Ret(Arg ...)>::set_handler(F) const [with F = main()::<lambda(int, int)>; Ret = int; Arg = {int}]’
/tmp/.tmp42a7Y9.cpp:323:46:   required from ‘auto TestCase::on_test_func(Functor&&) const [with Functor = main()::<lambda(int, int)>]’
/tmp/.tmp42a7Y9.cpp:389:26:   required from here
/usr/include/c++/12/tuple:1852:27: error: no matching function for call to ‘__invoke(main()::<lambda(int, int)>&, std::__tuple_element_t<0, std::tuple<int> >&)’
 1852 |       return std::__invoke(std::forward<_Fn>(__f),
      |              ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
 1853 |                            std::get<_Idx>(std::forward<_Tuple>(__t))...);
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/12/bits/refwrap.h:38,
                 from /usr/include/c++/12/vector:66,
                 from /home/olivier/slint/api/cpp/include/slint.h:11:
/usr/include/c++/12/bits/invoke.h:90:5: note: candidate: ‘template<class _Callable, class ... _Args> constexpr typename std::__invoke_result<_Functor, _ArgTypes>::type std::__invoke(_Callable&&, _Args&& ...)’
   90 |     __invoke(_Callable&& __fn, _Args&&... __args)
      |     ^~~~~~~~
/usr/include/c++/12/bits/invoke.h:90:5: note:   template argument deduction/substitution failed:
/usr/include/c++/12/bits/invoke.h: In substitution of ‘template<class _Callable, class ... _Args> constexpr typename std::__invoke_result<_Functor, _ArgTypes>::type std::__invoke(_Callable&&, _Args&& ...) [with _Callable = main()::<lambda(int, int)>&; _Args = {const int&}]’:
/usr/include/c++/12/tuple:1852:27:   required from ‘constexpr decltype(auto) std::__apply_impl(_Fn&&, _Tuple&&, index_sequence<_Idx ...>) [with _Fn = main()::<lambda(int, int)>&; _Tuple = const tuple<int>&; long unsigned int ..._Idx = {0}; index_sequence<_Idx ...> = integer_sequence<long unsigned int, 0>]’
/usr/include/c++/12/tuple:1863:31:   required from ‘constexpr decltype(auto) std::apply(_Fn&&, _Tuple&&) [with _Fn = main()::<lambda(int, int)>&; _Tuple = const tuple<int>&]’
/home/olivier/slint/api/cpp/include/slint_callbacks.h:37:39:   required from ‘void slint::private_api::Callback<Ret(Arg ...)>::set_handler(F) const [with F = main()::<lambda(int, int)>; Ret = int; Arg = {int}]’
/tmp/.tmp42a7Y9.cpp:323:46:   required from ‘auto TestCase::on_test_func(Functor&&) const [with Functor = main()::<lambda(int, int)>]’
/tmp/.tmp42a7Y9.cpp:389:26:   required from here
/usr/include/c++/12/bits/invoke.h:90:5: error: no type named ‘type’ in ‘struct std::__invoke_result<main()::<lambda(int, int)>&, const int&>’

With this patch (using concepts):

/tmp/.tmp1pCnPT.cpp: In function ‘int main()’:
/tmp/.tmp1pCnPT.cpp:389:26: error: no matching function for call to ‘TestCase::on_test_func(main()::<lambda(int, int)>) const’
  389 |     instance.on_test_func([weak = slint::ComponentWeakHandle(handle)](int a, int) {
      |     ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  390 |         return (*weak.lock())->get_some_value() * a;
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  391 |     });
      |     ~~                    
/tmp/.tmp1pCnPT.cpp:79:55: note: candidate: ‘template<class Functor>  requires  invocable<Functor, int> auto TestCase::on_test_func(Functor&&) const’
   79 |     template<std::invocable<int> Functor> inline auto on_test_func (Functor && callback_handler) const;
      |                                                       ^~~~~~~~~~~~
/tmp/.tmp1pCnPT.cpp:79:55: note:   template argument deduction/substitution failed:
/tmp/.tmp1pCnPT.cpp:79:55: note: constraints not satisfied
In file included from /usr/include/c++/12/compare:39,
                 from /usr/include/c++/12/array:38,
                 from /tmp/.tmp1pCnPT.cpp:1:
/usr/include/c++/12/concepts: In substitution of ‘template<class Functor>  requires  invocable<Functor, int> auto TestCase::on_test_func(Functor&&) const [with Functor = main()::<lambda(int, int)>]’:
/tmp/.tmp1pCnPT.cpp:389:26:   required from here
/usr/include/c++/12/concepts:336:13:   required for the satisfaction of ‘invocable<Functor, int>’ [with Functor = main::._anon_203]
/usr/include/c++/12/concepts:336:25: note: the expression ‘is_invocable_v<_Fn, _Args ...> [with _Fn = main::._anon_203; _Args = {int}]’ evaluated to ‘false’
  336 |     concept invocable = is_invocable_v<_Fn, _Args...>;
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
thread 'test_cpp_callbacks_return_value' panicked at 'called `Result::unwrap()` on an `Err` value: "C++ Compilation error (see stdout)"', /home/olivier/slint/target/debug/build/test-driver-cpp-aebc8830c5a0ccb6/out/test_functions.rs:529:20

Since the generated code don't check the return value, a missing error value produce (before and after this patch, its the same)

In file included from /home/olivier/slint/target/debug/build/slint-cpp-d1d488b931c455b4/out/generated_include/slint_internal.h:15,
                 from /home/olivier/slint/api/cpp/include/slint.h:29,
                 from /tmp/.tmp0oRsti.cpp:5:
/home/olivier/slint/api/cpp/include/slint_callbacks.h: In instantiation of ‘void slint::private_api::Callback<Ret(Arg ...)>::set_handler(F) const [with F = main()::<lambda(int)>; Ret = int; Arg = {int}]’:
/tmp/.tmp0oRsti.cpp:323:46:   required from ‘auto TestCase::on_test_func(Functor&&) const [with Functor = main()::<lambda(int)>]’
/tmp/.tmp0oRsti.cpp:389:26:   required from here
/home/olivier/slint/api/cpp/include/slint_callbacks.h:37:39: error: void value not ignored as it ought to be
   37 |                             std::apply(*reinterpret_cast<F *>(user_data),
      |                             ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   38 |                                        *reinterpret_cast<const Tuple *>(arg));
      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

ogoffart avatar Jan 26 '23 10:01 ogoffart

Ah, so the missing return value is the same. The argument mismatch example looks like an improvement to me.

tronical avatar Jan 26 '23 10:01 tronical