[libcxx-commits] [PATCH] D110598: [libc++] P0980R1 (constexpr std::string)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik marked an inline comment as not done.
philnik added inline comments.


================
Comment at: libcxx/include/__memory/compressed_pair.h:61-62
 
-
-  _LIBCPP_INLINE_VISIBILITY reference __get() _NOEXCEPT { return __value_; }
-  _LIBCPP_INLINE_VISIBILITY
-  const_reference __get() const _NOEXCEPT { return __value_; }
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 reference __get() _NOEXCEPT { return __value_; }
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 const_reference __get() const _NOEXCEPT { return __value_; }
 
----------------
Quuxplusone wrote:
> For internal implementation details such as `__compressed_pair`, please be aggressive in adding `constexpr` (anywhere it's actually needed, of course). On these two lines, you can use `_LIBCPP_CONSTEXPR` instead of `_LIBCPP_CONSTEXPR_AFTER_CXX17`. The benefit is that on that far-future day where we drop support for C++03, we can just mechanically change all `_LIBCPP_CONSTEXPR` to `constexpr`; whereas we can't mechanically change `_LIBCPP_CONSTEXPR_AFTER_CXX17` to `constexpr` until we drop support for C++17!
> 
> For user-facing (public, standards-conforming) bits, of course we have to apply constexpr conservatively.
I'm not sure what the problem is, but the test for C++11 fails (it think) because of the change to## _LIBCPP_CONSTEXPR##. C++03, C++14 and C++17 work fine. C++20 and C++2b most likely fail because constexpr string just doesn't work properly. Should I just change it to ##_LIBCPP_CONSTEXPR_AFTER_CXX11##? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110598