[libcxx-commits] [PATCH] D115986: [libc++] Allow __move_constexpr to work with unrelated pointers

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

As @Quuxplusone mentions, I would really support an effort to clean up our various `__foo_constexpr` functions.

LGTM with the renaming.



================
Comment at: libcxx/include/__string:293
 static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-_CharT* __move_constexpr(_CharT* __s1, const _CharT* __s2, size_t __n) _NOEXCEPT
+_CharT* __copy_constexpr(_CharT* __s1, const _CharT* __s2, size_t __n) _NOEXCEPT
 {
----------------
`__s1` and `__s2` is super confusing, especially since the order is reversed from `std::copy_n`. Please apply this throughout as a fly-by.


================
Comment at: libcxx/include/__string:295
 {
-    if (__n == 0) return __s1;
-    if (__s1 < __s2) {
-      _VSTD::copy(__s2, __s2 + __n, __s1);
-    } else if (__s2 < __s1) {
-      _VSTD::copy_backward(__s2, __s2 + __n, __s1 + __n);
-    }
-    return __s1;
+  _LIBCPP_ASSERT(__libcpp_is_constant_evaluated(), "__copy_constexpr() should always be constant evaluated");
+  _VSTD::copy_n(__s2, __n, __s1);
----------------
This is pretty annoying -- if we ever end up calling this function outside of `constexpr` (by mistake or otherwise), we'll end up with an assertion that fails at runtime. I don't see a way to make this fail at compile-time, since if you use `__libcpp_is_constant_evaluated()` in a place where a constant expression is required, it'll always return true. There's nothing actionable to this comment, but I thought I'd point out that this is a pretty bad defect of `is_constant_evaluated()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115986/new/

https://reviews.llvm.org/D115986