[libcxx-commits] [PATCH] D129823: [libc++][ranges] Make range algorithms support proxy iterators

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Thanks everyone for chiming in on this issue. I am speaking to @var-const right now who summarized the issues to me. It looks like there is some UB in MySQL right now because their funky iterator type is lying about `iterator_traits<It>::reference`, and that's really the crux of the problem IMO. That needs to be fixed, and it's not something that libc++ can accommodate by making the code work.

**However**, it also seems like it's something that we can easily catch at compile-time instead, hence my suggestion <https://reviews.llvm.org/D130538#inline-1256036> to add a `static_assert` in D130538 <https://reviews.llvm.org/D130538>. It will still break Chromium via its MySQL dependency, however the error will clearly point out how it is broken and how to fix it.

@rupprecht If you are able to get us a reproduction of the failure on top of D130538 <https://reviews.llvm.org/D130538> (without the `static_assert`), we'd be interested in taking a look to understand whether there's something more subtle at play. However, our current understanding is that D130538 <https://reviews.llvm.org/D130538> + `static_assert` will work in all cases (i.e. either it will work as intended, or it will issue a compile-time diagnostic). If you see additional issues after we move forward with D130538 <https://reviews.llvm.org/D130538>, please let us know via the usual means. This has been really useful in finding subtle problems early and we definitely want to address such issues with the highest priority.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129823