diff mbox

[v3] Implement make_array and to_array from the Fundamentals v2 TS draft

Message ID CAFk2RUaz+d7A2m_0Pnd9pPvKhHO-+o0Et+n4zwVOZAunZPEXJA@mail.gmail.com
State New
Headers show

Commit Message

Ville Voutilainen July 13, 2015, 10:55 a.m. UTC
On 13 July 2015 at 01:25, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
> On 12 July 2015 at 21:45, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
>> Tested on Linux-PPC64.
>>
>> 2015-07-12  Ville Voutilainen  <ville.voutilainen@gmail.com>
>>     Implement std::experimental::fundamentals_v2::make_array and
>>     std::experimental::fundamentals_v2::to_array.
>>     * include/Makefile.am: Add array.
>>     * include/Makefile.in: Add array.
>>     * include/experimental/array: New.
>>     * testsuite/experimental/array/make_array.cc: Likewise.
>>     * testsuite/experimental/array/neg.cc: Likewise.
>
> Very minor cleanup in a new patch, use is_void<_D> instead of is_same<_D, void>,
> indent the static assert a bit more clearly.

Oops, the implementation failed to forward() in make_array, new patch attached,
with a test for make_array with a move-only type.

Comments

Jonathan Wakely Aug. 7, 2015, 8:37 a.m. UTC | #1
On 13/07/15 13:55 +0300, Ville Voutilainen wrote:
>+template <typename _Up>
>+struct __is_reference_wrapper : false_type
>+{ };

Please indent the class-head and the body.

>+template <typename _Up>
>+struct __is_reference_wrapper<reference_wrapper<_Up>> : true_type
>+{ };

Likewise.

>+template <typename _D = void, typename... _Types>
>+constexpr auto make_array(_Types&&... __t)

Same here, everything after the 'template<...>' should be indented.

Maybe consider putting a newline after 'auto' too

And the single character name _D makes me nervous again :-)


>+  -> array<conditional_t<is_void_v<_D>,
>+                         common_type_t<_Types...>,
>+                         _D>,
>+           sizeof...(_Types)>
>+{
>+  static_assert(__or_<
>+                  __not_<is_void<_D>>,
>+                  __and_<__not_<__is_reference_wrapper<decay_t<_Types>>>...>>
>+                ::value,
>+                "make_array cannot be used without an explicit target type "
>+                "if any of the types given is a reference_wrapper");

I wonder if that would be more efficient to instantiate written as:

  __not_<__and_<is_void<_D>, __or_<__is_reference_wrapper<decay_t<_Types>>...>>>

>+  return {{forward<_Types>(__t)...}};
>+}
>+
>+template <typename _Tp, size_t _N, size_t... _Idx>

_N is a common macro in some C libs, please rename to something like _Nm.
(_D isn't listed in the docs as a name to avoid, so should be OK).

>diff --git a/libstdc++-v3/testsuite/experimental/array/make_array.cc b/libstdc++-v3/testsuite/experimental/array/make_array.cc
>new file mode 100644
>index 0000000..8456c37
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/experimental/array/make_array.cc
>@@ -0,0 +1,47 @@
>+// { dg-options "-std=gnu++14" }
>+// { dg-do run }

Does this need to be dg-do run? It doesn't seem to run anything
interesting so could be dg-do compile.
Jonathan Wakely Aug. 8, 2015, 10:39 a.m. UTC | #2
On 07/08/15 09:37 +0100, Jonathan Wakely wrote:
>On 13/07/15 13:55 +0300, Ville Voutilainen wrote:
>>+template <typename _Up>
>>+struct __is_reference_wrapper : false_type
>>+{ };
>
>Please indent the class-head and the body.
>
>>+template <typename _Up>
>>+struct __is_reference_wrapper<reference_wrapper<_Up>> : true_type
>>+{ };
>
>Likewise.
>
>>+template <typename _D = void, typename... _Types>
>>+constexpr auto make_array(_Types&&... __t)
>
>Same here, everything after the 'template<...>' should be indented.
>
>Maybe consider putting a newline after 'auto' too
>
>And the single character name _D makes me nervous again :-)
>
>
>>+  -> array<conditional_t<is_void_v<_D>,
>>+                         common_type_t<_Types...>,
>>+                         _D>,
>>+           sizeof...(_Types)>
>>+{
>>+  static_assert(__or_<
>>+                  __not_<is_void<_D>>,
>>+                  __and_<__not_<__is_reference_wrapper<decay_t<_Types>>>...>>
>>+                ::value,
>>+                "make_array cannot be used without an explicit target type "
>>+                "if any of the types given is a reference_wrapper");
>
>I wonder if that would be more efficient to instantiate written as:
>
> __not_<__and_<is_void<_D>, __or_<__is_reference_wrapper<decay_t<_Types>>...>>>
>
>>+  return {{forward<_Types>(__t)...}};
>>+}
>>+
>>+template <typename _Tp, size_t _N, size_t... _Idx>
>
>_N is a common macro in some C libs, please rename to something like _Nm.
>(_D isn't listed in the docs as a name to avoid, so should be OK).
>
>>diff --git a/libstdc++-v3/testsuite/experimental/array/make_array.cc b/libstdc++-v3/testsuite/experimental/array/make_array.cc
>>new file mode 100644
>>index 0000000..8456c37
>>--- /dev/null
>>+++ b/libstdc++-v3/testsuite/experimental/array/make_array.cc
>>@@ -0,0 +1,47 @@
>>+// { dg-options "-std=gnu++14" }
>>+// { dg-do run }
>
>Does this need to be dg-do run? It doesn't seem to run anything
>interesting so could be dg-do compile.


P.S. I should have said it's OK for trunk with those changes.

Up to you if you want to change the __or_<> condition or not, I didn't
do a proper analysis of which might perform better. Also up to you if
you want to change _D to stop me being nervous.
diff mbox

Patch

diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index 05be8ad..41fc4af 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -646,6 +646,7 @@  experimental_builddir = ./experimental
 experimental_headers = \
 	${experimental_srcdir}/algorithm \
 	${experimental_srcdir}/any \
+	${experimental_srcdir}/array \
 	${experimental_srcdir}/chrono \
 	${experimental_srcdir}/deque \
 	${experimental_srcdir}/erase_if.h \
@@ -657,6 +658,7 @@  experimental_headers = \
 	${experimental_srcdir}/memory \
 	${experimental_srcdir}/numeric \
 	${experimental_srcdir}/optional \
+	${experimental_srcdir}/propagate_const \
 	${experimental_srcdir}/ratio \
 	${experimental_srcdir}/set \
 	${experimental_srcdir}/string \
diff --git a/libstdc++-v3/include/Makefile.in b/libstdc++-v3/include/Makefile.in
index bab83b4..b2a140c 100644
--- a/libstdc++-v3/include/Makefile.in
+++ b/libstdc++-v3/include/Makefile.in
@@ -935,6 +935,7 @@  experimental_builddir = ./experimental
 experimental_headers = \
 	${experimental_srcdir}/algorithm \
 	${experimental_srcdir}/any \
+	${experimental_srcdir}/array \
 	${experimental_srcdir}/chrono \
 	${experimental_srcdir}/deque \
 	${experimental_srcdir}/erase_if.h \
@@ -946,6 +947,7 @@  experimental_headers = \
 	${experimental_srcdir}/memory \
 	${experimental_srcdir}/numeric \
 	${experimental_srcdir}/optional \
+	${experimental_srcdir}/propagate_const \
 	${experimental_srcdir}/ratio \
 	${experimental_srcdir}/set \
 	${experimental_srcdir}/string \
diff --git a/libstdc++-v3/include/experimental/array b/libstdc++-v3/include/experimental/array
new file mode 100644
index 0000000..b72895c
--- /dev/null
+++ b/libstdc++-v3/include/experimental/array
@@ -0,0 +1,107 @@ 
+// <experimental/array> -*- C++ -*-
+
+// Copyright (C) 2015 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.
+
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+// <http://www.gnu.org/licenses/>.
+
+/** @file experimental/array
+ *  This is a TS C++ Library header.
+ */
+
+#ifndef _GLIBCXX_EXPERIMENTAL_ARRAY
+#define _GLIBCXX_EXPERIMENTAL_ARRAY 1
+
+#pragma GCC system_header
+
+#if __cplusplus <= 201103L
+# include <bits/c++14_warning.h>
+#else
+
+#include <array>
+#include <functional>
+#include <experimental/type_traits>
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+namespace experimental
+{
+inline namespace fundamentals_v2
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  /**
+   * @defgroup make_array Array creation functions
+   * @ingroup experimental
+   *
+   * Array creation functions as described in N4529,
+   * Working Draft, C++ Extensions for Library Fundamentals, Version 2
+   *
+   * @{
+   */
+
+template <typename _Up>
+struct __is_reference_wrapper : false_type
+{ };
+
+template <typename _Up>
+struct __is_reference_wrapper<reference_wrapper<_Up>> : true_type
+{ };
+
+template <typename _D = void, typename... _Types>
+constexpr auto make_array(_Types&&... __t)
+  -> array<conditional_t<is_void_v<_D>,
+                         common_type_t<_Types...>,
+                         _D>,
+           sizeof...(_Types)>
+{
+  static_assert(__or_<
+                  __not_<is_void<_D>>,
+                  __and_<__not_<__is_reference_wrapper<decay_t<_Types>>>...>>
+                ::value,
+                "make_array cannot be used without an explicit target type "
+                "if any of the types given is a reference_wrapper");
+  return {{forward<_Types>(__t)...}};
+}
+
+template <typename _Tp, size_t _N, size_t... _Idx>
+constexpr array<remove_cv_t<_Tp>, _N>
+__to_array(_Tp (&__a)[_N],
+           index_sequence<_Idx...>)
+{
+  return {{__a[_Idx]...}};
+}
+
+template <typename _Tp, size_t _N>
+constexpr array<remove_cv_t<_Tp>, _N> to_array(_Tp (&__a)[_N])
+{
+  return __to_array(__a, make_index_sequence<_N>{});
+}
+
+  // @} group make_array
+  _GLIBCXX_END_NAMESPACE_VERSION
+} // namespace fundamentals_v2
+} // namespace experimental
+
+} // namespace std
+
+#endif // C++14
+
+#endif // _GLIBCXX_EXPERIMENTAL_ARRAY
diff --git a/libstdc++-v3/testsuite/experimental/array/make_array.cc b/libstdc++-v3/testsuite/experimental/array/make_array.cc
new file mode 100644
index 0000000..8456c37
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/array/make_array.cc
@@ -0,0 +1,47 @@ 
+// { dg-options "-std=gnu++14" }
+// { dg-do run }
+
+// Copyright (C) 2015 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 moved_to of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <experimental/array>
+
+struct MoveOnly
+{
+  MoveOnly() = default;
+  MoveOnly(MoveOnly&&) = default;
+  MoveOnly& operator=(MoveOnly&&) = default;
+};
+
+int main()
+{
+  char x[42];
+  std::array<char, 42> y = std::experimental::to_array(x);
+  std::array<int, 5> z = std::experimental::make_array(1,2,3,4,5);
+  std::array<long, 3> zz = std::experimental::make_array(1,2L, 3);
+  std::array<MoveOnly, 1> zzz = std::experimental::make_array(MoveOnly{});
+  int dummy;
+  auto good = std::experimental::make_array<
+    std::reference_wrapper<int>>(std::ref(dummy));
+  constexpr char x2[42]{};
+  constexpr std::array<char, 42> y2 = std::experimental::to_array(x2);
+  constexpr std::array<int, 5> z2 =
+    std::experimental::make_array(1,2,3,4,5);
+  constexpr std::array<long, 3> zz2
+    = std::experimental::make_array(1,2L, 3);
+  constexpr std::array<MoveOnly, 1> zzz2 = std::experimental::make_array(MoveOnly{});
+}
diff --git a/libstdc++-v3/testsuite/experimental/array/neg.cc b/libstdc++-v3/testsuite/experimental/array/neg.cc
new file mode 100644
index 0000000..643466b
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/array/neg.cc
@@ -0,0 +1,28 @@ 
+// { dg-options "-std=gnu++14" }
+// { dg-do compile }
+
+// Copyright (C) 2015 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 moved_to of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <experimental/array>
+
+int main()
+{
+  int dummy;
+  auto bad = std::experimental::make_array(std::ref(dummy));
+  // { dg-error "make_array cannot be used without an explicit target type if any of the types given is a reference_wrapper" "" { target *-*-* } 75 }
+}