[Bug libstdc++/107784] New: QOI: sizeof( bind_front( Member-Function ) ) too big

[Bug libstdc++/107784] New: QOI: sizeof( bind_front( Member-Function ) ) too big

[joerg.richter@pdv-fs.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

            Bug ID: 107784
           Summary: QOI: sizeof( bind_front( Member-Function ) ) too big
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: joerg.richter@pdv-fs.de
  Target Milestone: ---

Consider this example:

----8<----
cat > t.cc <<EOF
#include <iostream>
#include <functional>

struct Foo
{
  void func()
  {}
};

int main()
{
  std::cout << sizeof( &Foo::func ) << std::endl;
  std::cout << sizeof( std::bind_front( &Foo::func ) ) << std::endl;
}
EOF

g++ -v
g++ -std=c++20 -o t t.cc
t
----8<----
Outputs 16 and 24.

sizeof(Memberfunction) is 16.
sizeof(bind_front(Memberfunction)) is 24.

The resulting functor is too big to be stored inline in a std::function. It
would be nice if the trival case of bind_front is as big as the native
member-function-pointer.

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Changing this will most likely break the abi of bind_front.

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[joerg.richter@pdv-fs.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

--- Comment #2 from Jörg Richter <joerg.richter@pdv-fs.de> ---
I think it should only change the size of std::_Bind_front.  This type should
not be used at any ABI relevant border.  But even if it is, I should be
possible to just rename the type?

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-11-21
     Ever confirmed|0                           |1

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jörg Richter from comment #2)
> I think it should only change the size of std::_Bind_front.  This type
> should not be used at any ABI relevant border.  But even if it is, I should
> be possible to just rename the type?

Right. It's an ABI change, but manageable.

The problem is just that we have an empty tuple for the bound args, which would
be solved by this (although ABI-preservation is needed as mentioned):

--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -992,7 +992,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        }

       _Fd _M_fd;
-      std::tuple<_BoundArgs...> _M_bound_args;
+      [[no_unique_address]] std::tuple<_BoundArgs...> _M_bound_args;
     };

   template<typename _Fn, typename... _Args>


But why are you using bind_front if you don't need to bind any args? I'm not
sure this is a very important use case.

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jörg Richter from comment #0)
> The resulting functor is too big to be stored inline in a std::function.

Why wouldn't you just store &Foo::func in the std::function instead?

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Complete patch:

--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -995,9 +995,68 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       std::tuple<_BoundArgs...> _M_bound_args;
     };

+  template<typename _Fd>
+    struct _Bind_front0
+    {
+      static_assert(is_move_constructible_v<_Fd>);
+
+      // First parameter is to ensure this constructor is never used
+      // instead of the copy/move constructor.
+      template<typename _Fn>
+       explicit constexpr
+       _Bind_front0(int, _Fn&& __fn)
+       noexcept(is_nothrow_constructible_v<_Fd, _Fn>)
+       : _M_fd(std::forward<_Fn>(__fn))
+       { }
+
+      _Bind_front0(const _Bind_front0&) = default;
+      _Bind_front0(_Bind_front0&&) = default;
+      _Bind_front0& operator=(const _Bind_front0&) = default;
+      _Bind_front0& operator=(_Bind_front0&&) = default;
+      ~_Bind_front0() = default;
+
+      template<typename... _CallArgs>
+       constexpr
+       invoke_result_t<_Fd&, _CallArgs...>
+       operator()(_CallArgs&&... __call_args) &
+       noexcept(is_nothrow_invocable_v<_Fd&, _CallArgs...>)
+       { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
+
+      template<typename... _CallArgs>
+       constexpr
+       invoke_result_t<const _Fd&, _CallArgs...>
+       operator()(_CallArgs&&... __call_args) const &
+       noexcept(is_nothrow_invocable_v<const _Fd&, _CallArgs...>)
+       { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
+
+      template<typename... _CallArgs>
+       constexpr
+       invoke_result_t<_Fd, _CallArgs...>
+       operator()(_CallArgs&&... __call_args) &&
+       noexcept(is_nothrow_invocable_v<_Fd, _CallArgs...>)
+       {
+         return std::invoke(std::move(_M_fd),
+                            std::forward<_CallArgs>(__call_args)...);
+       }
+
+      template<typename... _CallArgs>
+       constexpr
+       invoke_result_t<const _Fd, _CallArgs...>
+       operator()(_CallArgs&&... __call_args) const &&
+       noexcept(is_nothrow_invocable_v<const _Fd, _CallArgs...>)
+       {
+         return std::invoke(std::move(_M_fd),
+                            std::forward<_CallArgs>(__call_args)...);
+       }
+
+    private:
+      _Fd _M_fd;
+    };
+
   template<typename _Fn, typename... _Args>
     using _Bind_front_t
-      = _Bind_front<decay_t<_Fn>, decay_t<_Args>...>;
+      = __conditional_t<sizeof...(_Args) == 0, _Bind_front0<decay_t<_Fn>>,
+                       _Bind_front<decay_t<_Fn>, decay_t<_Args>...>>;

   /** Create call wrapper by partial application of arguments to function.
    *

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[joerg.richter@pdv-fs.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

--- Comment #6 from Jörg Richter <joerg.richter@pdv-fs.de> ---
It's not that I'm intentionally using bind_front without further arguments. I
just stumbled across it when I was developing code that could output the target
of a std::function<>. As I've stumpelt upon it, I might as well report it
instead of ignoring it.

In the process, I also noticed that bind_front( one, two, three ) is stored in
memory as { one, three, two }. Here it would also help me a lot if the order is
kept. But I think that would be too big of an ABI change. Too bad. But if this
is considered, I could also create another QoI PR.

Regardless, I've come across a few posts that find the recursive implementation
of std::tuple out of date. 
It would be nice if libstdc++ had a mode to enable such incompatible
enhancements, since we compile all our C++ code ourselves and thus
automatically never have ABI issues.

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It already has such a mode, and has done for many many years.
See --enable-symvers=gnu-versioned-namespace

https://gcc.gnu.org/onlinedocs/libstdc++/manual/configure.html

Nobody bothers to use it, so maintaining another std::tuple implementation for
a single digit number of users isn't worth it.

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
             Status|NEW                         |ASSIGNED
   Target Milestone|---                         |13.0

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[joerg.richter@pdv-fs.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

--- Comment #8 from Jörg Richter <joerg.richter@pdv-fs.de> ---
To be honest, I can't tell from the description that this option unlocks a 
libstdc++ that makes no allowance for ABI incompatibilities.

If you had a libstdc++ that didn't care about the ABI, similar to boost, 
this would hopefully attract more contributors and you would have something 
ready to go if an ABI break does happen. 

If you advertise such a mode in the release notes, it will surely be used by 
more than a handful of users.

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'm not interested in encouraging people to use it, that just makes my job
harder. But if you want to use it, it exists.

[Bug libstdc++/107784] QOI: sizeof( bind_front( Member-Function ) ) too big

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107784

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This was fixed in r13-4214-gcbd05ca5ab1231 but I forgot to mention the PR in
the commit.

There's no ABI concern because std::bind_front is a C++20 feature and we
haven't stabilised our C++20 ABI yet. So fixed for GCC 13, but not suitable to
backport because we don't change the ABI within a single release series, even
for unstable C++20 features.