[libcxx-commits] [libcxx] [libc++][hardening] Don't trigger redundant checks in the fast mode. (PR #77176)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jan 9 13:29:22 PST 2024


================
@@ -331,6 +343,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
 #    define _LIBCPP_ASSERT_PEDANTIC(expression, message)                 _LIBCPP_ASSUME(expression)
 #    define _LIBCPP_ASSERT_INTERNAL(expression, message)                 _LIBCPP_ASSUME(expression)
 #    define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message)            _LIBCPP_ASSUME(expression)
+#    define _LIBCPP_REDUNDANT_ASSERTION(expression)                      _LIBCPP_ASSUME(expression)
----------------
ldionne wrote:

If `_LIBCPP_ASSUME` actually did something at all, this would pessimize the code by discarding the assume information. We could fix it with something like:

```c++
#if fast-mode
# define _LIBCPP_REDUNDANT_ASSERTION(do_assert, expression) _LIBCPP_ASSUME(expression)
#elif extensive-mode
# define _LIBCPP_REDUNDANT_ASSERTION(do_assert, expression) do_assert(expression)
#elif ...
  // ...
#endif
```

Then call it as `_LIBCPP_REDUNDANT_ASSERTION(_LIBCPP_ASSERT_NON_NULL, __imp_ != nullptr, "The end iterator cannot be dereferenced")`.

But this is really ugly. I would be OK with dropping the assume information since we don't use it at all right now.

However, there's currently a bug in how this is written. I think this needs to be

```c++
#if fast-mode
# define _LIBCPP_REDUNDANT_ASSERTION(expression) ((void)0)
#elif extensive-mode
# define _LIBCPP_REDUNDANT_ASSERTION(expression) expression
#elif ...
  // ...
#endif
```

We should add a test for this similar to `libcxx/test/libcxx/assertions/single_expression.pass.cpp` (maybe in the same file).

https://github.com/llvm/llvm-project/pull/77176


More information about the libcxx-commits mailing list