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

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/std/input.output/filesystems/class.path/range_concept_conformance.compile.pass.cpp:13-16
+// This test fails due to atomic constraints caching on clang-cl. Specifically, the constraints of
+// std::ranges::range concept changes over the lifetime of this TU. Similar story with libc++ and MinGW.
+// XFAIL: target={{.+}}-w64-windows-gnu
+// XFAIL: msvc
----------------
jloser wrote:
> mizvekov wrote:
> > Quuxplusone wrote:
> > > This sounds sketchy to me. Can you or @mizvekov explain further? (Explain in a thread here, that is. The code comment can be amended, if needed, after I get it :))
> > > 
> > > This test just includes a couple of libc++ headers and then tests some properties of `fs::path`. If this simple stuff doesn't work, then that smells like a libc++ bug (still), not something for the end-user to work around. In particular, this smells like we still have the same kind of issue as in `__lhs.compare(__rhs)`, but with some other member function, and in a codepath that gets compiled only on Windows... no?
> > I don't think caching is any different between clang and clang-cl.
> > 
> > I think the initial issue, from what I read about it, has to do with concepts satisfaction caching and how clang does not implement any kind of invalidation of the cache.
> > 
> > This could be tested by passing frontend command line switch `-fno-concept-satisfaction-caching`, or if using the driver, `-Xclang -fno-concept-satisfaction-caching`.
> > 
> > But the difference in behavior between clang and clang-cl must be something else.
> I'll precursor my comment with the fact that I don't have a great way to test the `clang-cl` or `MinGW` failing builds locally. It was failing for similar reasons with atomic constraints caching on CI. I think it's a similar issue with the `__lhs.compare(__rhs)` as you mentioned that is only a codepath for when `_LIBCPP_WIN32API` is defined.
> 
> I've reproduced the issue locally with `ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_WIN32API=1` to emulate a "windows only code path" that this error shows up. I'll try debugging it soon to figure out the problem.
Also just for future reference, if you want to XFAIL something for windows overall (both clang-cl and mingw), it's enough with just `XFAIL: windows`, or `XFAIL: {{.+}}-windows{{.*}}` for regex matching the triple for both variants).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113161