diff mbox series

libstdc++: fix uses of explicit object parameter [PR116038]

Message ID 20240725000433.3732987-1-ppalka@redhat.com
State New
Headers show
Series libstdc++: fix uses of explicit object parameter [PR116038] | expand

Commit Message

Patrick Palka July 25, 2024, 12:04 a.m. UTC 
Tested on x86_64-pc-linux-gnu, does this look OK for trunk/14 (after
14.2 is released)?

-- >8 --

The type of an implicit object parameter is always the current class.
For an explicit object parameter however, its deduced type can be a
class derived from the current class.  So when combining multiple
implicit-object overloads into a single explicit-object overload we need
to account for this possibility.  For example, when accessing a member
from the current class through an explicit object parameter, it may be a
derived class from which the member is not accessible, as in the below
testcases.

This pitfall is discussed[1] in the deducing this paper.  The general
solution is to cast the explicit object parameter to (a reference to)
the current class, appropriately qualified, rather than e.g. using
std::forward which preserves the deduced type.

This patch corrects the existing problematic uses of explicit object
parameters in the library, all of which forward the parameter via
std::forward, to instead cast the parameter to the current class via
our __like_t alias template.  Note that unlike the paper's like_t,
ours always returns a reference so when can just write

  __like_t<Self, B>(self)

instead of

  (_like_t<Self, B>&&)self

as the paper does.

[1]: https://wg21.link/P0847#name-lookup-within-member-functions and the
section after that

	PR libstdc++/116038

libstdc++-v3/ChangeLog:

	* include/std/functional (_Bind_front::operator()): Use __like_t
	instead of std::forward when forwarding __self.
	(_Bind_back::operator()): Likewise.
	* include/std/ranges (_Partial::operator()): Likewise.
	(_Pipe::operator()): Likewise.
	* testsuite/20_util/function_objects/bind_back/116038.cc: New test.
	* testsuite/20_util/function_objects/bind_front/116038.cc: New test.
	* testsuite/std/ranges/adaptors/116038.cc: New test.
---
 libstdc++-v3/include/std/functional           |  4 +--
 libstdc++-v3/include/std/ranges               | 11 ++++---
 .../function_objects/bind_back/116038.cc      | 27 +++++++++++++++++
 .../function_objects/bind_front/116038.cc     | 27 +++++++++++++++++
 .../testsuite/std/ranges/adaptors/116038.cc   | 29 +++++++++++++++++++
 5 files changed, 92 insertions(+), 6 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc
 create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc

Comments

Jonathan Wakely July 25, 2024, 10:51 a.m. UTC | #1 
On Thu, 25 Jul 2024 at 01:04, Patrick Palka <ppalka@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/14 (after
> 14.2 is released)?

Yes to both, thanks.


>
> -- >8 --
>
> The type of an implicit object parameter is always the current class.
> For an explicit object parameter however, its deduced type can be a
> class derived from the current class.  So when combining multiple
> implicit-object overloads into a single explicit-object overload we need
> to account for this possibility.  For example, when accessing a member
> from the current class through an explicit object parameter, it may be a
> derived class from which the member is not accessible, as in the below
> testcases.
>
> This pitfall is discussed[1] in the deducing this paper.  The general
> solution is to cast the explicit object parameter to (a reference to)
> the current class, appropriately qualified, rather than e.g. using
> std::forward which preserves the deduced type.
>
> This patch corrects the existing problematic uses of explicit object
> parameters in the library, all of which forward the parameter via
> std::forward, to instead cast the parameter to the current class via
> our __like_t alias template.  Note that unlike the paper's like_t,
> ours always returns a reference so when can just write
>
>   __like_t<Self, B>(self)
>
> instead of
>
>   (_like_t<Self, B>&&)self
>
> as the paper does.
>
> [1]: https://wg21.link/P0847#name-lookup-within-member-functions and the
> section after that
>
>         PR libstdc++/116038
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/functional (_Bind_front::operator()): Use __like_t
>         instead of std::forward when forwarding __self.
>         (_Bind_back::operator()): Likewise.
>         * include/std/ranges (_Partial::operator()): Likewise.
>         (_Pipe::operator()): Likewise.
>         * testsuite/20_util/function_objects/bind_back/116038.cc: New test.
>         * testsuite/20_util/function_objects/bind_front/116038.cc: New test.
>         * testsuite/std/ranges/adaptors/116038.cc: New test.
> ---
>  libstdc++-v3/include/std/functional           |  4 +--
>  libstdc++-v3/include/std/ranges               | 11 ++++---
>  .../function_objects/bind_back/116038.cc      | 27 +++++++++++++++++
>  .../function_objects/bind_front/116038.cc     | 27 +++++++++++++++++
>  .../testsuite/std/ranges/adaptors/116038.cc   | 29 +++++++++++++++++++
>  5 files changed, 92 insertions(+), 6 deletions(-)
>  create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc
>  create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc
>  create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc
>
> diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
> index 99364286a72..7788a963757 100644
> --- a/libstdc++-v3/include/std/functional
> +++ b/libstdc++-v3/include/std/functional
> @@ -944,7 +944,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         noexcept(is_nothrow_invocable_v<__like_t<_Self, _Fd>,
>                                         __like_t<_Self, _BoundArgs>..., _CallArgs...>)
>         {
> -         return _S_call(std::forward<_Self>(__self), _BoundIndices(),
> +         return _S_call(__like_t<_Self, _Bind_front>(__self), _BoundIndices(),
>                          std::forward<_CallArgs>(__call_args)...);
>         }
>  #else
> @@ -1072,7 +1072,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         noexcept(is_nothrow_invocable_v<__like_t<_Self, _Fd>,
>                                         _CallArgs..., __like_t<_Self, _BoundArgs>...>)
>         {
> -         return _S_call(std::forward<_Self>(__self), _BoundIndices(),
> +         return _S_call(__like_t<_Self, _Bind_back>(__self), _BoundIndices(),
>                          std::forward<_CallArgs>(__call_args)...);
>         }
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 3f335b95a08..b7c7aa36ddc 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -1033,7 +1033,7 @@ namespace views::__adaptor
>             return _Adaptor{}(std::forward<_Range>(__r),
>                               std::forward<decltype(__args)>(__args)...);
>           };
> -         return std::apply(__forwarder, std::forward<_Self>(__self)._M_args);
> +         return std::apply(__forwarder, __like_t<_Self, _Partial>(__self)._M_args);
>         }
>  #else
>        template<typename _Range>
> @@ -1082,7 +1082,10 @@ namespace views::__adaptor
>         requires __adaptor_invocable<_Adaptor, _Range, __like_t<_Self, _Arg>>
>         constexpr auto
>         operator()(this _Self&& __self, _Range&& __r)
> -       { return _Adaptor{}(std::forward<_Range>(__r), std::forward<_Self>(__self)._M_arg); }
> +       {
> +         return _Adaptor{}(std::forward<_Range>(__r),
> +                           __like_t<_Self, _Partial>(__self)._M_arg);
> +       }
>  #else
>        template<typename _Range>
>         requires __adaptor_invocable<_Adaptor, _Range, const _Arg&>
> @@ -1185,8 +1188,8 @@ namespace views::__adaptor
>         constexpr auto
>         operator()(this _Self&& __self, _Range&& __r)
>         {
> -         return (std::forward<_Self>(__self)._M_rhs
> -                 (std::forward<_Self>(__self)._M_lhs
> +         return (__like_t<_Self, _Pipe>(__self)._M_rhs
> +                 (__like_t<_Self, _Pipe>(__self)._M_lhs
>                    (std::forward<_Range>(__r))));
>         }
>  #else
> diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc b/libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc
> new file mode 100644
> index 00000000000..ed392b1434e
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc
> @@ -0,0 +1,27 @@
> +// PR libstdc++/116038
> +// { dg-do compile { target c++23 } }
> +
> +#include <functional>
> +#include <utility>
> +
> +struct A { };
> +struct B { };
> +
> +template<class... Ts>
> +struct overloaded : private Ts... {
> +  overloaded(Ts...);
> +  using Ts::operator()...;
> +};
> +
> +int apply_a(A, int);
> +int apply_b(B, int);
> +
> +int main() {
> +  overloaded o = { std::bind_back(apply_a, 1),
> +                  std::bind_back(apply_b, 2) };
> +  A a;
> +  o(a);
> +  std::as_const(o)(a);
> +  std::move(o)(a);
> +  std::move(std::as_const(o))(a);
> +}
> diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc
> new file mode 100644
> index 00000000000..3bf1226375b
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc
> @@ -0,0 +1,27 @@
> +// PR libstdc++/116038
> +// { dg-do compile { target c++20 } }
> +
> +#include <functional>
> +#include <utility>
> +
> +struct A { };
> +struct B { };
> +
> +template<class... Ts>
> +struct overloaded : private Ts... {
> +  overloaded(Ts...);
> +  using Ts::operator()...;
> +};
> +
> +int apply_a(int, A);
> +int apply_b(int, B);
> +
> +int main() {
> +  overloaded o = { std::bind_front(apply_a, 1),
> +                  std::bind_front(apply_b, 2) };
> +  A a;
> +  o(a);
> +  std::as_const(o)(a);
> +  std::move(o)(a);
> +  std::move(std::as_const(o))(a);
> +}
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc
> new file mode 100644
> index 00000000000..1afdf3d06d1
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc
> @@ -0,0 +1,29 @@
> +// PR libstdc++/116038
> +// { dg-do compile { target c++20 } }
> +
> +#include <ranges>
> +#include <utility>
> +
> +struct A { };
> +struct B { };
> +
> +template<class... Ts>
> +struct overloaded : private Ts... {
> +  overloaded(Ts...);
> +  using Ts::operator()...;
> +};
> +
> +int x[5];
> +struct integralish { operator int() const; } i;
> +
> +int main() {
> +  overloaded o1 = { std::views::drop(i) };
> +  o1(x);
> +  std::move(o1)(x);
> +  std::as_const(o1)(x);
> +
> +  overloaded o2 = { std::views::drop(i) | std::views::take(i) };
> +  o2(x);
> +  std::move(o2)(x);
> +  std::as_const(o1)(x);
> +}
> --
> 2.46.0.rc2
>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index 99364286a72..7788a963757 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -944,7 +944,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	noexcept(is_nothrow_invocable_v<__like_t<_Self, _Fd>,
 					__like_t<_Self, _BoundArgs>..., _CallArgs...>)
 	{
-	  return _S_call(std::forward<_Self>(__self), _BoundIndices(),
+	  return _S_call(__like_t<_Self, _Bind_front>(__self), _BoundIndices(),
 			 std::forward<_CallArgs>(__call_args)...);
 	}
 #else
@@ -1072,7 +1072,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	noexcept(is_nothrow_invocable_v<__like_t<_Self, _Fd>,
 					_CallArgs..., __like_t<_Self, _BoundArgs>...>)
 	{
-	  return _S_call(std::forward<_Self>(__self), _BoundIndices(),
+	  return _S_call(__like_t<_Self, _Bind_back>(__self), _BoundIndices(),
 			 std::forward<_CallArgs>(__call_args)...);
 	}
 
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 3f335b95a08..b7c7aa36ddc 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1033,7 +1033,7 @@  namespace views::__adaptor
 	    return _Adaptor{}(std::forward<_Range>(__r),
 			      std::forward<decltype(__args)>(__args)...);
 	  };
-	  return std::apply(__forwarder, std::forward<_Self>(__self)._M_args);
+	  return std::apply(__forwarder, __like_t<_Self, _Partial>(__self)._M_args);
 	}
 #else
       template<typename _Range>
@@ -1082,7 +1082,10 @@  namespace views::__adaptor
 	requires __adaptor_invocable<_Adaptor, _Range, __like_t<_Self, _Arg>>
 	constexpr auto
 	operator()(this _Self&& __self, _Range&& __r)
-	{ return _Adaptor{}(std::forward<_Range>(__r), std::forward<_Self>(__self)._M_arg); }
+	{
+	  return _Adaptor{}(std::forward<_Range>(__r),
+			    __like_t<_Self, _Partial>(__self)._M_arg);
+	}
 #else
       template<typename _Range>
 	requires __adaptor_invocable<_Adaptor, _Range, const _Arg&>
@@ -1185,8 +1188,8 @@  namespace views::__adaptor
 	constexpr auto
 	operator()(this _Self&& __self, _Range&& __r)
 	{
-	  return (std::forward<_Self>(__self)._M_rhs
-		  (std::forward<_Self>(__self)._M_lhs
+	  return (__like_t<_Self, _Pipe>(__self)._M_rhs
+		  (__like_t<_Self, _Pipe>(__self)._M_lhs
 		   (std::forward<_Range>(__r))));
 	}
 #else
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc b/libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc
new file mode 100644
index 00000000000..ed392b1434e
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_back/116038.cc
@@ -0,0 +1,27 @@ 
+// PR libstdc++/116038
+// { dg-do compile { target c++23 } }
+
+#include <functional>
+#include <utility>
+
+struct A { };
+struct B { };
+
+template<class... Ts>
+struct overloaded : private Ts... {
+  overloaded(Ts...);
+  using Ts::operator()...;
+};
+
+int apply_a(A, int);
+int apply_b(B, int);
+
+int main() {
+  overloaded o = { std::bind_back(apply_a, 1),
+		   std::bind_back(apply_b, 2) };
+  A a;
+  o(a);
+  std::as_const(o)(a);
+  std::move(o)(a);
+  std::move(std::as_const(o))(a);
+}
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc
new file mode 100644
index 00000000000..3bf1226375b
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/116038.cc
@@ -0,0 +1,27 @@ 
+// PR libstdc++/116038
+// { dg-do compile { target c++20 } }
+
+#include <functional>
+#include <utility>
+
+struct A { };
+struct B { };
+
+template<class... Ts>
+struct overloaded : private Ts... {
+  overloaded(Ts...);
+  using Ts::operator()...;
+};
+
+int apply_a(int, A);
+int apply_b(int, B);
+
+int main() {
+  overloaded o = { std::bind_front(apply_a, 1),
+		   std::bind_front(apply_b, 2) };
+  A a;
+  o(a);
+  std::as_const(o)(a);
+  std::move(o)(a);
+  std::move(std::as_const(o))(a);
+}
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc
new file mode 100644
index 00000000000..1afdf3d06d1
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/116038.cc
@@ -0,0 +1,29 @@ 
+// PR libstdc++/116038
+// { dg-do compile { target c++20 } }
+
+#include <ranges>
+#include <utility>
+
+struct A { };
+struct B { };
+
+template<class... Ts>
+struct overloaded : private Ts... {
+  overloaded(Ts...);
+  using Ts::operator()...;
+};
+
+int x[5];
+struct integralish { operator int() const; } i;
+
+int main() {
+  overloaded o1 = { std::views::drop(i) };
+  o1(x);
+  std::move(o1)(x);
+  std::as_const(o1)(x);
+
+  overloaded o2 = { std::views::drop(i) | std::views::take(i) };
+  o2(x);
+  std::move(o2)(x);
+  std::as_const(o1)(x);
+}