Page MenuHomePhabricator

Remove noexcept from basic_string::find and implementation functions.
AbandonedPublic

Authored by zoecarver on Jan 22 2021, 11:12 AM.

Details

Reviewers
mclow.lists
ldionne
Group Reviewers
Restricted Project
Summary

This patch removes noexcept from all fiver basic_string::find overloads
and all overloads of search_substring and str_find.

The standard requires that the following overloads have a noexcept
attribute, but as I demonstrate in the tests, this might have an
undesirable effect for char_traits that throw. Does it make sense for
me to file an LWG issue about this? Here are the two overloads that are
suppose to have noexcept:

size_type find(value_type __c, size_type __pos = 0)
size_type find(const basic_string& __str, size_type __pos = 0) const

Once this patch is approved I'll apply similar updates to rfind, find_first_of, etc.

Diff Detail

Event Timeline

zoecarver requested review of this revision.Jan 22 2021, 11:12 AM
zoecarver created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2021, 11:12 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Addresses a comment in D93912.

libcxx/include/string
1274

I won't commit these commented-out _NOEXCEPTs. This is just to make it easier to see which are supposed to be here according to the standard.

  • If the standard says they should be noexecpt, we should keep them noexcept.
  • Yes, file an LWG issue.
  • Yes, file an LWG issue.

In your issue, be sure to ask for clarification of [char.traits.require]/1's "In Table 71, X denotes a traits class defining types and functions for the character container type C ... Operations on X shall not throw exceptions.". I've always interpreted this to mean that the operations in the table shall not throw, but I'd have a hard time arguing that's the intended meaning of "operations on X".

  • Yes, file an LWG issue.

In your issue, be sure to ask for clarification of [char.traits.require]/1's "In Table 71, X denotes a traits class defining types and functions for the character container type C ... Operations on X shall not throw exceptions.". I've always interpreted this to mean that the operations in the table shall not throw, but I'd have a hard time arguing that's the intended meaning of "operations on X".

Hmm interesting. I didn't even think to look at the char traits. I suppose that paragraph could be more clear, but what, other than "calling static members," could "operations on X" be referring to (especially in the context of throwing exceptions)?

In the issue, I'll ask for clarification and propose either that all these find-like members are marked noexcept or none of them are. The thing that's really making me uneasy is that two of them are and the rest aren't.

In the issue, I'll ask for clarification and propose either that all these find-like members are marked noexcept or none of them are. The thing that's really making me uneasy is that two of them are and the rest aren't.

It's probably a wide vs. narrow contract thing. The find overloads that take const char*, for example, have a precondition that the pointer is non-null. Under the so-called "Lakos rule" they would/could/should be "Throws: Nothing" but not noexcept.

In the issue, I'll ask for clarification and propose either that all these find-like members are marked noexcept or none of them are. The thing that's really making me uneasy is that two of them are and the rest aren't.

It's probably a wide vs. narrow contract thing. The find overloads that take const char*, for example, have a precondition that the pointer is non-null. Under the so-called "Lakos rule" they would/could/should be "Throws: Nothing" but not noexcept.

that was my thought, too.
However, having a noexcept function that is required to call a non-noexcept function is madness :-)

It's probably a wide vs. narrow contract thing. The find overloads that take const char*, for example, have a precondition that the pointer is non-null. Under the so-called "Lakos rule" they would/could/should be "Throws: Nothing" but not noexcept.

Sure, that makes sense, and if the find function was entirely defined inside of the function (didn't make any calls to other functions), I think that would be OK. But, if I understand correctly (and that's a big if), then this doesn't have a narrow contract because a custom traits::find (depending on the wording of [char.traits.require]/1) could throw for any reason (it doesn't have a documented contract, it still might throw if the pointer is not-null).

So (again, if I understand correctly), I think all of these have a wide contract and should either be expected to throw or be marked noexcept.

It's probably a wide vs. narrow contract thing. The find overloads that take const char*, for example, have a precondition that the pointer is non-null. Under the so-called "Lakos rule" they would/could/should be "Throws: Nothing" but not noexcept.

Yes, exactly this. The string and string_view overloads all have correct noexcept specifiers in the standard (and we've relitigated that several times already). Operations on char_traits cannot throw, whether they are noexcept or not. But due to the Lakos rule, they can't all be marked noexcept (LEWG have agreed to change the rule, but that hasn't changed anything yet and I do not want to use the issues list to add noexcept to "*Throws: nothing" functions one-by-one throughout the library). I agree with Casey's interpretation of "Operations on X shall not throw exceptions." But I also think it could be clearer so that nobody interprets it another way.

If libc++ doesn't want to use exceptions to diagnose precondition violations in the find members that have narrow contracts, then just add noexcept to all your find overloads (which is what libstdc++ does). The standard is correct though.

So (again, if I understand correctly), I think all of these have a wide contract and should either be expected to throw or be marked noexcept.

No. The ones that construct a basic_string_view from a const CharT* (with or without a length) inherit the *Preconditions:* from the basic_string_view constructor. If they have a precondition, they have a narrow contract, by definition.

And those are the ones that are not marked noexcept in the standard (but can be marked noexcept in an implementation). It's OK for implementations to add noexcept to functions that can't throw, even if the standard doesn't say it's noexcept.

zoecarver abandoned this revision.Mon, Feb 1, 2:19 PM

@jwakely thanks for those comments. Super helpful/informative.

Here's the associated LWG issue to clear up the wording (thanks Daniel!): 3518. And here's the patch to clarify libc++'s policy on this going forward: D95821.

I'm going to close this patch, but I'll create another one to apply noexcept to all of these functions (and friends).