Re: [PATCH] 2/2 Remove debug/array

[Jonathan Wakely <jwakely@redhat.com>]

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

On 09/11/20 13:07 +0000, Jonathan Wakely wrote:
>On 08/11/20 15:27 +0100, François Dumont via Libstdc++ wrote:
>>Following a recent fix on std::array this test started to fail in 
>>_GLIBCXX_DEBUG mode.
>>
>>FAIL: 23_containers/array/comparison_operators/96851.cc (test for 
>>excess errors)
>>
>>Rather than fixing it and now that __glibcxx_assert is constexpr 
>>compatible I would like to propose to simply remove 
>>__gnu_debug::array.
>>
>>The only code we are losing with this change are the 
>>_Array_check_nonempty/_Array_check_subscript types. I am not sure 
>>about the purpose of this code as I saw no impact on tests. Maybe it 
>>was to avoid assertion in constexpr where the value of the 
>>expression is not use but there is a test doing that and it does 
>>produce an assertion.
>>
>>Note that I am also moving std::array in versioned namespace. It is 
>>just for consistency so no problem to remove it.
>>
>>I also manually edited include/Makefile.in cause I do not have the 
>>proper autoreconf version. Can you regenerate it on your side once 
>>patch is in ?
>>
>>    libstdc++: Remove <debug/array>
>>
>>    Add _GLIBCXX_ASSERTIONS assert in normal std::array and remove 
>>__gnu_debug::array
>>    implementation.
>>
>>    libstdc++-v3/ChangeLog:
>>
>>            * include/debug/array: Remove.
>>            * include/Makefile.am: Remove <debug/array>.
>>            * include/Makefile.in: Regenerate.
>>            * include/experimental/functional: Adapt.
>>            * include/std/array: Move to _GLIBCXX_INLINE_VERSION namespace.
>>            * include/std/functional: Adapt.
>>            * include/std/span: Adapt.
>>            * testsuite/23_containers/array/debug/back1_neg.cc:
>>            Remove dg-require-debug-mode. Add -D_GLIBCXX_ASSERTIONS option.
>>            * testsuite/23_containers/array/debug/back2_neg.cc: Likewise.
>>            * testsuite/23_containers/array/debug/front1_neg.cc: Likewise.
>>            * testsuite/23_containers/array/debug/front2_neg.cc: Likewise.
>>            * 
>>testsuite/23_containers/array/debug/square_brackets_operator1_neg.cc:
>>            Likewise.
>>            * 
>>testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc:
>>            Likewise.
>>            * testsuite/23_containers/array/element_access/60497.cc
>>            * 
>>testsuite/23_containers/array/tuple_interface/get_debug_neg.cc:
>>            Remove.
>>            * testsuite/23_containers/array/tuple_interface/get_neg.cc
>>            * testsuite/23_containers/array/tuple_interface/tuple_element_debug_neg.cc
>>            * 
>>testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc
>>
>>Tested under Linux x86_64 normal and debug modes.
>>
>>Ok to commit ?
>
>Yes, this is a nice simplification, thanks.

This broke the C++11 constexpr support in std::array. Fixed with this
patch. Tested x86_64-linux, committed to trunk.



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

commit 91cfacc4b5d317b12a3bdcd798273a581568f645
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Dec 3 17:08:01 2020

    libstdc++: Disable std::array assertions for C++11 constexpr
    
    The recent changes to add assertions to std::array broke the functions
    that need to be constexpr in C++11, because of the restrictive rules for
    constexpr functions in C++11.
    
    This simply disables the assertions for C++11 mode, so the functions can
    be constexpr again.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/array (array::operator[](size_t) const, array::front() const)
            (array::back() const) [__cplusplus == 201103]: Disable
            assertions.
            * testsuite/23_containers/array/element_access/constexpr_element_access.cc:
            Check for correct values.
            * testsuite/23_containers/array/tuple_interface/get_neg.cc:
            Adjust dg-error line numbers.
            * testsuite/23_containers/array/debug/constexpr_c++11.cc: New test.

diff --git a/libstdc++-v3/include/std/array b/libstdc++-v3/include/std/array
index 3c4c88a536e..80750994058 100644
--- a/libstdc++-v3/include/std/array
+++ b/libstdc++-v3/include/std/array
@@ -192,7 +192,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr const_reference
       operator[](size_type __n) const noexcept
       {
+#if __cplusplus >= 201402L
 	__glibcxx_requires_subscript(__n);
+#endif
 	return _AT_Type::_S_ref(_M_elems, __n);
       }
 
@@ -228,7 +230,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr const_reference
       front() const noexcept
       {
+#if __cplusplus >= 201402L
 	__glibcxx_requires_nonempty();
+#endif
 	return _AT_Type::_S_ref(_M_elems, 0);
       }
 
@@ -242,7 +246,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr const_reference
       back() const noexcept
       {
+#if __cplusplus >= 201402L
 	__glibcxx_requires_nonempty();
+#endif
 	return _Nm ? _AT_Type::_S_ref(_M_elems, _Nm - 1)
  	           : _AT_Type::_S_ref(_M_elems, 0);
       }
diff --git a/libstdc++-v3/testsuite/23_containers/array/debug/constexpr_c++11.cc b/libstdc++-v3/testsuite/23_containers/array/debug/constexpr_c++11.cc
new file mode 100644
index 00000000000..c8b35994102
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/array/debug/constexpr_c++11.cc
@@ -0,0 +1,32 @@
+// { dg-options "-std=gnu++11 -D_GLIBCXX_DEBUG" }
+// { dg-do compile { target c++11 } }
+
+// Copyright (C) 2011-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <array>
+
+void test01()
+{
+  // array
+  constexpr std::array<std::size_t, 6> a = { { 0, 55, 66, 99, 4115, 2 } };
+  constexpr auto v1 = a[1];
+  constexpr auto v2 = a.at(2);
+  constexpr auto v3 = a.front();
+  constexpr auto v4 = a.back();
+  static_assert( (v1 + v2 + v3 + v4) == (55 + 66 + 0 + 2), "" );
+}
diff --git a/libstdc++-v3/testsuite/23_containers/array/element_access/constexpr_element_access.cc b/libstdc++-v3/testsuite/23_containers/array/element_access/constexpr_element_access.cc
index c1dd9054606..037f6ea4223 100644
--- a/libstdc++-v3/testsuite/23_containers/array/element_access/constexpr_element_access.cc
+++ b/libstdc++-v3/testsuite/23_containers/array/element_access/constexpr_element_access.cc
@@ -19,14 +19,13 @@
 
 #include <array>
 
-int main()
+void test01()
 {
   // array
-  typedef std::array<std::size_t, 6> array_type;
-  constexpr array_type a = { { 0, 55, 66, 99, 4115, 2 } };
-  constexpr auto v1 __attribute__((unused)) = a[1];
-  constexpr auto v2 __attribute__((unused)) = a.at(2);
-  constexpr auto v3 __attribute__((unused)) = a.front();
-  constexpr auto v4 __attribute__((unused)) = a.back();
-  return 0;
+  constexpr std::array<std::size_t, 6> a = { { 0, 55, 66, 99, 4115, 2 } };
+  constexpr auto v1 = a[1];
+  constexpr auto v2 = a.at(2);
+  constexpr auto v3 = a.front();
+  constexpr auto v4 = a.back();
+  static_assert( (v1 + v2 + v3 + v4) == (55 + 66 + 0 + 2), "" );
 }
diff --git a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
index 88416cc661c..e287eef2f61 100644
--- a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
@@ -26,6 +26,6 @@ int n1 = std::get<1>(a);
 int n2 = std::get<1>(std::move(a));
 int n3 = std::get<1>(ca);
 
-// { dg-error "static assertion failed" "" { target *-*-* } 357 }
-// { dg-error "static assertion failed" "" { target *-*-* } 365 }
-// { dg-error "static assertion failed" "" { target *-*-* } 373 }
+// { dg-error "static assertion failed" "" { target *-*-* } 363 }
+// { dg-error "static assertion failed" "" { target *-*-* } 371 }
+// { dg-error "static assertion failed" "" { target *-*-* } 379 }