Re: [PATCH] Add constexpr to iterator adaptors, array and range access

[Jonathan Wakely]

On 23/08/16 11:16 +0100, Jonathan Wakely wrote:

On 16/07/16 14:55 +0200, Daniel Krügler wrote:

Since I made a similar (unintentional) omission recently myself:
Shouldn't you touch std::__debug::array as well? What about other
functions from std::__debug?


Yes, I'll fix those shortly.


Here's the update for debug and profile modes.

I didn't wait for the profile mode tests to finish because I have more
important things to do, i.e. pretty much anything I can think of.

Committed to trunk.

commit cd738b74460efce03114f058ee020f9bc09308ac
Author: Jonathan Wakely 
Date:   Tue Aug 23 12:14:28 2016 +0100

Add C++17 constexpr to debug and profile mode arrays

	* include/debug/array (array): Add _GLIBCXX17_CONSTEXPR.
	* include/profile/array (array): Likewise.
	(array::swap): Fix exception specification for zero-sized arrays.

diff --git a/libstdc++-v3/include/debug/array b/libstdc++-v3/include/debug/array
index 6d51309..48ab2fd 100644
--- a/libstdc++-v3/include/debug/array
+++ b/libstdc++-v3/include/debug/array
@@ -90,66 +90,66 @@ namespace __debug
   { std::swap_ranges(begin(), end(), __other.begin()); }
 
   // Iterators.
-  iterator
+  _GLIBCXX17_CONSTEXPR iterator
   begin() noexcept
   { return iterator(data()); }
 
-  const_iterator
+  _GLIBCXX17_CONSTEXPR const_iterator
   begin() const noexcept
   { return const_iterator(data()); }
 
-  iterator
+  _GLIBCXX17_CONSTEXPR iterator
   end() noexcept
   { return iterator(data() + _Nm); }
 
-  const_iterator
+  _GLIBCXX17_CONSTEXPR const_iterator
   end() const noexcept
   { return const_iterator(data() + _Nm); }
 
-  reverse_iterator 
+  _GLIBCXX17_CONSTEXPR reverse_iterator
   rbegin() noexcept
   { return reverse_iterator(end()); }
 
-  const_reverse_iterator 
+  _GLIBCXX17_CONSTEXPR const_reverse_iterator
   rbegin() const noexcept
   { return const_reverse_iterator(end()); }
 
-  reverse_iterator 
+  _GLIBCXX17_CONSTEXPR reverse_iterator
   rend() noexcept
   { return reverse_iterator(begin()); }
 
-  const_reverse_iterator 
+  _GLIBCXX17_CONSTEXPR const_reverse_iterator
   rend() const noexcept
   { return const_reverse_iterator(begin()); }
 
-  const_iterator
+  _GLIBCXX17_CONSTEXPR const_iterator
   cbegin() const noexcept
   { return const_iterator(data()); }
 
-  const_iterator
+  _GLIBCXX17_CONSTEXPR const_iterator
   cend() const noexcept
   { return const_iterator(data() + _Nm); }
 
-  const_reverse_iterator 
+  _GLIBCXX17_CONSTEXPR const_reverse_iterator
   crbegin() const noexcept
   { return const_reverse_iterator(end()); }
 
-  const_reverse_iterator 
+  _GLIBCXX17_CONSTEXPR const_reverse_iterator
   crend() const noexcept
   { return const_reverse_iterator(begin()); }
 
   // Capacity.
-  constexpr size_type 
+  constexpr size_type
   size() const noexcept { return _Nm; }
 
-  constexpr size_type 
+  constexpr size_type
   max_size() const noexcept { return _Nm; }
 
-  constexpr bool 
+  constexpr bool
   empty() const noexcept { return size() == 0; }
 
   // Element access.
-  reference
+  _GLIBCXX17_CONSTEXPR reference
   operator[](size_type __n) noexcept
   {
 	__glibcxx_check_subscript(__n);
@@ -164,7 +164,7 @@ namespace __debug
 	_AT_Type::_S_ref(_M_elems, 0));
   }
 
-  reference
+  _GLIBCXX17_CONSTEXPR reference
   at(size_type __n)
   {
 	if (__n >= _Nm)
@@ -186,14 +186,14 @@ namespace __debug
 	 _AT_Type::_S_ref(_M_elems, 0));
   }
 
-  reference 
+  _GLIBCXX17_CONSTEXPR reference
   front() noexcept
   {
 	__glibcxx_check_nonempty();
 	return *begin();
   }
 
-  constexpr const_reference 
+  constexpr const_reference
   front() const noexcept
   {
 	return _Nm ? _AT_Type::_S_ref(_M_elems, 0)
@@ -201,14 +201,14 @@ namespace __debug
 	 _AT_Type::_S_ref(_M_elems, 0));
   }
 
-  reference 
+  _GLIBCXX17_CONSTEXPR reference
   back() noexcept
   {
 	__glibcxx_check_nonempty();
 	return _Nm ? *(end() - 1) : *end();
   }
 
-  constexpr const_reference 
+  constexpr const_reference
   back() const noexcept
   {
 	return _Nm ? _AT_Type::_S_ref(_M_elems, _Nm - 1)
@@ -216,18 +216,18 @@ namespace __debug
 	 _AT_Type::_S_ref(_M_elems, 0));
   }
 
-  pointer
+  _GLIBCXX17_CONSTEXPR pointer
   data() noexcept
   { return _AT_Type::_S_ptr(_M_elems); }
 
-  const_pointer
+  _GLIBCXX17_CONSTEXPR const_pointer
   data() const noexcept
   { return _AT_Type::_S_ptr(_M_elems); }
 };
 
   // Array comparisons.
   template
-inline bool 
+inline bool
 operator==(const array<_Tp, _Nm>& __one, const array<_Tp, _Nm>& __two)
 { return std::equal(__one.begin(), __one.end(), __two.begin()); }
 

Re: [PATCH] Add constexpr to iterator adaptors, array and range access

[Jonathan Wakely]

On 16/07/16 14:55 +0200, Daniel Krügler wrote:

2016-07-16 1:09 GMT+02:00 Jonathan Wakely :

This patch implements http://wg21.link/p0031 which adds constexpr to
most operations on std::reverse_iterator, std::move_iterator,
std::array, as well as std::advance, std::distance, and the
range-access functions std::begin, std::rbegin etc.

Strictly speaking, those functions should only be constexpr in C++17
and not before, but this patch makes them constexpr whenever possible.
That means the changes are fully implemented for C++14 (and the
feature-test macro is defined) but only partially implemented for
C++11, because some of the functions can't be constexpr in C++11.

My thinking is that if the committee has decided that these functions
*should* be constexpr, and changed them for C++17, then it doesn't
serve users to make them non-constexpr in C++11 and C++14 just because
the standard says so.

How do other people feel about that?

The alternative would be to define a new _GLIBCXX17_CONSTEXPR macro
and use it in all these places, so they're only constexpr in C++17
(and probably for -std=gnu++14 too, but not -std=c++14).

How strict do we want to be about obeying the "implementors can't add
constexpr" rule in these cases?


As much as I hate the current restrictions, I tend to suggest to
follow them strictly in __STRICT_ANSI__ mode.


As there were objections to adding constexpr to C++11 and C++14 where
it's not meant to be, I'm only adding it for C++17.

We can relax that for gnu++14 (i.e. !__STRICT_ANSI__) later, but I
haven't done that for now.


Here's the patch, but I'm not committing it yet.


Since I made a similar (unintentional) omission recently myself:
Shouldn't you touch std::__debug::array as well? What about other
functions from std::__debug?


Yes, I'll fix those shortly.

Committed to trunk.

commit 4e025f534aff5a43d319b7f3f4a89d98f3dcaf53
Author: Jonathan Wakely 
Date:   Fri Jul 15 22:31:16 2016 +0100

Add constexpr to  and  for C++17

	* include/bits/c++config (_GLIBCXX17_CONSTEXPR): Define.
	* include/bits/range_access.h (begin, end, rbegin, rend, crbegin)
	(crend): Add _GLIBCXX17_CONSTEXPR as per P0031R0.
	* include/bits/stl_iterator.h (reverse_iterator, move_iterator)
	(__make_reverse_iterator, make_reverse_iterator, make_move_iterator):
	Likewise.
	* include/bits/stl_iterator_base_funcs.h (__distance, __advance): Add
	_GLIBCXX14_CONSTEXPR.
	(distance, advance, next, prev): Add _GLIBCXX17_CONSTEXPR.
	* include/std/array (array::begin, array::end, array::rbegin)
	(array::rend, array::cbegin, array:cend, array::crbegin)
	(array::crend, array::operator[], array::at, array::front)
	(array::back, array::data): Likewise.
	* testsuite/24_iterators/headers/iterator/range_access.cc: Replace
	with separate tests for C++11, C++14, and C++17.
	* testsuite/24_iterators/headers/iterator/range_access_c++11.cc: New.
	* testsuite/24_iterators/headers/iterator/range_access_c++14.cc: New.
	* testsuite/24_iterators/headers/iterator/range_access_c++17.cc: New.

diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 8d2c361..656ef78 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -111,6 +111,14 @@
 # endif
 #endif
 
+#ifndef _GLIBCXX17_CONSTEXPR
+# if __cplusplus > 201402L
+#  define _GLIBCXX17_CONSTEXPR constexpr
+# else
+#  define _GLIBCXX17_CONSTEXPR
+# endif
+#endif
+
 // Macro for noexcept, to support in mixed 03/0x mode.
 #ifndef _GLIBCXX_NOEXCEPT
 # if __cplusplus >= 201103L
diff --git a/libstdc++-v3/include/bits/range_access.h b/libstdc++-v3/include/bits/range_access.h
index d6f8fa1..27cc8ed 100644
--- a/libstdc++-v3/include/bits/range_access.h
+++ b/libstdc++-v3/include/bits/range_access.h
@@ -48,7 +48,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+inline _GLIBCXX17_CONSTEXPR auto
 begin(_Container& __cont) -> decltype(__cont.begin())
 { return __cont.begin(); }
 
@@ -58,7 +58,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+inline _GLIBCXX17_CONSTEXPR auto
 begin(const _Container& __cont) -> decltype(__cont.begin())
 { return __cont.begin(); }
 
@@ -68,7 +68,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+inline _GLIBCXX17_CONSTEXPR auto
 end(_Container& __cont) -> decltype(__cont.end())
 { return __cont.end(); }
 
@@ -78,7 +78,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+inline _GLIBCXX17_CONSTEXPR auto
 end(const _Container& __cont) -> decltype(__cont.end())
 { return __cont.end(); }
 
@@ -138,7 +138,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+ 

Re: [PATCH] Add constexpr to iterator adaptors, array and range access

[Ville Voutilainen]

On 16 July 2016 at 18:38, Ville Voutilainen  wrote:
> I think it's reasonable to make the functions constexpr even when the standard
> doesn't allow it, and if c++17 allows them to be constexpr, make the functions
> constexpr in other standard modes as well. We could of course ask whether
> LWG thought about applying the changes to previous standards or not.

Let me elaborate a bit. I think it's perfectly sane to enable
constexpr for c++11 and c++14
when c++17 enables it, and talk to other vendors and LEWG/LWG about syncing such
changes.

>> How strict do we want to be about obeying the "implementors can't add
>> constexpr" rule in these cases?
>
> I'd rather not be strict.

..and here I would prefer accepting some amounts of non-compliance and
feeding the results
back to LEWG/LWG and other vendors. If/when users run into trouble, we
get better constexpr
support in the spec, and we gain better performance for cases where
constexpr evaluation
isn't strictly required.

I have had some ruminations for an annotation of some kind that would
allow the front-end to detect non-portable
uses of constexpr but allow the performance increase for portable
cases. I haven't had the time
to do the actual work for the front-end to be able to tell the
difference based on such an annotation.

Re: [PATCH] Add constexpr to iterator adaptors, array and range access

[Ville Voutilainen]

> This patch implements http://wg21.link/p0031 which adds constexpr to
> most operations on std::reverse_iterator, std::move_iterator,
> std::array, as well as std::advance, std::distance, and the
> range-access functions std::begin, std::rbegin etc.
>
> Strictly speaking, those functions should only be constexpr in C++17
> and not before, but this patch makes them constexpr whenever possible.
> That means the changes are fully implemented for C++14 (and the
> feature-test macro is defined) but only partially implemented for
> C++11, because some of the functions can't be constexpr in C++11.
>
> My thinking is that if the committee has decided that these functions
> *should* be constexpr, and changed them for C++17, then it doesn't
> serve users to make them non-constexpr in C++11 and C++14 just because
> the standard says so.
>
> How do other people feel about that?

I think it's reasonable to make the functions constexpr even when the standard
doesn't allow it, and if c++17 allows them to be constexpr, make the functions
constexpr in other standard modes as well. We could of course ask whether
LWG thought about applying the changes to previous standards or not.

> The alternative would be to define a new _GLIBCXX17_CONSTEXPR macro
> and use it in all these places, so they're only constexpr in C++17
> (and probably for -std=gnu++14 too, but not -std=c++14).

That's certainly "safer".

> How strict do we want to be about obeying the "implementors can't add
> constexpr" rule in these cases?

I'd rather not be strict.

Re: [PATCH] Add constexpr to iterator adaptors, array and range access

[Daniel Krügler]

2016-07-16 1:09 GMT+02:00 Jonathan Wakely :
> This patch implements http://wg21.link/p0031 which adds constexpr to
> most operations on std::reverse_iterator, std::move_iterator,
> std::array, as well as std::advance, std::distance, and the
> range-access functions std::begin, std::rbegin etc.
>
> Strictly speaking, those functions should only be constexpr in C++17
> and not before, but this patch makes them constexpr whenever possible.
> That means the changes are fully implemented for C++14 (and the
> feature-test macro is defined) but only partially implemented for
> C++11, because some of the functions can't be constexpr in C++11.
>
> My thinking is that if the committee has decided that these functions
> *should* be constexpr, and changed them for C++17, then it doesn't
> serve users to make them non-constexpr in C++11 and C++14 just because
> the standard says so.
>
> How do other people feel about that?
>
> The alternative would be to define a new _GLIBCXX17_CONSTEXPR macro
> and use it in all these places, so they're only constexpr in C++17
> (and probably for -std=gnu++14 too, but not -std=c++14).
>
> How strict do we want to be about obeying the "implementors can't add
> constexpr" rule in these cases?

As much as I hate the current restrictions, I tend to suggest to
follow them strictly in __STRICT_ANSI__ mode.

> Here's the patch, but I'm not committing it yet.

Since I made a similar (unintentional) omission recently myself:
Shouldn't you touch std::__debug::array as well? What about other
functions from std::__debug?

- Daniel

Re: [PATCH] Add constexpr to iterator adaptors, array and range access

[Oleg Endo]

On Sat, 2016-07-16 at 00:09 +0100, Jonathan Wakely wrote:
> This patch implements http://wg21.link/p0031 which adds constexpr to
> most operations on std::reverse_iterator, std::move_iterator,
> std::array, as well as std::advance, std::distance, and the
> range-access functions std::begin, std::rbegin etc.
> 
> Strictly speaking, those functions should only be constexpr in C++17
> and not before, but this patch makes them constexpr whenever
> possible.
> That means the changes are fully implemented for C++14 (and the
> feature-test macro is defined) but only partially implemented for
> C++11, because some of the functions can't be constexpr in C++11.
> 
> My thinking is that if the committee has decided that these functions
> *should* be constexpr, and changed them for C++17, then it doesn't
> serve users to make them non-constexpr in C++11 and C++14 just
> because
> the standard says so.
> 
> How do other people feel about that?
> 
The alternative would be to define a new _GLIBCXX17_CONSTEXPR macro
> and use it in all these places, so they're only constexpr in C++17
> (and probably for -std=gnu++14 too, but not -std=c++14).
> 
> How strict do we want to be about obeying the "implementors can't add
> constexpr" rule in these cases?

Other std libs are even more ignorant regarding this and do stuff like
defining several C++11 features and functionalities even if C++11 is
not enabled, which can be annoying at times.

One thing that could happen with those extra constexpr, is having
people accidentally writing non-C++11/14-compliant code on a newer
C++17 toolchain, although they actually wanted to write C++11/14
compliant code.

GCC itself, with its conservative toolchain requirements, is probably a
good example.  If GCC's compiler requirement is bumped from C++98/03 to
C++11 some day, most GCC developers are probably going to use a pretty
recent GCC version for GCC development.  As a consequence, silent non
-compliant constexpr related constructs might slip in over and over
again simply because people don't notice.  So everyone would need to
keep some older C++11-only toolchain version just for testing the
build.  I can imagine this to be a source of frustration.  If users
want to write C++11 or C++14 code, the toolchain should assist them in
doing so as much as possible.

I'd vote for the _GLIBCXX17_CONSTEXPR macro.

Cheers,
Oleg

[PATCH] Add constexpr to iterator adaptors, array and range access

[Jonathan Wakely]

This patch implements http://wg21.link/p0031 which adds constexpr to
most operations on std::reverse_iterator, std::move_iterator,
std::array, as well as std::advance, std::distance, and the
range-access functions std::begin, std::rbegin etc.

Strictly speaking, those functions should only be constexpr in C++17
and not before, but this patch makes them constexpr whenever possible.
That means the changes are fully implemented for C++14 (and the
feature-test macro is defined) but only partially implemented for
C++11, because some of the functions can't be constexpr in C++11.

My thinking is that if the committee has decided that these functions
*should* be constexpr, and changed them for C++17, then it doesn't
serve users to make them non-constexpr in C++11 and C++14 just because
the standard says so.

How do other people feel about that?

The alternative would be to define a new _GLIBCXX17_CONSTEXPR macro
and use it in all these places, so they're only constexpr in C++17
(and probably for -std=gnu++14 too, but not -std=c++14).

How strict do we want to be about obeying the "implementors can't add
constexpr" rule in these cases?

Here's the patch, but I'm not committing it yet.

* include/bits/range_access.h (begin, end, rbegin, rend, crbegin)
(crend): Add constexpr specifier as per P0031R0.
* include/bits/stl_iterator.h (reverse_iterator, move_iterator)
(__make_reverse_iterator, make_reverse_iterator, make_move_iterator):
Add _GLIBCXX_CONSTEXPR or _GLIBCXX14_CONSTEXPR as appropriate.
* include/bits/stl_iterator_base_funcs.h (__distance, distance)
(__advance, advance, next, prev): Likewise.
* include/std/array (array::begin, array::end, array::rbegin)
(array::rend, array::cbegin, array:cend, array::crbegin)
(array::crend, array::operator[], array::at, array::front)
(array::back, array::data): Likewise.
* testsuite/24_iterators/headers/iterator/range_access.cc: Add
constexpr.
* testsuite/24_iterators/headers/iterator/synopsis.cc: Add constexpr
to reverse_iterator operations, advance, and distance.
commit 7e46198cc511bcc8876ab9d3f11e938dbe4cfea0
Author: Jonathan Wakely 
Date:   Fri Jul 15 22:31:16 2016 +0100

Add constexpr to iterator adaptors, array and range access

* include/bits/range_access.h (begin, end, rbegin, rend, crbegin)
(crend): Add constexpr specifier as per P0031R0.
* include/bits/stl_iterator.h (reverse_iterator, move_iterator)
(__make_reverse_iterator, make_reverse_iterator, make_move_iterator):
Add _GLIBCXX_CONSTEXPR or _GLIBCXX14_CONSTEXPR as appropriate.
* include/bits/stl_iterator_base_funcs.h (__distance, distance)
(__advance, advance, next, prev): Likewise.
* include/std/array (array::begin, array::end, array::rbegin)
(array::rend, array::cbegin, array:cend, array::crbegin)
(array::crend, array::operator[], array::at, array::front)
(array::back, array::data): Likewise.
* testsuite/24_iterators/headers/iterator/range_access.cc: Add
constexpr.
* testsuite/24_iterators/headers/iterator/synopsis.cc: Add constexpr
to reverse_iterator operations, advance, and distance.

diff --git a/libstdc++-v3/include/bits/range_access.h 
b/libstdc++-v3/include/bits/range_access.h
index e2ec072..257122e 100644
--- a/libstdc++-v3/include/bits/range_access.h
+++ b/libstdc++-v3/include/bits/range_access.h
@@ -44,7 +44,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+constexpr auto
 begin(_Container& __cont) -> decltype(__cont.begin())
 { return __cont.begin(); }
 
@@ -54,7 +54,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+constexpr auto
 begin(const _Container& __cont) -> decltype(__cont.begin())
 { return __cont.begin(); }
 
@@ -64,7 +64,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+constexpr auto
 end(_Container& __cont) -> decltype(__cont.end())
 { return __cont.end(); }
 
@@ -74,7 +74,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template
-inline auto
+constexpr auto
 end(const _Container& __cont) -> decltype(__cont.end())
 { return __cont.end(); }
 
@@ -83,7 +83,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __arr  Array.
*/
   template
-inline _GLIBCXX14_CONSTEXPR _Tp*
+constexpr _Tp*
 begin(_Tp (&__arr)[_Nm])
 { return __arr; }
 
@@ -93,7 +93,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __arr  Array.
*/
   template
-inline _GLIBCXX14_CONSTEXPR _Tp*
+constexpr _Tp*
 end(_Tp (&__arr)[_Nm])
 { return __arr + _Nm; }
 
@@ -112,7 +112,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @param  __cont  Container.
*/
   template