[libcxx-commits] [PATCH] D119769: [libc++] Move everything related solely to _LIBCPP_ASSERT to its own file

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

Seems reasonable to me.



================
Comment at: libcxx/include/CMakeLists.txt:102
   __algorithm/upper_bound.h
+  __assert
   __availability
----------------
Are we at all worried about name conflicts? (Or stylistically about adding new top-level detail headers?) Should this be something like `__utility/libcpp_assert.h` instead?
OTOH, maybe we keep it like this right now, and then at some point in the future we move `__assert` to `__foo/libcpp_assert.h` and `__debug` to `__foo/debug_iterators.h` in the same commit, for some value of `__foo` on which I'm sure we'll disagree. :)


================
Comment at: libcxx/include/__debug:27
 
-#if _LIBCPP_DEBUG_LEVEL == 0
+#if _LIBCPP_DEBUG_LEVEL < 2
 #   define _LIBCPP_DEBUG_ASSERT(x, m) ((void)0)
----------------
This `<` allows people to set `-D_LIBCPP_DEBUG_LEVEL=-1` without triggering the `#error` on line 32.


================
Comment at: libcxx/src/assert.cpp:19
+  string msg = __file_;
+  msg += ":" + to_string(__line_) + ": _LIBCPP_ASSERT '";
+  msg += __pred_;
----------------
Consider `std::to_string(__line_)` for consistency with `std::fprintf` and `std::abort` on lines 27-28.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119769