On Fri, Dec 5, 2025 at 4:06 PM Tomasz Kaminski <[email protected]> wrote:
> Just one small suggestion, otherwise LGTM. > > On Fri, Dec 5, 2025 at 3:57 PM Patrick Palka <[email protected]> wrote: > >> Tested on x86_64-pc-linu-xgnu, does this look OK for trunk? >> >> -- >8 -- >> >> Implement the cv-qual forwarding required by std::bind using deducing >> this when available, instead of needing 4 operator() overloads. Using >> deducing this here is more complicated here than in other call >> wrappers because std::bind is not really "perfect forwarding": it >> doesn't forward value category, and along with const-ness it also >> forwards volatile-ness (before C++20). >> >> The old implementation suffers from the same problem that other >> SFINAE-friendly call wrappers have which is solved by using deducing >> this (see p5.5 of the deducing this paper P0847R7). >> >> PR libstdc++/80564 >> >> libstdc++-v3/ChangeLog: >> >> * include/std/functional (__cv_like): New. >> (_Bind::_Res_type): Don't define when not needed. >> (_Bind::__dependent): Likewise. >> (_Bind::_Res_type_cv): Likewise. >> (_Bind::operator()) [_GLIBCXX_EXPLICIT_THIS_PARAMETER]: >> Define as two instead of four overloads using deducing >> this. >> * testsuite/20_util/bind/cv_quals_2.cc: Ignore SFINAE >> diagnostics inside headers. >> * testsuite/20_util/bind/ref_neg.cc: Likewise. >> * testsuite/20_util/bind/cv_quals_4.cc: New test. >> --- >> libstdc++-v3/include/std/functional | 81 +++++++++++++++++++ >> .../testsuite/20_util/bind/cv_quals_2.cc | 3 + >> .../testsuite/20_util/bind/cv_quals_4.cc | 39 +++++++++ >> .../testsuite/20_util/bind/ref_neg.cc | 1 + >> 4 files changed, 124 insertions(+) >> create mode 100644 libstdc++-v3/testsuite/20_util/bind/cv_quals_4.cc >> >> diff --git a/libstdc++-v3/include/std/functional >> b/libstdc++-v3/include/std/functional >> index 1928a27d3fd6..9348356f03ce 100644 >> --- a/libstdc++-v3/include/std/functional >> +++ b/libstdc++-v3/include/std/functional >> @@ -495,6 +495,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> # define _GLIBCXX_DEPR_BIND >> #endif >> >> +#if _GLIBCXX_EXPLICIT_THIS_PARAMETER >> + // Return a _Up that has the same cv-quals as _Tp. >> + template<typename _Tp, typename _Up> >> + struct __cv_like >> + { using type = _Up; }; >> + >> + template<typename _Tp, typename _Up> >> + struct __cv_like<const _Tp, _Up> >> + { using type = const _Up; }; >> + >> + template<typename _Tp, typename _Up> >> + struct __cv_like<volatile _Tp, _Up> >> + { using type = volatile _Up; }; >> + >> + template<typename _Tp, typename _Up> >> + struct __cv_like<const volatile _Tp, _Up> >> + { using type = const volatile _Up; }; >> + >> + template<typename _Tp, typename _Up> >> + using __cv_like_t = typename __cv_like<_Tp, _Up>::type; >> +#endif >> + >> /// Type of the function object returned from bind(). >> template<typename _Signature> >> class _Bind; >> @@ -564,6 +586,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> using _Res_type_impl >> = __invoke_result_t<_Fn&, _Mu_type<_BArgs, _CallArgs>&&...>; >> >> +#if !_GLIBCXX_EXPLICIT_THIS_PARAMETER >> template<typename _CallArgs> >> using _Res_type = _Res_type_impl<_Functor, _CallArgs, >> _Bound_args...>; >> >> @@ -576,6 +599,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> typename __cv_quals<__dependent<_CallArgs>>::type, >> _CallArgs, >> typename __cv_quals<_Bound_args>::type...>; >> +#endif >> >> public: >> template<typename... _Args> >> @@ -593,6 +617,62 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> _Bind(const _Bind&) = default; >> _Bind(_Bind&&) = default; >> >> +#if _GLIBCXX_EXPLICIT_THIS_PARAMETER >> +# pragma GCC diagnostic push >> +# pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr >> +# pragma GCC diagnostic ignored "-Wc++23-extensions" // deducing this >> + // Call unqualified >> + template<typename... _Args, >> + typename _Self, >> + typename _Self_nonref = typename >> remove_reference<_Self>::type, >> + __enable_if_t<!is_volatile<_Self_nonref>::value, int> = 0, >> + typename _Result >> + = _Res_type_impl<__cv_like_t<_Self_nonref, _Functor>, >> + tuple<_Args...>, >> + __cv_like_t<_Self_nonref, >> _Bound_args>...>> >> + _GLIBCXX20_CONSTEXPR >> + _Result >> + operator()(this _Self&& __self, _Args&&... __args) >> + { >> + if constexpr (is_const<_Self_nonref>::value) >> + return static_cast<const _Bind&>(__self) >> > Once you have __cv_like_t, is there any reason to not use something like > here: > using _Bind_ref_t = __cv_like_t<_Self_nonref, _Bind>&; > _Bind_ref_t(__self). > I think we also need to use c-cast, if someone privately inherited from > bind specialization, > for some reason. I thinking of version of overload: > template<typename... Funcs> > class Overload : Funcs.... > { > public: > Overload(Funcs... func) : Funcs(funcs)... {} > using Funcs::operator()....; > }; > > And then used with bind. > Could you add a test like this. > + .template __call_c<_Result>(std::forward_as_tuple >> + >> (std::forward<_Args>(__args)...), >> + _Bound_indexes()); >> + else >> + return static_cast<_Bind&>(__self) >> + .template __call<_Result>(std::forward_as_tuple >> + (std::forward<_Args>(__args)...), >> + _Bound_indexes()); >> + } >> + >> +# if defined(_GLIBCXX_VOLATILE_BIND) >> + template<typename... _Args, >> + typename _Self, >> + typename _Self_nonref = typename >> remove_reference<_Self>::type, >> + __enable_if_t<is_volatile<_Self_nonref>::value, int> = 0, >> + typename _Result >> + = _Res_type_impl<__cv_like_t<_Self_nonref, _Functor>, >> + tuple<_Args...>, >> + __cv_like_t<_Self_nonref, >> _Bound_args>...>> >> + _GLIBCXX_DEPR_BIND >> + _Result >> + operator()(this _Self&& __self, _Args&&... __args) >> + { >> + if constexpr (is_const<_Self_nonref>::value) >> + return static_cast<const volatile _Bind&>(__self) >> + .template __call_c_v<_Result>(std::forward_as_tuple >> + >> (std::forward<_Args>(__args)...), >> + _Bound_indexes()); >> + else >> + return static_cast<volatile _Bind&>(__self) >> + .template __call_v<_Result>(std::forward_as_tuple >> + >> (std::forward<_Args>(__args)...), >> + _Bound_indexes()); >> + } >> +# endif >> +# pragma GCC diagnostic pop >> +#else >> // Call unqualified >> template<typename... _Args, >> typename _Result = _Res_type<tuple<_Args...>>> >> @@ -642,6 +722,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> _Bound_indexes()); >> } >> #endif // volatile >> +#endif >> }; >> >> /// Type of the function object returned from bind<R>(). >> diff --git a/libstdc++-v3/testsuite/20_util/bind/cv_quals_2.cc >> b/libstdc++-v3/testsuite/20_util/bind/cv_quals_2.cc >> index e4c348f7a3ca..6d37cc43fd3a 100644 >> --- a/libstdc++-v3/testsuite/20_util/bind/cv_quals_2.cc >> +++ b/libstdc++-v3/testsuite/20_util/bind/cv_quals_2.cc >> @@ -44,6 +44,9 @@ void test01() >> // { dg-error "no match" "" { target c++20 } 43 } >> } >> >> +// Ignore the reasons for deduction/substitution failure in the headers. >> +// { dg-prune-output "no type named 'type' in 'struct >> std::enable_if<false" } >> + >> int main() >> { >> test01(); >> diff --git a/libstdc++-v3/testsuite/20_util/bind/cv_quals_4.cc >> b/libstdc++-v3/testsuite/20_util/bind/cv_quals_4.cc >> new file mode 100644 >> index 000000000000..365a6ff4e00f >> --- /dev/null >> +++ b/libstdc++-v3/testsuite/20_util/bind/cv_quals_4.cc >> @@ -0,0 +1,39 @@ >> +// PR libstdc++/80564 - bind on SFINAE unfriendly generic lambda >> +// { dg-do compile { target c++14 } } >> + >> +#include <functional> >> + >> +struct A >> +{ >> + template<class T> >> + auto operator()(T&) >> + { } >> + >> + template<class T> >> + auto operator()(T&) const >> + { T::fail; } >> +}; >> + >> +void >> +test01() >> +{ >> + A a; >> + std::bind(a, 0)(); // doesn't consider the const overload >> + std::bind<void>(a, 0)(); >> +} >> + >> +void >> +test02() >> +{ >> + auto f = [] (auto& x) { x = 1; }; >> + int i; >> + std::bind(f, i)(); // doesn't try const-invoking the lambda >> + std::bind<void>(f, i)(); >> +} >> + >> +int >> +main() >> +{ >> + test01(); >> + test02(); >> +} >> diff --git a/libstdc++-v3/testsuite/20_util/bind/ref_neg.cc >> b/libstdc++-v3/testsuite/20_util/bind/ref_neg.cc >> index dd47c437d426..46cc4bb330e2 100644 >> --- a/libstdc++-v3/testsuite/20_util/bind/ref_neg.cc >> +++ b/libstdc++-v3/testsuite/20_util/bind/ref_neg.cc >> @@ -51,6 +51,7 @@ void test02() >> // Ignore the reasons for deduction/substitution failure in the headers. >> // Arrange for the match to work on installed trees as well as build >> trees. >> // { dg-prune-output "no type named 'type' in 'struct >> std::__invoke_result" } >> +// { dg-prune-output "no type named 'type' in 'struct >> std::enable_if<false" } >> >> int main() >> { >> -- >> 2.52.0.154.gf0ef5b6d9b >> >>
