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

Tue Feb 15 08:50:41 PST 2022

philnik added inline comments.

================
Comment at: libcxx/include/CMakeLists.txt:102
   __algorithm/upper_bound.h
+  __assert
   __availability
----------------
Quuxplusone wrote:
> 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. :)
I'm definitely in favor of moving that stuff into some directory. Maybe `__libcpp/`? That would most likely not clash with any new C++ headers and we could leave the `libcpp_` prefix out.

================
Comment at: libcxx/include/__debug:27

-#if _LIBCPP_DEBUG_LEVEL == 0
+#if _LIBCPP_DEBUG_LEVEL < 2
 #   define _LIBCPP_DEBUG_ASSERT(x, m) ((void)0)
----------------
Quuxplusone wrote:
> This `<` allows people to set `-D_LIBCPP_DEBUG_LEVEL=-1` without triggering the `#error` on line 32.
`_LIBCPP_DEBUG_LEVEL` should never be set directly. The public interface is `_LIBCPP_DEBUG`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119769