[PATCH] D93912: [libc++][P1679] add string contains

[Marek Kurdej via Phabricator via llvm-commits]

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


================
Comment at: libcxx/include/string:1442
+    _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
+    bool contains(__self_view __sv) const _NOEXCEPT
+    { return __self_view(data(), size()).contains(__sv); }
----------------
Mordante wrote:
> Wouldn't it be better to use `constexpr` and `noexcept` since the code is never compiled using C++98.
@WimLeflere, please use `constexpr` and `noexcept` directly as suggested above.


================
Comment at: libcxx/test/std/strings/basic.string/string.contains/contains.char.pass.cpp:21
+{
+    using S = std::string;
+
----------------
You cannot just use std::string as it's not constexpr-friendly (yet), but need to use a constexpr-loving allocator or do some other hacks.


================
Comment at: libcxx/test/std/strings/string.view/string.view.template/contains.string_view.pass.cpp:8
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
----------------
You should still use "c++2a" and not "c++20" as it's derived from the flag name (and we still use c++2a).
The same applies to all other tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93912