[committed] libstdc++: Allow unique_ptr<Incomplete[]>::operator[] [PR 101236]

[committed] libstdc++: Allow unique_ptr<Incomplete[]>::operator[] [PR 101236]

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1218 bytes --]

PR libstdc++/101236 shows that LLVM depends on being able to use
unique_ptr<T[]>::operator[] when T is incomplete. This is undefined, but
previously worked with libstdc++. When I added the conditional noexcept
to that operator we started to diagnose the incomplete type.

This change restores support for that case, by making the noexcept
condition check that the type is complete before checking whether
indexing on the pointer can throw.  A workaround for PR c++/101239 is
needed to avoid a bogus error where G++ fails to do SFINAE on the
ill-formed p[n] expression and gets an ICE. Instead of checking that the
p[n] expression is valid in the trailing-return-type, we only check that
the element_type is complete.

libstdc++-v3/ChangeLog:

	PR libstdc++/101236
	* include/bits/unique_ptr.h (unique_ptr<T[], D>::operator[]):
	Fail gracefully if element_type is incomplete.
	* testsuite/20_util/unique_ptr/cons/incomplete.cc: Clarify that
	the standard doesn't require this test to work for array types.
	* testsuite/20_util/unique_ptr/lwg2762.cc: Check that incomplete
	types can be used with array specialization.
	* testsuite/20_util/unique_ptr/101236.cc: New test.

Tested powerpc64le-linux. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 6497 bytes --]

commit b7a89c041aa1d67654f1ba7b2839e221c3e14748
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jun 28 12:59:19 2021

    libstdc++: Allow unique_ptr<Incomplete[]>::operator[] [PR 101236]
    
    PR libstdc++/101236 shows that LLVM depends on being able to use
    unique_ptr<T[]>::operator[] when T is incomplete. This is undefined, but
    previously worked with libstdc++. When I added the conditional noexcept
    to that operator we started to diagnose the incomplete type.
    
    This change restores support for that case, by making the noexcept
    condition check that the type is complete before checking whether
    indexing on the pointer can throw.  A workaround for PR c++/101239 is
    needed to avoid a bogus error where G++ fails to do SFINAE on the
    ill-formed p[n] expression and gets an ICE. Instead of checking that the
    p[n] expression is valid in the trailing-return-type, we only check that
    the element_type is complete.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101236
            * include/bits/unique_ptr.h (unique_ptr<T[], D>::operator[]):
            Fail gracefully if element_type is incomplete.
            * testsuite/20_util/unique_ptr/cons/incomplete.cc: Clarify that
            the standard doesn't require this test to work for array types.
            * testsuite/20_util/unique_ptr/lwg2762.cc: Check that incomplete
            types can be used with array specialization.
            * testsuite/20_util/unique_ptr/101236.cc: New test.

diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index 1781fe15649..e478056c755 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -491,6 +491,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  = __and_< is_base_of<_Tp, _Up>,
 		    __not_<is_same<__remove_cv<_Tp>, __remove_cv<_Up>>> >;
 
+      // This checks whether p[n] is noexcept, but fails gracefully when
+      // element_type is incomplete. The standard requires a complete type
+      // for unique_ptr<T[], D>, but we try to support it anyway (PR 101236).
+      template<typename _Ptr, typename _Elt>
+	static constexpr auto
+	_S_nothrow_deref(size_t __n)
+	-> decltype(sizeof(_Elt) != 0) // PR c++/101239
+	{ return noexcept(std::declval<_Ptr>()[__n]); }
+
+      template<typename _Ptr, typename _Elt>
+	static constexpr bool
+	_S_nothrow_deref(...)
+	{ return false; }
+
     public:
       using pointer	  = typename __uniq_ptr_impl<_Tp, _Dp>::pointer;
       using element_type  = _Tp;
@@ -655,7 +669,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       /// Access an element of owned array.
       typename std::add_lvalue_reference<element_type>::type
       operator[](size_t __i) const
-      noexcept(noexcept(std::declval<pointer>()[std::declval<size_t&>()]))
+      noexcept(_S_nothrow_deref<pointer, element_type>(0))
       {
 	__glibcxx_assert(get() != pointer());
 	return get()[__i];
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/101236.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/101236.cc
new file mode 100644
index 00000000000..2f55f4baf9a
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/101236.cc
@@ -0,0 +1,13 @@
+// { dg-do compile { target c++11 } }
+#include <memory>
+
+struct Incomplete;
+struct pr101236
+{
+  // The standard says "T shall be a complete type" for unique_ptr<T[], D>
+  // so this is a GCC extension.
+  std::unique_ptr<Incomplete[]> p;
+
+  Incomplete& f() { return p[0]; }
+};
+struct Incomplete { };
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/incomplete.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/incomplete.cc
index 879a1d021a1..6b55d5744ed 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/cons/incomplete.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/cons/incomplete.cc
@@ -24,9 +24,17 @@ struct Incomplete;
 void f(void** p)
 {
   ::new (p[0]) std::unique_ptr<Incomplete>();
-  ::new (p[1]) std::unique_ptr<Incomplete[]>();
 
   // PR libstdc++/87704
-  ::new (p[2]) std::unique_ptr<Incomplete>(nullptr);
-  ::new (p[3]) std::unique_ptr<Incomplete[]>(nullptr);
+  ::new (p[1]) std::unique_ptr<Incomplete>(nullptr);
+}
+
+// The standard says "T shall be a complete type" for unique_ptr<T[], D>
+// so this is a GCC extension.
+void f_array(void** p)
+{
+  ::new (p[0]) std::unique_ptr<Incomplete[]>();
+
+  // PR libstdc++/87704
+  ::new (p[1]) std::unique_ptr<Incomplete[]>(nullptr);
 }
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc
index 3cc2ea6b87d..c88237dd9ea 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc
@@ -32,12 +32,24 @@ template<typename T, bool Nothrow>
   using UPtr = std::unique_ptr<T, deleter<Nothrow>>;
 
 // noexcept-specifier depends on the pointer type
-static_assert( noexcept(*std::declval<UPtr<int, true>&>()), "" );
-static_assert( ! noexcept(*std::declval<UPtr<int, false>&>()), "" );
+static_assert( noexcept(*std::declval<UPtr<int, true>&>()), "LWG 2762" );
+static_assert( ! noexcept(*std::declval<UPtr<int, false>&>()), "LWG 2762" );
 
 // This has always been required, even in C++11.
-static_assert( noexcept(std::declval<UPtr<int, false>&>().operator->()), "" );
+static_assert( noexcept(std::declval<std::unique_ptr<long>>().operator->()),
+	       "operator-> is always noexcept" );
+static_assert( noexcept(std::declval<UPtr<int, false>&>().operator->()),
+	       "operator-> is always noexcept" );
 
-// This is not required by the standard
-static_assert( noexcept(std::declval<UPtr<int[], true>&>()[0]), "" );
-static_assert( ! noexcept(std::declval<UPtr<int[], false>&>()[0]), "" );
+// This is not required by the standard, but we make it depend on the pointer.
+static_assert( noexcept(std::declval<std::unique_ptr<long[]>>()[0]), "QoI" );
+static_assert( noexcept(std::declval<UPtr<int[], true>&>()[0]), "QoI" );
+static_assert( ! noexcept(std::declval<UPtr<int[], false>&>()[0]), "QoI" );
+
+// This is forbidden by the standard ("T shall be a complete type")
+// but we try to support it anyway, see PR libstdc++/101236.
+struct Incomplete;
+static_assert( ! noexcept(std::declval<UPtr<Incomplete[], true>>()[0]),
+	       "this would be noexcept if the type was complete");
+static_assert( ! noexcept(std::declval<UPtr<Incomplete[], false>>()[0]),
+	       "this would still be noexcept(false) if the type was complete");

[committed] libstdc++: Revert changes to std::unique_ptr<T[]>::operator[] [PR 101271]

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

On 28/06/21 14:24 +0100, Jonathan Wakely wrote:
>PR libstdc++/101236 shows that LLVM depends on being able to use
>unique_ptr<T[]>::operator[] when T is incomplete. This is undefined, but
>previously worked with libstdc++. When I added the conditional noexcept
>to that operator we started to diagnose the incomplete type.
>
>This change restores support for that case, by making the noexcept
>condition check that the type is complete before checking whether
>indexing on the pointer can throw.  A workaround for PR c++/101239 is
>needed to avoid a bogus error where G++ fails to do SFINAE on the
>ill-formed p[n] expression and gets an ICE. Instead of checking that the
>p[n] expression is valid in the trailing-return-type, we only check that
>the element_type is complete.
>
>libstdc++-v3/ChangeLog:
>
>	PR libstdc++/101236
>	* include/bits/unique_ptr.h (unique_ptr<T[], D>::operator[]):
>	Fail gracefully if element_type is incomplete.
>	* testsuite/20_util/unique_ptr/cons/incomplete.cc: Clarify that
>	the standard doesn't require this test to work for array types.
>	* testsuite/20_util/unique_ptr/lwg2762.cc: Check that incomplete
>	types can be used with array specialization.
>	* testsuite/20_util/unique_ptr/101236.cc: New test.

This (and the previous change that it tried to fix) needs to be
reverted, as attached.

Tested powerpc64le-linux. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 4164 bytes --]

commit bc8f0ed7042280282035168428f6afc839997cf0
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jul 2 08:46:18 2021

    libstdc++: Revert changes to std::unique_ptr<T[]>::operator[] [PR 101271]
    
    This reverts the changes in r12-1778 which added a noexcept-specifier to
    std::unique_ptr<T[]>::operator[], and the changes in r12-1844 which
    tried to make it work with incomplete types (for PR 101236).
    
    The noexcept-specifier is not required by the standard, and is causing
    regressions, so just remove it.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101271
            * include/bits/unique_ptr.h (unique_ptr<T[],D>::operator[]):
            Remove noexcept-specifier.
            (unique_ptr<T[],D>::_S_nothrow_deref): Remove.
            * testsuite/20_util/unique_ptr/lwg2762.cc: Remove checks for
            operator[].

diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index e478056c755..d483f13f2b0 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -491,20 +491,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  = __and_< is_base_of<_Tp, _Up>,
 		    __not_<is_same<__remove_cv<_Tp>, __remove_cv<_Up>>> >;
 
-      // This checks whether p[n] is noexcept, but fails gracefully when
-      // element_type is incomplete. The standard requires a complete type
-      // for unique_ptr<T[], D>, but we try to support it anyway (PR 101236).
-      template<typename _Ptr, typename _Elt>
-	static constexpr auto
-	_S_nothrow_deref(size_t __n)
-	-> decltype(sizeof(_Elt) != 0) // PR c++/101239
-	{ return noexcept(std::declval<_Ptr>()[__n]); }
-
-      template<typename _Ptr, typename _Elt>
-	static constexpr bool
-	_S_nothrow_deref(...)
-	{ return false; }
-
     public:
       using pointer	  = typename __uniq_ptr_impl<_Tp, _Dp>::pointer;
       using element_type  = _Tp;
@@ -669,7 +655,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       /// Access an element of owned array.
       typename std::add_lvalue_reference<element_type>::type
       operator[](size_t __i) const
-      noexcept(_S_nothrow_deref<pointer, element_type>(0))
       {
 	__glibcxx_assert(get() != pointer());
 	return get()[__i];
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc
index c88237dd9ea..ea067eb3af3 100644
--- a/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/lwg2762.cc
@@ -12,11 +12,6 @@ struct deleter
     int& operator*() && noexcept(B);  // this is used by unique_ptr
     int& operator*() const& = delete; // this should not be
 
-    int& operator[](std::size_t) && noexcept(B); // this is used by unique_ptr
-    int& operator[](std::size_t) const& = delete; // should not be used
-    int& operator[](int) && = delete; // should not be used
-    int& operator[](double) && = delete; // should not be used
-
     int* operator->() noexcept(false); // noexcept here doesn't affect anything
 
     // Needed for NullablePointer requirements
@@ -40,16 +35,3 @@ static_assert( noexcept(std::declval<std::unique_ptr<long>>().operator->()),
 	       "operator-> is always noexcept" );
 static_assert( noexcept(std::declval<UPtr<int, false>&>().operator->()),
 	       "operator-> is always noexcept" );
-
-// This is not required by the standard, but we make it depend on the pointer.
-static_assert( noexcept(std::declval<std::unique_ptr<long[]>>()[0]), "QoI" );
-static_assert( noexcept(std::declval<UPtr<int[], true>&>()[0]), "QoI" );
-static_assert( ! noexcept(std::declval<UPtr<int[], false>&>()[0]), "QoI" );
-
-// This is forbidden by the standard ("T shall be a complete type")
-// but we try to support it anyway, see PR libstdc++/101236.
-struct Incomplete;
-static_assert( ! noexcept(std::declval<UPtr<Incomplete[], true>>()[0]),
-	       "this would be noexcept if the type was complete");
-static_assert( ! noexcept(std::declval<UPtr<Incomplete[], false>>()[0]),
-	       "this would still be noexcept(false) if the type was complete");