https://gcc.gnu.org/g:1be3e4e43839d313364ffa99012ada41b4fae8da

commit r14-10705-g1be3e4e43839d313364ffa99012ada41b4fae8da
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Wed Jul 10 23:14:19 2024 +0100

    libstdc++: Fix std::allocator_traits::construct constraints [PR108619]
    
    Using std::is_constructible in the constraints introduces a spurious
    dependency on the type being destructible, which should not be required
    for constructing with an allocator. The test case shows a case where the
    type has a private destructor, which can be destroyed by the allocator,
    but std::is_destructible and std::is_constructible are false.
    
    Similarly, using is_nothrow_constructible in the noexcept-specifiers
    for the construct members of allocator_traits and std::allocator,
    __gnu_cxx::__new_allocator, and __gnu_cxx::__malloc_allocator gives the
    wrong answer if the type isn't destructible.
    We need a new type trait to define those correctly, so that we only
    check if the placement new-expression is nothrow after using
    is_constructible to check that it would be well-formed.
    
    On trunk all members of std::allocator_traits were rewritten in terms of
    'if constexpr' using variable templates and the detection idiom. For the
    release branch this backport only changes the 'construct' member.
    
    Although we can use 'if constexpr' and variable templates in C++11 with
    appropriate uses of diagnostic pragmas, we can't have constexpr
    functions with multiple return statements. This means that in C++11 mode
    the _S_nothrow_construct helper used for noexcept-specifiers still needs
    to be a pair of overloads using enable_if.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/108619
            * include/bits/alloc_traits.h (__allocator_traits_base): Add
            variable templates for detecting whether the allocator has a
            construct member, or if placement new can be used instead.
            (allocator_traits::__construct_helper): Remove.
            (allocator_traits::__has_construct): Remove.
            (allocator_traits::construct): Use 'if constexpr' instead of
            dispatching to overloads constrained with enable_if.
            (allocator_traits<allocator<T>>::construct): Use _Construct if
            construct_at is not supported. Use
            __is_nothrow_new_constructible for noexcept-specifier.
            (allocator_traits<allocator<void>>::construct): Use
            __is_nothrow_new_constructible for noexcept-specifier.
            * include/bits/new_allocator.h (construct): Likewise.
            * include/ext/malloc_allocator.h (construct): Likewise.
            * include/std/type_traits (__is_nothrow_new_constructible): New
            variable template.
            * testsuite/20_util/allocator/89510.cc: Adjust expected results.
            * testsuite/ext/malloc_allocator/89510.cc: Likewise.
            * testsuite/ext/new_allocator/89510.cc: Likewise.
            * testsuite/20_util/allocator_traits/members/108619.cc: New test.
    
    (cherry picked from commit 8cf51d7516b92b352c358c14ab4e456ae53c3371)

Diff:
---
 libstdc++-v3/include/bits/alloc_traits.h           | 131 +++++++++++++--------
 libstdc++-v3/include/bits/new_allocator.h          |   2 +-
 libstdc++-v3/include/ext/malloc_allocator.h        |   2 +-
 libstdc++-v3/include/std/type_traits               |  15 +++
 libstdc++-v3/testsuite/20_util/allocator/89510.cc  |  14 +--
 .../20_util/allocator_traits/members/108619.cc     |  35 ++++++
 .../testsuite/ext/malloc_allocator/89510.cc        |  14 +--
 libstdc++-v3/testsuite/ext/new_allocator/89510.cc  |  14 +--
 8 files changed, 154 insertions(+), 73 deletions(-)

diff --git a/libstdc++-v3/include/bits/alloc_traits.h 
b/libstdc++-v3/include/bits/alloc_traits.h
index 82fc79c7b9f9..a81b286eee70 100644
--- a/libstdc++-v3/include/bits/alloc_traits.h
+++ b/libstdc++-v3/include/bits/alloc_traits.h
@@ -48,6 +48,11 @@ namespace std _GLIBCXX_VISIBILITY(default)
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #if __cplusplus >= 201103L
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++14-extensions" // for variable templates
+#pragma GCC diagnostic ignored "-Wc++17-extensions" // for if-constexpr
+
   /// @cond undocumented
   struct __allocator_traits_base
   {
@@ -89,6 +94,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       using __pocs = typename _Tp::propagate_on_container_swap;
     template<typename _Tp>
       using __equal = __type_identity<typename _Tp::is_always_equal>;
+
+    // __has_construct is true if a.construct(p, args...) is well-formed.
+    // __can_construct is true if either __has_construct is true, or if
+    // a placement new-expression for T(args...) is well-formed. We use this
+    // to constrain allocator_traits::construct, as a libstdc++ extension.
+    template<typename _Alloc, typename _Tp, typename... _Args>
+      using __construct_t
+       = decltype(std::declval<_Alloc&>().construct(std::declval<_Tp*>(),
+                                                    std::declval<_Args>()...));
+    template<typename _Alloc, typename _Tp, typename, typename... _Args>
+      static constexpr bool __has_construct_impl = false;
+    template<typename _Alloc, typename _Tp, typename... _Args>
+      static constexpr bool
+      __has_construct_impl<_Alloc, _Tp,
+                          __void_t<__construct_t<_Alloc, _Tp, _Args...>>,
+                          _Args...>
+       = true;
+    template<typename _Alloc, typename _Tp, typename... _Args>
+      static constexpr bool __has_construct
+       = __has_construct_impl<_Alloc, _Tp, void, _Args...>;
+    template<typename _Tp, typename... _Args>
+      using __new_expr_t
+       = decltype(::new((void*)0) _Tp(std::declval<_Args>()...));
+    template<typename _Tp, typename, typename... _Args>
+      static constexpr bool __has_new_expr = false;
+    template<typename _Tp, typename... _Args>
+      static constexpr bool
+      __has_new_expr<_Tp, __void_t<__new_expr_t<_Tp, _Args...>>, _Args...>
+       = true;
+    template<typename _Alloc, typename _Tp, typename... _Args>
+      static constexpr bool __can_construct
+       = __has_construct<_Alloc, _Tp, _Args...>
+           || __has_new_expr<_Tp, void, _Args...>;
   };
 
   template<typename _Alloc, typename _Up>
@@ -242,43 +280,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        _S_allocate(_Alloc2& __a, size_type __n, const_void_pointer, ...)
        { return __a.allocate(__n); }
 
-      template<typename _Tp, typename... _Args>
-       struct __construct_helper
-       {
-         template<typename _Alloc2,
-           typename = decltype(std::declval<_Alloc2*>()->construct(
-                 std::declval<_Tp*>(), std::declval<_Args>()...))>
-           static true_type __test(int);
-
-         template<typename>
-           static false_type __test(...);
-
-         using type = decltype(__test<_Alloc>(0));
-       };
-
-      template<typename _Tp, typename... _Args>
-       using __has_construct
-         = typename __construct_helper<_Tp, _Args...>::type;
-
-      template<typename _Tp, typename... _Args>
-       static _GLIBCXX14_CONSTEXPR _Require<__has_construct<_Tp, _Args...>>
-       _S_construct(_Alloc& __a, _Tp* __p, _Args&&... __args)
-       noexcept(noexcept(__a.construct(__p, std::forward<_Args>(__args)...)))
-       { __a.construct(__p, std::forward<_Args>(__args)...); }
-
-      template<typename _Tp, typename... _Args>
-       static _GLIBCXX14_CONSTEXPR
-       _Require<__and_<__not_<__has_construct<_Tp, _Args...>>,
-                              is_constructible<_Tp, _Args...>>>
-       _S_construct(_Alloc&, _Tp* __p, _Args&&... __args)
-       noexcept(std::is_nothrow_constructible<_Tp, _Args...>::value)
-       {
-#if __cplusplus <= 201703L
-         ::new((void*)__p) _Tp(std::forward<_Args>(__args)...);
-#else
-         std::construct_at(__p, std::forward<_Args>(__args)...);
-#endif
-       }
 
       template<typename _Alloc2, typename _Tp>
        static _GLIBCXX14_CONSTEXPR auto
@@ -372,12 +373,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *  arguments @a __args...
       */
       template<typename _Tp, typename... _Args>
-       static _GLIBCXX20_CONSTEXPR auto
+       static _GLIBCXX20_CONSTEXPR
+       __enable_if_t<__can_construct<_Alloc, _Tp, _Args...>>
        construct(_Alloc& __a, _Tp* __p, _Args&&... __args)
-       noexcept(noexcept(_S_construct(__a, __p,
-                                      std::forward<_Args>(__args)...)))
-       -> decltype(_S_construct(__a, __p, std::forward<_Args>(__args)...))
-       { _S_construct(__a, __p, std::forward<_Args>(__args)...); }
+       noexcept(_S_nothrow_construct<_Tp, _Args...>())
+       {
+         if constexpr (__has_construct<_Alloc, _Tp, _Args...>)
+           __a.construct(__p, std::forward<_Args>(__args)...);
+         else
+           std::_Construct(__p, std::forward<_Args>(__args)...);
+       }
 
       /**
        *  @brief  Destroy an object of type @a _Tp
@@ -416,7 +421,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static _GLIBCXX20_CONSTEXPR _Alloc
       select_on_container_copy_construction(const _Alloc& __rhs)
       { return _S_select(__rhs, 0); }
+
+    private:
+#if __cpp_constexpr >= 201304 // >= C++14
+      template<typename _Tp, typename... _Args>
+       static constexpr bool
+       _S_nothrow_construct(_Alloc* __a = nullptr, _Tp* __p = nullptr)
+       {
+         if constexpr (__has_construct<_Alloc, _Tp, _Args...>)
+           return noexcept(__a->construct(__p, std::declval<_Args>()...));
+         else
+           return __is_nothrow_new_constructible<_Tp, _Args...>;
+       }
+#else
+      template<typename _Tp, typename... _Args>
+       static constexpr
+       __enable_if_t<__has_construct<_Alloc, _Tp, _Args...>, bool>
+       _S_nothrow_construct(_Alloc* __a = nullptr, _Tp* __p = nullptr)
+       { return noexcept(__a->construct(__p, std::declval<_Args>()...)); }
+
+      template<typename _Tp, typename... _Args>
+       static constexpr
+       __enable_if_t<!__has_construct<_Alloc, _Tp, _Args...>, bool>
+       _S_nothrow_construct(_Alloc* = nullptr, _Tp* __p = nullptr)
+       { return __is_nothrow_new_constructible<_Tp, _Args...>; }
+#endif
     };
+#pragma GCC diagnostic pop
 
 #if _GLIBCXX_HOSTED
   /// Partial specialization for std::allocator.
@@ -526,14 +557,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       template<typename _Up, typename... _Args>
        [[__gnu__::__always_inline__]]
        static _GLIBCXX20_CONSTEXPR void
-       construct(allocator_type& __a __attribute__((__unused__)), _Up* __p,
-                 _Args&&... __args)
-       noexcept(std::is_nothrow_constructible<_Up, _Args...>::value)
+       construct(allocator_type& __a __attribute__((__unused__)),
+                 _Up* __p, _Args&&... __args)
+#if __cplusplus <= 201703L
+       noexcept(noexcept(__a.construct(__p, std::forward<_Args>(__args)...)))
+#else
+       noexcept(__is_nothrow_new_constructible<_Up, _Args...>)
+#endif
        {
 #if __cplusplus <= 201703L
          __a.construct(__p, std::forward<_Args>(__args)...);
-#else
+#elif __cpp_constexpr_dynamic_alloc // >= C++20
          std::construct_at(__p, std::forward<_Args>(__args)...);
+#else
+         std::_Construct(__p, std::forward<_Args>(__args)...);
 #endif
        }
 
@@ -653,7 +690,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        [[__gnu__::__always_inline__]]
        static _GLIBCXX20_CONSTEXPR void
        construct(allocator_type&, _Up* __p, _Args&&... __args)
-       noexcept(std::is_nothrow_constructible<_Up, _Args...>::value)
+       noexcept(__is_nothrow_new_constructible<_Up, _Args...>)
        { std::_Construct(__p, std::forward<_Args>(__args)...); }
 
       /**
diff --git a/libstdc++-v3/include/bits/new_allocator.h 
b/libstdc++-v3/include/bits/new_allocator.h
index 0e90c8819acd..21e7e22cae00 100644
--- a/libstdc++-v3/include/bits/new_allocator.h
+++ b/libstdc++-v3/include/bits/new_allocator.h
@@ -187,7 +187,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        __attribute__((__always_inline__))
        void
        construct(_Up* __p, _Args&&... __args)
-       noexcept(std::is_nothrow_constructible<_Up, _Args...>::value)
+       noexcept(__is_nothrow_new_constructible<_Up, _Args...>)
        { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
 
       template<typename _Up>
diff --git a/libstdc++-v3/include/ext/malloc_allocator.h 
b/libstdc++-v3/include/ext/malloc_allocator.h
index b0a87b62d5ac..cde06d83241a 100644
--- a/libstdc++-v3/include/ext/malloc_allocator.h
+++ b/libstdc++-v3/include/ext/malloc_allocator.h
@@ -161,7 +161,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       template<typename _Up, typename... _Args>
         void
         construct(_Up* __p, _Args&&... __args)
-       noexcept(std::is_nothrow_constructible<_Up, _Args...>::value)
+       noexcept(std::__is_nothrow_new_constructible<_Up, _Args...>)
        { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
 
       template<typename _Up>
diff --git a/libstdc++-v3/include/std/type_traits 
b/libstdc++-v3/include/std/type_traits
index b441bf9908f7..fe5f4ee1bade 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -1597,6 +1597,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 #endif // __cpp_lib_is_nothrow_convertible
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++14-extensions" // for variable templates
+  template<typename _Tp, typename... _Args>
+    struct __is_nothrow_new_constructible_impl
+    : __bool_constant<
+       noexcept(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...))
+      >
+    { };
+
+  template<typename _Tp, typename... _Args>
+    _GLIBCXX17_INLINE constexpr bool __is_nothrow_new_constructible
+      = __and_<is_constructible<_Tp, _Args...>,
+              __is_nothrow_new_constructible_impl<_Tp, _Args...>>::value;
+#pragma GCC diagnostic pop
+
   // Const-volatile modifications.
 
   /// remove_const
diff --git a/libstdc++-v3/testsuite/20_util/allocator/89510.cc 
b/libstdc++-v3/testsuite/20_util/allocator/89510.cc
index 95c85d2634ee..91526462a096 100644
--- a/libstdc++-v3/testsuite/20_util/allocator/89510.cc
+++ b/libstdc++-v3/testsuite/20_util/allocator/89510.cc
@@ -136,13 +136,11 @@ struct Z
 };
 
 Z* zp;
-// These construct calls should be noexcept, but they are false because
-// they use is_nothrow_constructible which depends on is_nothrow_destructible.
 #if __cplusplus <= 201703L
-static_assert( ! noexcept(a.construct(zp)), "wrong" );
-static_assert( ! noexcept(a.construct(zp, 1)), "wrong" );
-static_assert( ! noexcept(a.destroy(zp)), "" );
+static_assert( noexcept(a.construct(zp)), "" );
+static_assert( noexcept(a.construct(zp, 1)), "" );
+static_assert( ! noexcept(a.destroy(zp)), "~Z is noexcept(false)" );
 #endif
-static_assert( ! noexcept(AT::construct(a, zp)), "" );
-static_assert( ! noexcept(AT::construct(a, zp, 1)), "" );
-static_assert( ! noexcept(AT::destroy(a, zp)), "" );
+static_assert( noexcept(AT::construct(a, zp)), "" );
+static_assert( noexcept(AT::construct(a, zp, 1)), "" );
+static_assert( ! noexcept(AT::destroy(a, zp)), "~Z is noexcept(false)" );
diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/members/108619.cc 
b/libstdc++-v3/testsuite/20_util/allocator_traits/members/108619.cc
new file mode 100644
index 000000000000..01bf611b8048
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/allocator_traits/members/108619.cc
@@ -0,0 +1,35 @@
+// { dg-do compile { target c++11 } }
+
+#include <memory>
+
+template<typename T>
+struct Alloc
+{
+  Alloc() = default;
+
+  template<typename U> Alloc(const Alloc<U>&) { }
+
+  using value_type = T;
+
+  T* allocate(unsigned n)
+  { return std::allocator<T>().allocate(n); }
+
+  void deallocate(T* p, unsigned n)
+  { return std::allocator<T>().deallocate(p, n); }
+
+  template<typename U> void destroy(U* p){ p->~U(); }
+};
+
+
+class S
+{
+  ~S() = default;
+
+  friend Alloc<S>;
+};
+
+void
+test_pr108619(Alloc<int> a, S* p)
+{
+  std::allocator_traits<Alloc<int>>::construct(a, p);
+}
diff --git a/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc 
b/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc
index 1889c88d6e5a..771facbdf749 100644
--- a/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc
+++ b/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc
@@ -137,13 +137,11 @@ struct Z
 };
 
 Z* zp;
-// These construct calls should be noexcept, but they are false because
-// they use is_nothrow_constructible which depends on is_nothrow_destructible.
 #if __cplusplus <= 201703L
-static_assert( ! noexcept(a.construct(zp)), "wrong" );
-static_assert( ! noexcept(a.construct(zp, 1)), "wrong" );
-static_assert( ! noexcept(a.destroy(zp)), "" );
+static_assert( noexcept(a.construct(zp)), "" );
+static_assert( noexcept(a.construct(zp, 1)), "" );
+static_assert( ! noexcept(a.destroy(zp)), "~Z is noexcept(false)" );
 #endif
-static_assert( ! noexcept(AT::construct(a, zp)), "" );
-static_assert( ! noexcept(AT::construct(a, zp, 1)), "" );
-static_assert( ! noexcept(AT::destroy(a, zp)), "" );
+static_assert( noexcept(AT::construct(a, zp)), "" );
+static_assert( noexcept(AT::construct(a, zp, 1)), "" );
+static_assert( ! noexcept(AT::destroy(a, zp)), "~Z is noexcept(false)" );
diff --git a/libstdc++-v3/testsuite/ext/new_allocator/89510.cc 
b/libstdc++-v3/testsuite/ext/new_allocator/89510.cc
index 06384ae55700..7fc443831c96 100644
--- a/libstdc++-v3/testsuite/ext/new_allocator/89510.cc
+++ b/libstdc++-v3/testsuite/ext/new_allocator/89510.cc
@@ -137,13 +137,11 @@ struct Z
 };
 
 Z* zp;
-// These construct calls should be noexcept, but they are false because
-// they use is_nothrow_constructible which depends on is_nothrow_destructible.
 #if __cplusplus <= 201703L
-static_assert( ! noexcept(a.construct(zp)), "wrong" );
-static_assert( ! noexcept(a.construct(zp, 1)), "wrong" );
-static_assert( ! noexcept(a.destroy(zp)), "" );
+static_assert( noexcept(a.construct(zp)), "" );
+static_assert( noexcept(a.construct(zp, 1)), "" );
+static_assert( ! noexcept(a.destroy(zp)), "~Z is noexcept(false)" );
 #endif
-static_assert( ! noexcept(AT::construct(a, zp)), "" );
-static_assert( ! noexcept(AT::construct(a, zp, 1)), "" );
-static_assert( ! noexcept(AT::destroy(a, zp)), "" );
+static_assert( noexcept(AT::construct(a, zp)), "" );
+static_assert( noexcept(AT::construct(a, zp, 1)), "" );
+static_assert( ! noexcept(AT::destroy(a, zp)), "~Z is noexcept(false)" );

Reply via email to