[libcxx-commits] [PATCH] D60249: [pstl] Replace direct use of assert() with __PSTL_ASSERT

[Thomas Rodgers via Phabricator via libcxx-commits]

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


================
Comment at: pstl/include/pstl/internal/pstl_config.h:17-28
+#if defined(PSTL_USE_ASSERT)
+#    undef __PSTL_USE_ASSERT
+#    define __PSTL_USE_ASSERT PSTL_USE_ASSERT
+#    include <cassert>
+#    define __PSTL_ASSERT(pred) (assert((pred)))
+// TODO make this smarter
+#    define __PSTL_ASSERT_MSG(pred, msg) (assert((pred)))
----------------
ldionne wrote:
> I would do this:
> 
> ```
> #if !defined(_PSTL_ASSERT)
> #  include <cassert>
> #  define _PSTL_ASSERT(x) assert(x)
> #endif
> ```
> 
> And I would just use `_PSTL_ASSERT(x)` elsewhere in the code. I don't see a reason to have a separate `_PSTL_USE_ASSERT` macro if we can define `_PSTL_ASSERT` directly. Also, whether `NDEBUG` is defined will control whether `_PSTL_ASSERT` does something by default, so the default is fine for unit tests. WDYT?
That works for me. I will be injecting the definitions for __PSTL_ASSERT() and __PSTL_ASSERT_MSG() from libstdc++'s configuration anyway.

This is really just to try and provide a sane default the standalone impl. I'll update the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60249