[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
Mon Apr 12 12:05:54 PDT 2021


Quuxplusone planned changes to this revision.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/string:2662
+        if (__ptr_in_range(_VSTD::addressof(__tmp_ref), data(), data() + size()) ||
+            ((!__libcpp_is_trivial_iterator<_ForwardIterator>::value) && (__cap - __sz < __n)))
         {
----------------
ldionne wrote:
> Nit: I find the parens around `!__libcpp_is_trivial_iterator<_ForwardIterator>::value` confusing, are you OK with removing them?
> 
> Also, I would add a short comment saying something like
> 
> ```
> // If we're appending from a range inside this string or if the iterators have non-trivial operations AND we need to allocate to complete the operation, make a temporary copy so we can provide the strong exception guarantee.
> ```
> 
> ... or something like that. You're much better at English than I am, but I think a comment would be helpful since this line is quite packed with important information.
Re parens: I prefer never to write `!x && y` because I believe people often thinko that when they meant `!(x && y)`. However, swapping them to give `(y && !x)` has no such issue, and in this case it's safe to write it that way, so I probably will do that.

Between this and your comment below, I think I need to revisit this patch and make sure //I// still understand the situations in which these codepaths are hit! :) I'll repost with more code comments.


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