[libcxx-commits] [PATCH] D147089: [libc++] Add assertions for potential OOB reads in std::sort

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D147089#4344102 <https://reviews.llvm.org/D147089#4344102>, @alexfh wrote:

> Hi Louis, we've started seeing compilation errors in code using custom iterators after this patch:
>
>   .../include/c++/v1/__algorithm/sort.h:681:14 no viable overloaded '='
>       *__begin = _Ops::__iter_move(__pivot_pos);
>
> One example that relies on an open-source library is here https://gcc.godbolt.org/z/be7xMhcn3, but we have other instances as well. Is this a intended effect of this patch? If so, can we have an escape hatch?
>
> FWIW, the error doesn't depend on whether `_LIBCPP_ENABLE_ASSERTIONS` is enabled.

Thanks for reducing to a godbolt! This happens because `__begin` was made `const` in the new code and we're assigning to the result of `*__begin`. This is required to be valid since `operator*` on iterators is supposed to be const (see https://en.cppreference.com/w/cpp/iterator/indirectly_readable). So the code that fails should be fixed.

That being said, I can make `__begin` non-const for the time being to give you time to fix up these places, but I'll mark it with a TODO to make `const` again in LLVM 18. How does that sound?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147089