[libcxx-commits] [PATCH] D119633: [libc++] Remove recursion in basic_string::insert(const_iterator, ForwardIterator, ForwardIterator)

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

I think this code looks correct enough, but it would be a lot less scary if you could just make `__addr_in_range` a non-constexpr function, instead of having it "lie" during constexpr evaluation and get us into infinite loops. E.g.:

  if (__libcpp_is_constant_evaluated() || !__string_is_trivial_iterator<_ForwardIterator>::value || __addr_in_range(*__first)) {
      // Make a copy and then insert from the copy.
      const basic_string __temp(__first, __last, __alloc());
      return __insert_from_safe_copy(__n, __ip, __temp.begin(), __temp.end());
  } else {
      return __insert_from_safe_copy(__n, __ip, __first, __last);
  }

Note that `__insert_from_safe_copy` might be a better name than `__insert_not_in_range`, because being overlapping is just //one// of the many failure modes we need to guard against here.

Read (or re-read) https://quuxplusone.github.io/blog/2021/04/17/pathological-string-appends/ before deciding on this approach. "Burn the whole thing to the ground and rewrite it" //might// be a reasonable alternative, especially if the alternative is //increasing// the complexity of this already-complex code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119633