[libcxx-commits] [PATCH] D121478: [libc++] Add a lightweight overridable assertion handler

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked an inline comment as done.
ldionne added inline comments.


================
Comment at: libcxx/include/__assert:32
+
+#if _LIBCPP_ENABLE_ASSERTIONS != 0 && _LIBCPP_ENABLE_ASSERTIONS != 1
+# error "_LIBCPP_ENABLE_ASSERTIONS must be set to 0 or 1"
----------------
smeenai wrote:
> Was the intent here to also catch cases where `_LIBCPP_ENABLE_ASSERTIONS` is undefined because `_LIBCPP_ENABLE_ASSERTIONS_DEFAULT` was undefined? If so, this check won't catch that, because the undefined `_LIBCPP_ENABLE_ASSERTIONS` macro will just evaluate to 0 here. `_LIBCPP_ENABLE_ASSERTIONS_DEFAULT` is defined in `__config_site` normally so you wouldn't run into this, but I'm not sure what the desired behavior was if that ended up being undefined (e.g. because you're using a different build system).
Sorry for not noticing that earlier. I believe my intent was to avoid confusion with `_LIBCPP_DEBUG`, which allowed other values like 2 to be specified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121478