[libcxx-commits] [PATCH] D113161: [libc++] Implement P1989R2: range constructor for string_view

[Joe Loser via Phabricator via libcxx-commits]

jloser added a comment.

In D113161#3129568 <https://reviews.llvm.org/D113161#3129568>, @jloser wrote:

> I don't yet fully understand why CI is failing some range concepts that were previously true for the type `const std::filesystem::path&` in a few tests. It's only when the `std::string_view` constructor is implemented; they work fine without it (since that's just top-of-tree from `libc++` library code perspective).
>
> I //think// one of them is a Clang bug in eager instantiation of constraints in `libcxx/test/std/input.output/filesystems/class.path/range_concept_conformance.compile.pass.cpp` - specifically these below. They all fail, but only when the new `std::string_view` constructor is implemented.
>
>   static_assert(std::same_as<std::ranges::iterator_t<fs::path const>, fs::path::const_iterator>);
>   static_assert(std::ranges::common_range<fs::path const>);
>   static_assert(std::ranges::bidirectional_range<fs::path const>);
>
> The GCC11 CI failures are due to a change in behavior with `ranges::begin` for `const std::filesystem::path` that I don't yet understand.  Possibly a `libc++` bug in `ranges::begin`?
>
> Any ideas, @Quuxplusone @cjdb @ldionne?

Something is up with `std::ranges::begin` with this new constructor. Consider the following snippet:

  template<class T>
  concept can_call_ranges_begin = requires(T& t) {
    std::ranges::begin(t);
  };
  
  static_assert(can_call_ranges_begin<fs::path>);
  static_assert(can_call_ranges_begin<const fs::path>);

The two `static_assert`s pass on top of tree as you'd except (since both `path` and `const path` satisfy `std::ranges::range` concept after all). However, with this new `string_view` constructor, the second `static_assert` fails. That is, none of the three `operator()` in the `ranges::begin` definition are satisfied, so we hard error and fall through to the deleted `operator()`. I don't yet understand why this changes with the new `string_view` constructor though. Any hints, @Quuxplusone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113161