[libcxx-commits] [PATCH] D65721: Make rotate a constexpr

Mon May 18 08:34:34 PDT 2020

zoecarver marked 4 inline comments as done.
zoecarver added inline comments.

================
Comment at: libcxx/include/algorithm:1682
 template <class _Tp>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 typename enable_if
----------------
zoecarver wrote:
> ldionne wrote:
> > Does this break the library when used with the debug mode? There must have been a reason why we added `_LIBCPP_CONSTEXPR_IF_NODEBUG` in the first place?
> > 
> > @ericwf Do we care about this? If not, it looks like now's the time to remove the debug mode (for something better) as we discussed.
> I think it should be the opposite: the tests fail in debug mode because this is no longer constexpr. I'll test it, though. 
The tests all pass with `_LIBCPP_DEBUG=1`. 

================
Comment at: libcxx/include/algorithm:1863
 template <class _InputIterator, class _OutputIterator>
-inline _LIBCPP_INLINE_VISIBILITY
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 _OutputIterator
----------------
These all need to be `_LIBCPP_CONSTEXPR_AFTER_CXX17`. Otherwise, the compiler will complain about the loops in pre C++20 mode. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65721