[libcxx-commits] [PATCH] D98573: [libc++] Remove most of the special logic for "noexcept iterators" in basic_string

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri Apr 16 13:01:58 PDT 2021


Quuxplusone added inline comments.


================
Comment at: libcxx/include/string:2670
+        if (__needs_grow && (!__libcpp_is_trivial_iterator<_ForwardIterator>::value ||
+                             _VSTD::__ptr_in_range(_VSTD::addressof(__tmp_ref), data(), data() + size())))
         {
----------------
Quuxplusone wrote:
> Quuxplusone wrote:
> > It occurred to me just now (and I've asked in #standardese on Slack) that this condition is insufficient to deal with the trailing null byte, which is accessible but not part of the range `[data, data+size)`. Here are two problematic cases in the wild today:
> > https://godbolt.org/z/q6dnKEvqc (libstdc++ seems to fail but only on Clang, somehow)
> > https://godbolt.org/z/da7zq58c6 (libc++ fails)
> > If these cases are legitimate, then I might need to continue messing with this code.
> Botheration! I don't think we can use //any// of the fast paths here.
> https://godbolt.org/z/e8hTj5Ysf
> http://eel.is/c++draft/string.modifiers#string.append-14 is super clear that the effects of `s.append(first, last)` must be the same as the naïve `s.append(string(first, last))`; and it's super easy to create a pathological iterator where `*__first` is //not// in the range `[data, data+size]` but `*(__first+1)` //is// in that range.
Oh yeah, duh. https://godbolt.org/z/TdhnTeq1Y
`__libcpp_trivial_iterator` ensures that the iterator operations are noexcept, but it completely fails to check that the //conversion to char// is noexcept, so we lose the strong guarantee there, as well. Sounds like we have a good excuse to kill off not only `__libcpp_string_gets_noexcept_iterator<T>` but also `__libcpp_is_trivial_iterator<T>`.

Well, this has certainly been a can of worms!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98573



More information about the libcxx-commits mailing list