On Wed, 11 Dec 2024 at 14:59, Arsen Arsenović <ar...@aarsen.me> wrote:
>
> Jonathan Wakely <jwak...@redhat.com> writes:
>
> >> +  for (auto _ : gen("const This& argument", std::allocator_arg , &mr))
> >
> > I have no idea what the promise_type::operator new overload that takes
> > a const This& argument is for, but this invocation of gen uses it, so
> > ... yay?
>
> Member coroutines can call that overload:
>
>   struct foo {
>     std::generator<int, void, std::allocator<foo>>
>     bar(std::allocator_arg_t, std::allocator<foo>) const&
>     { co_yield 123; }
>   };
>
>   void f() {
>       foo x;
>       x.bar(std::allocator_arg, {});
>   }
>

Yeah, I figured that out eventually!

Here's a v2 patch that adds the static assert messages and now also
tests allocation for member coroutines (with implicit and explicit
object parameters).
commit 220d5bd2d1ff38dce3a8a7dfe7cea476126ff5a4
Author:     Jonathan Wakely <jwak...@redhat.com>
AuthorDate: Wed Dec 11 10:44:33 2024
Commit:     Jonathan Wakely <r...@gcc.gnu.org>
CommitDate: Wed Dec 11 15:23:56 2024

    libstdc++: Remove constraints on std::generator::promise_type::operator new
    
    This was approved in Wrocław as LWG 3900, so that passing an incorrect
    argument intended as an allocator will be ill-formed, instead of
    silently ignored.
    
    This also renames the template parameters and function parameters for
    the allocators, to align with the names in the standard. I found it too
    confusing to have a parameter _Alloc which doesn't correspond to Alloc
    in the standard. Rename _Alloc to _Allocator (which the standard calls
    Allocator) and rename _Na to _Alloc (which the standard calls Alloc).
    
    libstdc++-v3/ChangeLog:
    
            * include/std/generator (_Promise_alloc): Rename template
            parameter. Use __alloc_rebind to rebind allocator.
            (_Promise_alloc::operator new): Replace constraints with a
            static_assert in the body. Rename allocator parameter.
            (_Promise_alloc<void>::_M_allocate): Rename allocator parameter.
            Use __alloc_rebind to rebind allocator.
            (_Promise_alloc<void>::operator new): Rename allocator
            parameter.
            * testsuite/24_iterators/range_generators/alloc.cc: New test.
            * testsuite/24_iterators/range_generators/lwg3900.cc: New test.
    
    Reviewed-by: Arsen Arsenović <ar...@aarsen.me>

diff --git a/libstdc++-v3/include/std/generator 
b/libstdc++-v3/include/std/generator
index 0a14e70064e..bba85bd0aa4 100644
--- a/libstdc++-v3/include/std/generator
+++ b/libstdc++-v3/include/std/generator
@@ -416,13 +416,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     concept _Stateless_alloc = (allocator_traits<_All>::is_always_equal::value
                                && default_initializable<_All>);
 
-    template<typename _Alloc>
+    template<typename _Allocator>
       class _Promise_alloc
       {
-       using _ATr = allocator_traits<_Alloc>;
-       using _Rebound = typename _ATr::template rebind_alloc<_Alloc_block>;
-       using _Rebound_ATr = typename _ATr
-         ::template rebind_traits<_Alloc_block>;
+       using _Rebound = __alloc_rebind<_Allocator, _Alloc_block>;
+       using _Rebound_ATr = allocator_traits<_Rebound>;
        static_assert(is_pointer_v<typename _Rebound_ATr::pointer>,
                      "Must use allocators for true pointers with generators");
 
@@ -465,30 +463,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       public:
        void*
        operator new(std::size_t __sz)
-         requires default_initializable<_Rebound> // _Alloc is non-void
+         requires default_initializable<_Rebound> // _Allocator is non-void
        { return _M_allocate({}, __sz); }
 
-       template<typename _Na, typename... _Args>
+       // _GLIBCXX_RESOLVE_LIB_DEFECTS
+       // 3900. The allocator_arg_t overloads of promise_type::operator new
+       // should not be constrained
+       template<typename _Alloc, typename... _Args>
        void*
        operator new(std::size_t __sz,
-                    allocator_arg_t, const _Na& __na,
+                    allocator_arg_t, const _Alloc& __a,
                     const _Args&...)
-         requires convertible_to<const _Na&, _Alloc>
        {
-         return _M_allocate(static_cast<_Rebound>(static_cast<_Alloc>(__na)),
-                            __sz);
+         static_assert(convertible_to<const _Alloc&, _Allocator>,
+                       "the allocator argument to the coroutine must be "
+                       "convertible to the generator's allocator type");
+         return _M_allocate(_Rebound(_Allocator(__a)), __sz);
        }
 
-       template<typename _This, typename _Na, typename... _Args>
+       template<typename _This, typename _Alloc, typename... _Args>
        void*
        operator new(std::size_t __sz,
                     const _This&,
-                    allocator_arg_t, const _Na& __na,
+                    allocator_arg_t, const _Alloc& __a,
                     const _Args&...)
-         requires convertible_to<const _Na&, _Alloc>
        {
-         return _M_allocate(static_cast<_Rebound>(static_cast<_Alloc>(__na)),
-                            __sz);
+         static_assert(convertible_to<const _Alloc&, _Allocator>,
+                       "the allocator argument to the coroutine must be "
+                       "convertible to the generator's allocator type");
+         return _M_allocate(_Rebound(_Allocator(__a)), __sz);
        }
 
        void
@@ -578,20 +581,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
            }
        }
 
-       template<typename _Na>
+       template<typename _Alloc>
        static void*
-       _M_allocate(const _Na& __na, std::size_t __csz)
+       _M_allocate(const _Alloc& __a, std::size_t __csz)
        {
-         using _Rebound = typename std::allocator_traits<_Na>
-           ::template rebind_alloc<_Alloc_block>;
-         using _Rebound_ATr = typename std::allocator_traits<_Na>
-           ::template rebind_traits<_Alloc_block>;
+         using _Rebound = __alloc_rebind<_Alloc, _Alloc_block>;
+         using _Rebound_ATr = allocator_traits<_Rebound>;
 
          static_assert(is_pointer_v<typename _Rebound_ATr::pointer>,
                        "Must use allocators for true pointers with 
generators");
 
          _Dealloc_fn __d = &_M_deallocator<_Rebound>;
-         auto __b = static_cast<_Rebound>(__na);
+         auto __b = static_cast<_Rebound>(__a);
          auto __asz = _M_alloc_size<_Rebound>(__csz);
          auto __nblk = _Alloc_block::_M_cnt(__asz);
          void* __p = __b.allocate(__nblk);
@@ -619,20 +620,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
          return __p;
        }
 
-       template<typename _Na, typename... _Args>
+       template<typename _Alloc, typename... _Args>
        void*
        operator new(std::size_t __sz,
-                    allocator_arg_t, const _Na& __na,
+                    allocator_arg_t, const _Alloc& __a,
                     const _Args&...)
-       { return _M_allocate(__na, __sz); }
+       { return _M_allocate(__a, __sz); }
 
-       template<typename _This, typename _Na, typename... _Args>
+       template<typename _This, typename _Alloc, typename... _Args>
        void*
        operator new(std::size_t __sz,
                     const _This&,
-                    allocator_arg_t, const _Na& __na,
+                    allocator_arg_t, const _Alloc& __a,
                     const _Args&...)
-       { return _M_allocate(__na, __sz); }
+       { return _M_allocate(__a, __sz); }
 
        void
        operator delete(void* __ptr, std::size_t __sz) noexcept
diff --git a/libstdc++-v3/testsuite/24_iterators/range_generators/alloc.cc 
b/libstdc++-v3/testsuite/24_iterators/range_generators/alloc.cc
new file mode 100644
index 00000000000..ad154cfc000
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/range_generators/alloc.cc
@@ -0,0 +1,71 @@
+// { dg-do run { target c++23 } }
+
+#include <generator>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+template<typename... Args>
+std::pmr::generator<int>
+gen(Args...)
+{
+  co_yield 1;
+  co_yield 2;
+}
+
+struct S
+{
+  template<typename... Args>
+  std::pmr::generator<char>
+  gen(Args...)
+  {
+    co_yield '1';
+    co_yield '2';
+  }
+
+  template<typename Self, typename... Args>
+  std::pmr::generator<long long>
+  genx(this Self& self, Args...)
+  {
+    co_yield 1LL;
+    co_yield 2LL;
+  }
+};
+
+int
+main()
+{
+  __gnu_test::memory_resource mr;
+  for (auto _ : gen())
+    VERIFY(mr.number_of_active_allocations() == 0);
+
+  for (auto _ : gen(std::allocator_arg))
+    VERIFY(mr.number_of_active_allocations() == 0);
+
+  for (auto _ : gen(std::allocator_arg, std::pmr::new_delete_resource()))
+    VERIFY(mr.number_of_active_allocations() == 0);
+
+#if __cpp_exceptions
+  try {
+    for (auto _ : gen(std::allocator_arg, std::pmr::null_memory_resource()))
+      VERIFY(false);
+  } catch (const std::bad_alloc&) {
+  }
+#endif
+
+  VERIFY(mr.number_of_active_allocations() == 0);
+
+  for (auto _ : gen(std::allocator_arg , &mr))
+    VERIFY(mr.number_of_active_allocations() == 1);
+
+  VERIFY(mr.number_of_active_allocations() == 0);
+
+  S s;
+  for (auto _ : s.gen(std::allocator_arg , &mr))
+    VERIFY(mr.number_of_active_allocations() == 1);
+
+  VERIFY(mr.number_of_active_allocations() == 0);
+  for (auto _ : s.genx(std::allocator_arg , &mr))
+    VERIFY(mr.number_of_active_allocations() == 1);
+
+  VERIFY(mr.number_of_active_allocations() == 0);
+}
diff --git a/libstdc++-v3/testsuite/24_iterators/range_generators/lwg3900.cc 
b/libstdc++-v3/testsuite/24_iterators/range_generators/lwg3900.cc
new file mode 100644
index 00000000000..957879e8862
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/range_generators/lwg3900.cc
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++23 } }
+
+// LWG 3900. allocator_arg_t overloads of generator::promise_type::operator new
+// should not be constrained
+
+#include <generator>
+#include <memory_resource>
+
+std::pmr::generator<int>
+bar(std::allocator_arg_t, std::pmr::memory_resource& mr) // { dg-error "here" }
+{
+  co_yield 3;
+}
+
+// { dg-error "static assertion failed" "" { target *-*-* } 0 }
+// { dg-error "no matching function .*memory_resource&" "" { target *-*-* } 0 }

Reply via email to