[libcxx-commits] [PATCH] D93912: [libc++][P1679] add string contains

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

In D93912#2507132 <https://reviews.llvm.org/D93912#2507132>, @zoecarver wrote:

>> It's unclear to me if I should add noexcept to the const char* overloads.
>
> I think @mclow.lists is right. We shouldn't have noexcept here, and we also need to remove it from some of the `__str_find` overloads (as a separate patch, which I'm happy to make).

@WimLeflere  Don't add `noexcept`. Your patch is fine as-is -- we'll tackle the `noexcept` story separately.

In D93912#2505552 <https://reviews.llvm.org/D93912#2505552>, @mclow.lists wrote:

> Ok, I take it back. `__str_find` calls `Traits::find`, which is NOT always noexcept.  (It *is* for all the specializations of `std::char_traits`, but that's not enough). So we can't slap `NOEXCEPT` here; in fact, we should revisit `__str_find`, and remove some of the `NOEXCEPT`s that are already there.

Hmm, indeed. `std::char_traits::find` is marked as `constexpr` since C++17, but technically it could still throw at runtime. @zoecarver Feel free to submit a patch to remove it if you want. You could also audit the other helper functions (like `__str_rfind` - I see there's a bunch) for the same problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93912