[libcxx-commits] [PATCH] D70343: Add a `_LIBCPP_HARDEN` define

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF requested changes to this revision.
EricWF added inline comments.
This revision now requires changes to proceed.


================
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];
+    }
----------------
jdoerrie wrote:
> 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];
>     }
> ```
> 
`string_view` doesn't imply C++17. We support it in all dialects.


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

https://reviews.llvm.org/D70343