[libc++][NFC] Mark LWG3573 as complete

[jloser]

Summary

Mark LWG3573 as complete. It involves a change in wording around when
basic_string_view's constructor for iterator/sentinel can throw. The
current implementation is not marked conditionally noexcept, so there
is nothing to do here. Add a test that binds this behavior to verify the
constructor is not marked noexcept(true) when end - begin throws.

[Mordante]

Thanks for addressing these LWG-issues.

libcxx/test/std/strings/string.view/string.view.cons/from_iterator_sentinel.pass.cpp

54

It's possible to use Clang in a way it won't throw exceptions. This test fails in that context. Please guard all code you added with #ifndef TEST_HAS_NO_EXCEPTIONS.

55

Is there a reason for this return?

59

I'm not fond of constexpr since throwing isn't allowed in a constexpr function.

64

Please test the result of comparing the exception's what() against "LWG 3573".

[• Quuxplusone]

libcxx/test/std/strings/string.view/string.view.cons/from_iterator_sentinel.pass.cpp

54

You should know I'm going to object every time someone adds a global-namespace overloaded operator to a type that already defines its own operators. If throwing_sized_sentinel<I> IS-A sized_sentinel<I>, then its operator- shouldn't be doing anything different from the existing operator- of sized_sentinel<I>.
Also, although this is a technical nitpick, I believe throwing from operator- in this case violates the semantic requirement in https://eel.is/c++draft/iterator.concept.sizedsentinel#2.1 , which defines what operator- needs to return in this case (and implicitly defines that it does return).
Also, std::input_or_output_iterator auto should be I, and just generally this code is way more crufty than it could be.

I think we can get by with

template<class CharT>
struct ThrowingSentinel {
  friend bool operator==(const CharT*, ThrowingSentinel) noexcept;
  friend int operator-(const CharT *, ThrowingSentinel) noexcept;
  friend int operator-(ThrowingSentinel, const CharT *) { throw 42; }
};

template<class CharT>
void test_throwing() {
  auto val = MAKE_STRING_VIEW(CharT, "test");
  try {
    (void)std::basic_string_view<CharT>(val.begin(), ThrowingSentinel());
    assert(false);
  } catch (int i) {
    assert(i == 42);
  }
}
~~~
  test_throwing<char>();
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
  test_throwing<wchar_t>();
#endif
  test_throwing<char8_t>();
  test_throwing<char16_t>();
  test_throwing<char32_t>();

(I marked some undefined functions noexcept merely to try to be evil and trick the library into getting some intermediate noexcept-specification wrong. I don't really care about those noexcepts. Those functions are undefined because they will never be called.)

[jloser]

Address Arthur's feedback

[jloser]

Guard with TEST_HAS_NO_EXCEPTIONS

[jloser]

In D111925#3068330, @Mordante wrote:

Thanks for addressing these LWG-issues.

libcxx/test/std/strings/string.view/string.view.cons/from_iterator_sentinel.pass.cpp

54

Good call, just guarded with TEST_HAS_NO_EXCEPTIONS.

54

Your suggestions are a good refactoring indeed -- just applied them. I did feel a bit bad writing that original operator- for throwing_sized_sentinel, but it did satisfy the requirements for the test. :)

As for the concept for sized_sentinel_for, it does indeed *need* to return std::iter_difference_t<I>. Note that your original writing with a return type of int would hard error in concept checking sized_sentinel_for for example. I'll wave my hands a bit on whether the return *is in fact reachable* (because in this case it's not due to the unconditional throwing, but it still "works").

55

It originally was strictly required to avoid hard erroring during concept checking sized_sentinel_for.

59

Removed.

[Mordante]

Can you try to fix the CI? Also please verify the Cxx2bIssues.csv changes are still there. Maybe you need to rebase due to recent changes I did. Other then that it looks good.

[jloser]

Add bodies to operator== and operator- for ThrowingSizedSentinel to avoid GCC warning about -Wnon-template-friend.

[jloser]

In D111925#3068891, @Mordante wrote:

Can you try to fix the CI? Also please verify the Cxx2bIssues.csv changes are still there. Maybe you need to rebase due to recent changes I did. Other then that it looks good.

Just rebased and resolved the conflicts in Cxx2bIssues.csv. Also worked around the GCC warning causing CI to fail.

@Mordante @Quuxplusone this should be ready now!

[Mordante]

LGTM!

[jloser]

ping @ldionne in case you're interested

[ldionne]

Thanks!

libcxx/docs/Status/Cxx2bIssues.csv

 `3561 <https://wg21.link/LWG3561>`__,"Issue with internal counter in ``discard_block_engine``","October 2021","",""
 `3563 <https://wg21.link/LWG3563>`__,"``keys_view`` example is broken","October 2021","","","|ranges|"
 `3566 <https://wg21.link/LWG3566>`__,"Constraint recursion for ``operator<=>(optional<T>, U)``","October 2021","","","|spaceship|"
 `3567 <https://wg21.link/LWG3567>`__,"Formatting move-only iterators take two","October 2021","","","|format|"
 `3568 <https://wg21.link/LWG3568>`__,"``basic_istream_view`` needs to initialize ``value_``","October 2021","","","|ranges|"
 `3570 <https://wg21.link/LWG3570>`__,"``basic_osyncstream::emit`` should be an unformatted output function","October 2021","",""
 `3571 <https://wg21.link/LWG3571>`__,"``flush_emit`` should set ``badbit`` if the ``emit`` call fails","October 2021","",""
 `3572 <https://wg21.link/LWG3572>`__,"``copyable-box`` should be fully ``constexpr``","October 2021","","","|ranges|"
-`3573 <https://wg21.link/LWG3573>`__,"Missing Throws element for ``basic_string_view(It begin, End end)``","October 2021","",""
+`3573 <https://wg21.link/LWG3573>`__,"Missing Throws element for ``basic_string_view(It begin, End end)``","October 2021","|Complete|","14.0"
 `3574 <https://wg21.link/LWG3574>`__,"``common_iterator`` should be completely ``constexpr``-able","October 2021","","","|ranges|"
 `3580 <https://wg21.link/LWG3580>`__,"``iota_view``'s ``iterator``'s binary ``operator+`` should be improved","October 2021","","","|ranges|"
 `3581 <https://wg21.link/LWG3581>`__,"The range constructor makes ``basic_string_view`` not trivially move constructible","October 2021","","","|ranges|"
 `3585 <https://wg21.link/LWG3585>`__,"``variant`` converting assignment with immovable alternative","October 2021","",""
 `3589 <https://wg21.link/LWG3589>`__,"The ``const`` lvalue reference overload of ``get`` for ``subrange`` does not constrain ``I`` to be ``copyable`` when ``N == 0``","October 2021","","","|ranges|"
 `3590 <https://wg21.link/LWG3590>`__,"``split_view::base() const &`` is overconstrained","October 2021","","","|ranges|"
 `3591 <https://wg21.link/LWG3591>`__,"``lazy_split_view<input_view>::inner-iterator::base() &&`` invalidates outer iterators","October 2021","","","|ranges|"
 `3592 <https://wg21.link/LWG3592>`__,"``lazy_split_view`` needs to check the simpleness of Pattern","October 2021","","","|ranges|"
 `3593 <https://wg21.link/LWG3593>`__,"Several iterators' ``base() const &`` and ``lazy_split_view::outer-iterator::value_type::end()`` missing ``noexcept``","October 2021","","","|ranges|"
 `3595 <https://wg21.link/LWG3595>`__,"Exposition-only classes proxy and postfix-proxy for ``common_iterator`` should be fully ``constexpr``","October 2021","","","|ranges|"
 "","","","",""

libcxx/test/std/strings/string.view/string.view.cons/from_iterator_sentinel.pass.cpp

 
 // <string_view>
 
 //  template <class It, class End>
 //  constexpr basic_string_view(It begin, End end)
 
 #include <string_view>
 #include <cassert>
+#include <iterator>
 #include <ranges>
 
 #include "make_string.h"
 #include "test_iterators.h"
 
 template<class CharT, class Sentinel>
 constexpr void test() {
   auto val = MAKE_STRING_VIEW(CharT, "test");
#endif
   test<char8_t, char8_t*>();
   test<char16_t, char16_t*>();
   test<char32_t, char32_t*>();
   test<char, const char*>();
   test<char, sized_sentinel<const char*>>();
   return true;
 }
 
+#ifndef TEST_HAS_NO_EXCEPTIONS
+template<class CharT>
+struct ThrowingSentinel {
+  friend bool operator==(const CharT*, ThrowingSentinel) noexcept { return true; }
+  friend std::iter_difference_t<const CharT*> operator-(const CharT*, ThrowingSentinel) noexcept { return {}; }
+  friend std::iter_difference_t<const CharT*> operator-(ThrowingSentinel, const CharT*) { throw 42; }
+};
+
+template <class CharT>
+void test_throwing() {
+  auto val = MAKE_STRING_VIEW(CharT, "test");
+  try {
+    (void)std::basic_string_view<CharT>(val.begin(), ThrowingSentinel<CharT>());
+    assert(false);
+  } catch (int i) {
+    assert(i == 42);
+  }
+}
+
+void test_throwing() {
+  test_throwing<char>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  test_throwing<wchar_t>();
+#endif
+  test_throwing<char8_t>();
+  test_throwing<char16_t>();
+  test_throwing<char32_t>();
+}
+#endif
+
 static_assert( std::is_constructible_v<std::string_view, const char*, char*>);
 static_assert( std::is_constructible_v<std::string_view, char*, const char*>);
 static_assert(!std::is_constructible_v<std::string_view, char*, void*>);               // not a sentinel
 static_assert(!std::is_constructible_v<std::string_view, signed char*, signed char*>); // wrong char type
 static_assert(!std::is_constructible_v<std::string_view, random_access_iterator<char*>, random_access_iterator<char*>>); // not contiguous
 static_assert( std::is_constructible_v<std::string_view, contiguous_iterator<char*>, contiguous_iterator<char*>>);
 
 int main(int, char**) {
   test();
   static_assert(test());
 
+#ifndef TEST_HAS_NO_EXCEPTIONS
+  test_throwing();
+#endif
+
   return 0;
 }