[libcxx-commits] [PATCH] D70343: Add a `_LIBCPP_HARDEN` define
Jan Wilken Dörrie via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Nov 15 17:27:23 PST 2019
jdoerrie added a comment.
In D70343#1748522 <https://reviews.llvm.org/D70343#1748522>, @EricWF wrote:
> What's the difference between `_LIBCPP_HARDEN` and simply defining `_LIBCPP_DEBUG=0`? That should turn on `_LIBCPP_ASSERT` in the same cases.
I believe the intent of `_LIBCPP_HARDEN` is to turn asserts into program terminations. Ideally this happens as quickly as possible with as little overhead as possible. Thus we directly call `__builtin_trap()` instead of going through `_VSTD::__libcpp_debug_function`. Furthermore, I believe we were under the impression that defining `_LIBCPP_DEBUG=0` turns on other checks as well, that we are not necessarily interested in. Please correct me if I'm wrong.
================
Comment at: libcxx/include/string_view:282
+ const_reference operator[](size_type __pos) const _NOEXCEPT {
+ return _LIBCPP_ASSERT(size() == 0 || __pos < size(), "string_view[] index out of bounds"), __data[__pos];
+ }
----------------
Checking that `size() == 0` seems like a mistake, as otherwise you would allow arbitrary indices for an empty string_view. This should just be `_LIBCPP_ASSERT(__pos < size(), "...");` unless I'm missing something obvious.
Given that string_view implies C++17 or newer, you should also be able to use multiple expressions in a constexpr function. IMO the following is easier to read:
```
_LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
const_reference operator[](size_type __pos) const _NOEXCEPT {
_LIBCPP_ASSERT(__pos < size(), "string_view[] index out of bounds");
return __data[__pos];
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70343/new/
https://reviews.llvm.org/D70343
More information about the libcxx-commits
mailing list