[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]

Quuxplusone created this revision.
Quuxplusone added a reviewer: libc++.
Quuxplusone added a project: libc++.
Quuxplusone requested review of this revision.
Herald added a subscriber: libcxx-commits.
Herald added 1 blocking reviewer(s): libc++.

This reverts a large chunk of D15862 <https://reviews.llvm.org/D15862> ,
and also fixes a bug in `insert`, which is now regression-tested.

Before this patch, we did a special dance in `append`, `assign`, and `insert`
(but not `replace`). All of these require the strong exception guarantee,
even when the user-provided InputIterator might have throwing operations.

The naive way to accomplish this is to construct a temporary string and
then append/assign/insert from the temporary; i.e., finish all the potentially
throwing InputIterator operations *before* starting to modify the `*this`
object. This is our default approach.

However, if we know that the InputIterator operations can't throw (because
the InputIterator is "trivial" i.e. libc++-provided), then it's safe to
iterate using the original InputIterator while modifying the `*this` object,
and we don't need the temporary copy. (At least, not for that reason. We
might still need it to avoid self-assignment, or because the InputIterator
isn't a forward iterator.) Call this the "optimized" approach.

For `append`, we can always use the optimized approach, because if an iterator
operation throws, we just restore the null terminator and we're back where
we started, no problem.

For `assign` and `insert`, we cannot always use the optimized approach.
IMO it would be reasonable to always use the default approach -- I think that's
what libstdc++ does -- but I've preserved the old behavior. I've just changed
the SFINAE check from "are all the relevant iterator operations sufficiently noexcept?"
to "is the iterator a 'trivial' iterator known to libc++?" The old condition
was so complicated that of course it was wrong; the new regression test
in `basic.string/string.modifiers/string_insert/strong_guarantee.pass.cpp`
fails without this patch.

( Background: https://stackoverflow.com/questions/66459162/where-do-standard-library-or-compilers-leverage-noexcept-move-semantics-other-t/66498981#66498981 )


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98573

Files:
  libcxx/include/filesystem
  libcxx/include/string
  libcxx/test/libcxx/strings/iterators.exceptions.pass.cpp
  libcxx/test/libcxx/strings/iterators.noexcept.pass.cpp
  libcxx/test/std/strings/basic.string/string.modifiers/string_insert/strong_guarantee.pass.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98573.330418.patch
Type: text/x-patch
Size: 21962 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20210313/bf8b012b/attachment-0001.bin>