[libcxx-commits] [PATCH] D68365: Prototype implementation of P1004R2 (making std::vector constexpr).

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/memory:1390-1401
+template <class _Tp>
+_LIBCPP_CONSTEXPR_AFTER_CXX17
+_Tp* __libcpp_memcpy(_Tp* __dest, _Tp const* __src, std::size_t __count) {
+    if (__libcpp_is_constant_evaluated()) {
+        for (std::size_t __n = 0; __n != __count; ++__n)
+            __dest[__n] = __src[__n];
+    } else {
----------------
miscco wrote:
> I am wondering whether we should reuse the simplementation of std::copy_n here (or at the call site for what its worth)
I think this and the comment below have been addressed. But I'm not 100% sure what you were referring to exactly. Could you comment again at the correct line if the comments haven't been addressed?


================
Comment at: libcxx/include/memory:1984
 
-
-  _LIBCPP_INLINE_VISIBILITY reference __get() _NOEXCEPT { return __value_; }
-  _LIBCPP_INLINE_VISIBILITY
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
+  reference __get() _NOEXCEPT { return __value_; }
----------------
miscco wrote:
> From here and below  there is `_LIBCPP_CONSTEXPR_AFTER_CXX11` rather than `_LIBCPP_CONSTEXPR_AFTER_CXX17 `
> 
> Also the ordering of `_LIBCPP_INLINE_VISIBILITY ` and `_LIBCPP_CONSTEXPR_AFTER_CXX11` is swapped.
Sorry, I don't know what you are referring to again. I guess they have been addressed since I can't see any `_LIBCPP_CONSTEXPR_AFTER_CXX11` of `inline` in the diff.


================
Comment at: libcxx/include/vector:422
 template <class _Tp, class _Allocator>
-inline _LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR_AFTER_CXX17 inline _LIBCPP_INLINE_VISIBILITY
 void
----------------
miscco wrote:
> Many inline below here
Again, I guess it has been addressed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68365