[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#3145181 <https://reviews.llvm.org/D113161#3145181>, @Quuxplusone wrote:

> @jloser: Found it! You need to do this in `<filesystem>`:
>
>      friend _LIBCPP_INLINE_VISIBILITY bool operator==(const path& __lhs, const path& __rhs) noexcept {
>   -    return __lhs.compare(__rhs) == 0;
>   +    return __lhs.__compare(__rhs.__pn_) == 0;
>      }
>
> (and likewise for each of `path`'s six comparison operators).
> The problem is that instantiating `path` instantiates `operator==`, which triggers overload resolution on `__lhs.compare(__rhs)`, which evaluates whether `__rhs` can be converted to `string_view`... and at that point, it //cannot// (because its iterator type is still incomplete at that point). So, it goes boom. The hack-around is to make sure we don't call anything that might ever try to convert `path` to `string_view` before its time.

Ah! That makes total sense. I think the thing that threw me off is the `const`ness of the problem. It works fine for `path` but not `const path` because of the const-qualified-ness of the `compare` functions, right?

I just pushed a fix with these `path` operator changes - let's see what CI thinks. I still need to test locally with llvm + this patched libc++ to see whether we have compile errors in the mentioned bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113161