On 26/06/19 19:13 -0400, Ed Smith-Rowland via libstdc++ wrote:
Here is the first of three patches for C++20 constexpr library.

?????? Implement C++20 p0202 - Add constexpr Modifiers to Functions in <algorithm> and <utility> Headers.
???? ??Implement C++20 p1023 - constexpr comparison operators for std::array.

I believe I have answered peoples concerns with the last patch attempts [https://gcc.gnu.org/ml/libstdc++/2019-03/msg00132.html].

The patch is large because of test cases but really just boils down to adding constexpr for c++2a.

I still have some concerns about the changes like:

    template<typename _Iterator, typename _Value>
+      _GLIBCXX20_CONSTEXPR
      bool
-      operator()(_Iterator __it, _Value& __val) const
+      operator()(_Iterator __it, const _Value& __val) const
      { return *__it < __val; }


I think that might change semantics for some types, and I haven't yet
figured out if we care about those cases or not. I'll try to come up
with some examples that change meaning.

This could use _GLIBCXX14_CONSTEXPR:

+  /**
+   * A constexpr wrapper for __builtin_memmove.
+   * @param __num The number of elements of type _Tp (not bytes).

I don't think we want a Doxygen comment for this, as it's not part of
the public API. Just a normal comment is fine.

+   */
+  template<bool _IsMove, typename _Tp>
+    _GLIBCXX20_CONSTEXPR
+    inline void*
+    __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
+    {
+#if __cplusplus > 201703L \
+    && defined(_GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED)
+      if (__builtin_is_constant_evaluated())
+       {
+         for(; __num > 0; --__num)
+           {
+             if constexpr (_IsMove)
+               *__dst = std::move(*__src);
+             else
+               *__dst = *__src;

Do we need this _IsMove condition here? We should only be using this
function for trivially copyable types, in which case copy vs move
shouldn't matter, should it?

Oh ... is it because we also end up using this __memmove function for
non-trivially copyable types, during constant evaluation? Hmm. Then
it's more than just a wrapper for __builtin_memmove, right? It's
"either memmove or constexpr range copy", or something.

I don't think this will work in a constant expression:

  /// Assign @p __new_val to @p __obj and return its previous value.
  template <typename _Tp, typename _Up = _Tp>
+    _GLIBCXX20_CONSTEXPR
    inline _Tp
    exchange(_Tp& __obj, _Up&& __new_val)
    { return std::__exchange(__obj, std::forward<_Up>(__new_val)); }

Because std::__exchange hasn't been marked constexpr. The test passes
because it doesn't call it in a context that requires constant
evaluation:

 const auto x = std::exchange(e, pi);

I see the same problem in other tests too:

+  constexpr std::array<int, 12> car{{0, 1, 2, 3, 4, 5, 6, 6, 8, 9, 9, 11}};
+
+  const auto out0x = std::adjacent_find(car.begin(), car.end());
+
+  const auto out1x = std::adjacent_find(car.begin(), car.end(),
+                                       std::equal_to<int>());



Reply via email to