Remove noexcept from basic_string::find and implementation functions.

[zoecarver]

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.

[zoecarver]

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.

[mclow.lists]

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

[CaseyCarter]

In D95251#2516190, @mclow.lists wrote:

• 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".

[zoecarver]

In D95251#2516236, @CaseyCarter wrote:

In D95251#2516190, @mclow.lists wrote:

• 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.

[CaseyCarter]

In D95251#2516256, @zoecarver wrote:

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.

[mclow.lists]

In D95251#2516668, @CaseyCarter wrote:

In D95251#2516256, @zoecarver wrote:

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 :-)

[zoecarver]

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.

[jwakely]

In D95251#2516668, @CaseyCarter wrote:

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.

[jwakely]

In D95251#2517099, @zoecarver wrote:

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]

@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).

libcxx/include/__string

 #endif  // _LIBCPP_HAS_NO_UNICODE_CHARS
 
 // helper fns for basic_string and string_view
 
 // __str_find
 template<class _CharT, class _SizeT, class _Traits, _SizeT __npos>
 inline _SizeT _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
 __str_find(const _CharT *__p, _SizeT __sz,
-             _CharT __c, _SizeT __pos) _NOEXCEPT
+             _CharT __c, _SizeT __pos)
 {
     if (__pos >= __sz)
         return __npos;
     const _CharT* __r = _Traits::find(__p + __pos, __sz - __pos, __c);
     if (__r == nullptr)
         return __npos;
     return static_cast<_SizeT>(__r - __p);
 }
while (true) {
 
     ++__first1;
   }
 }
 
 template<class _CharT, class _SizeT, class _Traits, _SizeT __npos>
 inline _SizeT _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
 __str_find(const _CharT *__p, _SizeT __sz,
-       const _CharT* __s, _SizeT __pos, _SizeT __n) _NOEXCEPT
+       const _CharT* __s, _SizeT __pos, _SizeT __n)
 {
     if (__pos > __sz)
         return __npos;
 
     if (__n == 0) // There is nothing to search, just return __pos.
         return __pos;
 
     const _CharT *__r = __search_substring<_CharT, _Traits>(

libcxx/include/string

#if _LIBCPP_STD_VER > 14 || defined(_LIBCPP_BUILDING_LIBRARY)
     _LIBCPP_INLINE_VISIBILITY
     value_type* data()             _NOEXCEPT  {return _VSTD::__to_address(__get_pointer());}
 #endif
 
     _LIBCPP_INLINE_VISIBILITY
     allocator_type get_allocator() const _NOEXCEPT {return __alloc();}
 
     _LIBCPP_INLINE_VISIBILITY
-    size_type find(const basic_string& __str, size_type __pos = 0) const _NOEXCEPT;
+    size_type find(const basic_string& __str, size_type __pos = 0) const; // _NOEXCEPT
 
     template <class _Tp>
     _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
     _EnableIf
         <
             __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
             size_type
         >
               find(const _Tp& __t, size_type __pos = 0) const;
-    size_type find(const value_type* __s, size_type __pos, size_type __n) const _NOEXCEPT;
+    size_type find(const value_type* __s, size_type __pos, size_type __n) const;
     _LIBCPP_INLINE_VISIBILITY
-    size_type find(const value_type* __s, size_type __pos = 0) const _NOEXCEPT;
-    size_type find(value_type __c, size_type __pos = 0) const _NOEXCEPT;
+    size_type find(const value_type* __s, size_type __pos = 0) const;
+    size_type find(value_type __c, size_type __pos = 0) const; // _NOEXCEPT
 
     _LIBCPP_INLINE_VISIBILITY
     size_type rfind(const basic_string& __str, size_type __pos = npos) const _NOEXCEPT;
 
     template <class _Tp>
     _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
     _EnableIf
         <
struct _LIBCPP_HIDDEN __traits_eq
     bool operator()(const char_type& __x, const char_type& __y) _NOEXCEPT
         {return _Traits::eq(__x, __y);}
 };
 
 template<class _CharT, class _Traits, class _Allocator>
 typename basic_string<_CharT, _Traits, _Allocator>::size_type
 basic_string<_CharT, _Traits, _Allocator>::find(const value_type* __s,
                                                 size_type __pos,
-                                                size_type __n) const _NOEXCEPT
+                                                size_type __n) const
 {
     _LIBCPP_ASSERT(__n == 0 || __s != nullptr, "string::find(): received nullptr");
     return __str_find<value_type, size_type, traits_type, npos>
         (data(), size(), __s, __pos, __n);
 }
 
 template<class _CharT, class _Traits, class _Allocator>
 inline
 typename basic_string<_CharT, _Traits, _Allocator>::size_type
 basic_string<_CharT, _Traits, _Allocator>::find(const basic_string& __str,
-                                                size_type __pos) const _NOEXCEPT
+                                                size_type __pos) const // _NOEXCEPT
 {
     return __str_find<value_type, size_type, traits_type, npos>
         (data(), size(), __str.data(), __pos, __str.size());
 }
 
 template<class _CharT, class _Traits, class _Allocator>
 template <class _Tp>
 _EnableIf
 <
     __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
     typename basic_string<_CharT, _Traits, _Allocator>::size_type
 >
 basic_string<_CharT, _Traits, _Allocator>::find(const _Tp &__t,
                                                 size_type __pos) const
 {
     __self_view __sv = __t;
     return __str_find<value_type, size_type, traits_type, npos>
         (data(), size(), __sv.data(), __pos, __sv.size());
 }
 
 template<class _CharT, class _Traits, class _Allocator>
 inline
 typename basic_string<_CharT, _Traits, _Allocator>::size_type
 basic_string<_CharT, _Traits, _Allocator>::find(const value_type* __s,
-                                                size_type __pos) const _NOEXCEPT
+                                                size_type __pos) const
 {
     _LIBCPP_ASSERT(__s != nullptr, "string::find(): received nullptr");
     return __str_find<value_type, size_type, traits_type, npos>
         (data(), size(), __s, __pos, traits_type::length(__s));
 }
 
 template<class _CharT, class _Traits, class _Allocator>
 typename basic_string<_CharT, _Traits, _Allocator>::size_type
 basic_string<_CharT, _Traits, _Allocator>::find(value_type __c,
-                                                size_type __pos) const _NOEXCEPT
+                                                size_type __pos) const // _NOEXCEPT
 {
     return __str_find<value_type, size_type, traits_type, npos>
         (data(), size(), __c, __pos);
 }
 
 // rfind
 
 template<class _CharT, class _Traits, class _Allocator>

libcxx/test/std/strings/basic.string/string.ops/string_find/char_size.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <string>

 void
 test(const S& s, typename S::value_type c, typename S::size_type x)
 {
     assert(s.find(c) == x);
     if (x != S::npos)
         assert(0 <= x && x + 1 <= s.size());
 }
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+template<class T>
+struct ThrowingTrait : std::char_traits<T>
+{
+    static const T*
+    find(const T*, std::size_t, const T&) {
+        throw 0;
+    }
+};
+
+template<class T>
+void
+test_trait_find_throws(const T *s) {
+    std::basic_string<T, ThrowingTrait<T>, std::allocator<T> > str = s;
+    try {
+        str.find('x');
+        assert(false);
+    } catch (int) { /* Do nothing... */ }
+}
+#endif // _LIBCPP_NO_EXCEPTIONS
+
 int main(int, char**)
 {
     {
     typedef std::string S;
     test(S(""), 'c', 0, S::npos);
     test(S(""), 'c', 1, S::npos);
     test(S("abcde"), 'c', 0, 2);
     test(S("abcde"), 'c', 1, 2);
#if TEST_STD_VER >= 11
 
     test(S(""), 'c', S::npos);
     test(S("abcde"), 'c', 2);
     test(S("abcdeabcde"), 'c', 2);
     test(S("abcdeabcdeabcdeabcde"), 'c', 2);
     }
 #endif
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    test_trait_find_throws("xyz");
+#endif // _LIBCPP_NO_EXCEPTIONS
+
   return 0;
 }

libcxx/test/std/strings/basic.string/string.ops/string_find/pointer_size.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <string>
test(const S& s, const typename S::value_type* str, typename S::size_type x)
     assert(s.find(str) == x);
     if (x != S::npos)
     {
         typename S::size_type n = S::traits_type::length(str);
         assert(0 <= x && x + n <= s.size());
     }
 }
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+template<class T>
+struct ThrowingTrait : std::char_traits<T>
+{
+    static const T*
+    find(const T*, std::size_t, const T&) {
+        throw 0;
+    }
+};
+
+template<class T>
+void
+test_trait_find_throws(const T *s) {
+    std::basic_string<T, ThrowingTrait<T>, std::allocator<T> > str = s;
+    try {
+        str.find(s);
+        assert(false);
+    } catch (int) { /* Do nothing... */ }
+}
+#endif // _LIBCPP_NO_EXCEPTIONS
+
 template <class S>
 void test0()
 {
     test(S(""), "", 0, 0);
     test(S(""), "abcde", 0, S::npos);
     test(S(""), "abcdeabcde", 0, S::npos);
     test(S(""), "abcdeabcdeabcdeabcde", 0, S::npos);
     test(S(""), "", 1, S::npos);

 #if TEST_STD_VER >= 11
     {
     typedef std::basic_string<char, std::char_traits<char>, min_allocator<char>> S;
     test0<S>();
     test1<S>();
     }
 #endif
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    test_trait_find_throws("xyz");
+#endif // _LIBCPP_NO_EXCEPTIONS
+
   return 0;
 }

libcxx/test/std/strings/basic.string/string.ops/string_find/pointer_size_size.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <string>

 test(const S& s, const typename S::value_type* str, typename S::size_type pos,
      typename S::size_type n, typename S::size_type x)
 {
     assert(s.find(str, pos, n) == x);
     if (x != S::npos)
         assert(pos <= x && x + n <= s.size());
 }
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+template<class T>
+struct ThrowingTrait : std::char_traits<T>
+{
+    static const T*
+    find(const T*, std::size_t, const T&) {
+        throw 0;
+    }
+};
+
+template<class T>
+void
+test_trait_find_throws(const T *s) {
+    std::basic_string<T, ThrowingTrait<T>, std::allocator<T> > str = s;
+    try {
+        str.find(s, 0, 2);
+        assert(false);
+    } catch (int) { /* Do nothing... */ }
+}
+#endif // _LIBCPP_NO_EXCEPTIONS
+
 template <class S>
 void test0()
 {
     test(S(""), "", 0, 0, 0);
     test(S(""), "abcde", 0, 0, 0);
     test(S(""), "abcde", 0, 1, S::npos);
     test(S(""), "abcde", 0, 2, S::npos);
     test(S(""), "abcde", 0, 4, S::npos);
#if TEST_STD_VER >= 11
     typedef std::basic_string<char, std::char_traits<char>, min_allocator<char>> S;
     test0<S>();
     test1<S>();
     test2<S>();
     test3<S>();
     }
 #endif
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    test_trait_find_throws("xyz");
+#endif // _LIBCPP_NO_EXCEPTIONS
+
   return 0;
 }

libcxx/test/std/strings/basic.string/string.ops/string_find/string_size.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <string>

 void
 test(const S& s, const S& str, typename S::size_type x)
 {
     assert(s.find(str) == x);
     if (x != S::npos)
         assert(0 <= x && x + str.size() <= s.size());
 }
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+template<class T>
+struct ThrowingTrait : std::char_traits<T>
+{
+    static const T*
+    find(const T*, std::size_t, const T&) {
+        throw 0;
+    }
+};
+
+template<class T>
+void
+test_trait_find_throws(const T *s) {
+    std::basic_string<T, ThrowingTrait<T>, std::allocator<T> > str = s;
+    std::basic_string<T, ThrowingTrait<T>, std::allocator<T> > search_val = "xyz";
+    try {
+        str.find(search_val);
+        assert(false);
+    } catch (int) { /* Do nothing... */ }
+}
+#endif // _LIBCPP_NO_EXCEPTIONS
+
 template <class S>
 void test0()
 {
     test(S(""), S(""), 0, 0);
     test(S(""), S("abcde"), 0, S::npos);
     test(S(""), S("abcdeabcde"), 0, S::npos);
     test(S(""), S("abcdeabcdeabcdeabcde"), 0, S::npos);
     test(S(""), S(""), 1, S::npos);

 
 #if TEST_STD_VER > 3
     {   // LWG 2946
     std::string s = " !";
     assert(s.find({"abc", 1}) == std::string::npos);
     }
 #endif
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    test_trait_find_throws("xyz");
+#endif // _LIBCPP_NO_EXCEPTIONS
+
   return 0;
 }

libcxx/test/std/strings/basic.string/string.ops/string_find/string_view_size.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <string>

 void
 test(const S& s, SV sv, typename S::size_type x)
 {
     assert(s.find(sv) == x);
     if (x != S::npos)
         assert(0 <= x && x + sv.size() <= s.size());
 }
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+template<class T>
+struct ThrowingTrait : std::char_traits<T>
+{
+    static const T*
+    find(const T*, std::size_t, const T&) {
+        throw 0;
+    }
+};
+
+template<class T>
+void
+test_trait_find_throws(const T *s) {
+    std::basic_string<T, ThrowingTrait<T>, std::allocator<T> > str = s;
+    std::basic_string_view<T, ThrowingTrait<T> > view = s;
+    try {
+        str.find(view);
+        assert(false);
+    } catch (int) { /* Do nothing... */ }
+}
+#endif // _LIBCPP_NO_EXCEPTIONS
+
 template <class S, class SV>
 void test0()
 {
     test(S(""), SV(""), 0, 0);
     test(S(""), SV("abcde"), 0, S::npos);
     test(S(""), SV("abcdeabcde"), 0, S::npos);
     test(S(""), SV("abcdeabcdeabcdeabcde"), 0, S::npos);
     test(S(""), SV(""), 1, S::npos);
#if TEST_STD_VER >= 11
     {
     typedef std::basic_string<char, std::char_traits<char>, min_allocator<char>> S;
     typedef std::string_view SV;
     test0<S, SV>();
     test1<S, SV>();
     }
 #endif
 
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    test_trait_find_throws("xyz");
+#endif // _LIBCPP_NO_EXCEPTIONS
+
   return 0;
 }