[libcxx-commits] [PATCH] D110718: [libc++] Implement P1391

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/string_view:281
 
+#if _LIBCPP_STD_VER > 17
+    template <contiguous_iterator _It, sized_sentinel_for<_It> _End>
----------------



================
Comment at: libcxx/include/string_view:693
+
+#if _LIBCPP_STD_VER > 17
+template <contiguous_iterator _It, sized_sentinel_for<_It> _End>
----------------



================
Comment at: libcxx/test/std/strings/string.view/string.view.cons/deduct.pass.cpp:10
+// UNSUPPORTED: libcpp-no-concepts
+// XFAIL: apple-clang-12.0
+
----------------
jloser wrote:
> Mordante wrote:
> > Minor nit, XFAIL isn't required, since `// UNSUPPORTED: libcpp-no-concepts` achieves the same.
> Are you sure? Without the `XFAIL`, I was seeing CI failures only for `AppleClang 12.0.0` as it couldn't resolve `contiguous_iterator` from what I could tell. The build failure link is https://buildkite.com/llvm-project/libcxx-ci/builds/5619.
> 
> BTW, we're about to drop support for `AppleClang 12.0.0` as I talked with @ldionne the other day. `AppleClang 13.0.0` just came out on 9/20 IIRC which has full support for concepts and then we can remove this `XFAIL`.
Actually, I believe @Mordante is correct here. We can remove the `XFAIL: apple-clang-12` since `libcpp-no-concepts` handle it. What you were seeing in https://buildkite.com/llvm-project/libcxx-ci/builds/5619 is a failure to build the library, we were not even getting to this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110718