[PATCH] D93912: [libc++][P1679] add string contains
Arthur O'Dwyer via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 18 13:07:27 PST 2021
Quuxplusone added a comment.
@ldionne: LGTM except that the non-constexpr tests no longer need to `return true;` from `bool test()` — they could do `void test()` for now.
Also, I don't think this will rebase as-is on generate_feature_test_macro_components.py, will it? You'll have to add the entry for `__cpp_lib_string_contains`, which will be easy — but I notice that it //will// be a divergence from what's literally in https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations as of this writing. I would bet that's fine; I'm just bringing it up for awareness.
The current direction on constexpr/noexcept sounds great to me.
================
Comment at: libcxx/test/std/strings/basic.string/string.contains/contains.char.pass.cpp:35
+
+ return true;
+}
----------------
Nit: you can remove the `bool` and `return true` for now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93912/new/
https://reviews.llvm.org/D93912
More information about the llvm-commits
mailing list