[libc++] [c++2b] [P2166] Prohibit string and string_view construction from nullptr.

[curdeius]

Summary

• https://wg21.link/P2166

[• Quuxplusone]

libcxx/test/std/strings/basic.string/string.cons/nullptr.fail.cpp

18 ↗

(On Diff #361680)

(1) I think we should also test at least one of the implicit-conversion syntaxes: std::string s = nullptr; and/or void f(std::string); f(nullptr);
(2) Personally I see no problem with testing explicit construction, implicit conversion, and assignment all in the same test file (but defer to @ldionne if he has an opinion).
(3) The wording of this error message may depend on details of Clang and/or libc++; does that mean this test needs to go in libcxx/test/libcxx/? If so, then it might be better simply to write a compile-only (non-fail) test that just does

static_assert(!std::is_convertible_v<decltype(nullptr), std::string>);
static_assert(!std::is_constructible_v<std::string, decltype(nullptr)>);
static_assert(!std::is_assignable_v<std::string, decltype(nullptr)>);

and leave it at that.

I was going to suggest an overload-resolution test like

void f(std::unique_ptr<int>) { }
void f(const std::string&);
int main() { f(nullptr); }

which I'm sure programmers would like to Just Work... but [p2166] deliberately keeps that as ambiguous. (The user-defined conversion from nullptr to std::string ranks lower in preference than a standard conversion such as nullptr-to-int*, but delete'ing it doesn't make it go away; it just says "The API designer knows what you're trying to do, and you're wrong to try it." So we still have an ambiguity here between the right thing (conversion to unique_ptr) and the wrong thing (conversion to string), with no tiebreaker for picking between them.)

[ldionne]

libcxx/test/std/strings/basic.string/string.cons/nullptr.fail.cpp

18 ↗

(On Diff #361680)

I would go for the static_assert tests in a .compile.pass.cpp test file too. All in the same file IMO.

[ldionne]

Requesting changes so it shows up right in the review queue, but feel free to ping me on Discord for a pretty trivial review once you update this and CI passes.

[curdeius]

• Fix tests.
• Address review comments.

[ldionne]

LGTM but please remove main()s!

libcxx/test/std/strings/string.view/string.view.cons/nullptr.compile.pass.cpp

23

You don't need to define a main() for compile.pass.cpp tests. Not defining a main makes it clear that the test was never intended to be run. Applies above too.

libcxx/docs/Status/Cxx2bPapers.csv

 "`P1132R8 <https://wg21.link/P1132R8>`__","LWG","out_ptr - a scalable output pointer abstraction","June 2021","",""
 "`P1328R1 <https://wg21.link/P1328R1>`__","LWG","Making std::type_info::operator== constexpr","June 2021","",""
 "`P1425R4 <https://wg21.link/P1425R4>`__","LWG","Iterators pair constructors for stack and queue","June 2021","",""
 "`P1518R2 <https://wg21.link/P1518R2>`__","LWG","Stop overconstraining allocators in container deduction guides","June 2021","|Complete|","13.0"
 "`P1659R3 <https://wg21.link/P1659R3>`__","LWG","starts_with and ends_with","June 2021","",""
 "`P1951R1 <https://wg21.link/P1951R1>`__","LWG","Default Arguments for pair Forwarding Constructor","June 2021","",""
 "`P1989R2 <https://wg21.link/P1989R2>`__","LWG","Range constructor for std::string_view","June 2021","",""
 "`P2136R3 <https://wg21.link/P2136R3>`__","LWG","invoke_r","June 2021","",""
-"`P2166R1 <https://wg21.link/P2166R1>`__","LWG","A Proposal to Prohibit std::basic_string and std::basic_string_view construction from nullptr","June 2021","",""
+"`P2166R1 <https://wg21.link/P2166R1>`__","LWG","A Proposal to Prohibit std::basic_string and std::basic_string_view construction from nullptr","June 2021","|Complete|","13.0"
 "","","","","",""
 No newline at end of file

libcxx/include/string

// types:
     basic_string(const basic_string& str, size_type pos, size_type n,
                  const Allocator& a = Allocator());
     template<class T>
         basic_string(const T& t, size_type pos, size_type n, const Allocator& a = Allocator()); // C++17
     template <class T>
         explicit basic_string(const T& t, const Allocator& a = Allocator()); // C++17
     basic_string(const value_type* s, const allocator_type& a = allocator_type());
     basic_string(const value_type* s, size_type n, const allocator_type& a = allocator_type());
+    basic_string(nullptr_t) = delete; // C++2b
     basic_string(size_type n, value_type c, const allocator_type& a = allocator_type());
     template<class InputIterator>
         basic_string(InputIterator begin, InputIterator end,
                      const allocator_type& a = allocator_type());
     basic_string(initializer_list<value_type>, const Allocator& = Allocator());
     basic_string(const basic_string&, const Allocator&);
     basic_string(basic_string&&, const Allocator&);
 
     ~basic_string();
 
     operator basic_string_view<charT, traits>() const noexcept;
 
     basic_string& operator=(const basic_string& str);
     template <class T>
         basic_string& operator=(const T& t); // C++17
     basic_string& operator=(basic_string&& str)
         noexcept(
              allocator_type::propagate_on_container_move_assignment::value ||
              allocator_type::is_always_equal::value ); // C++17
     basic_string& operator=(const value_type* s);
+    basic_string& operator=(nullptr_t) = delete; // C++2b
     basic_string& operator=(value_type c);
     basic_string& operator=(initializer_list<value_type>);
 
     iterator       begin() noexcept;
     const_iterator begin() const noexcept;
     iterator       end() noexcept;
     const_iterator end() const noexcept;
 
#   if _LIBCPP_DEBUG_LEVEL == 2
       __get_db()->__insert_c(this);
 #   endif
     }
 
     template <class = _EnableIf<__is_allocator<_Allocator>::value, nullptr_t> >
         _LIBCPP_INLINE_VISIBILITY
         basic_string(const _CharT* __s, const _Allocator& __a);
 
+#if _LIBCPP_STD_VER > 20
+    basic_string(nullptr_t) = delete;
+#endif
+
     _LIBCPP_INLINE_VISIBILITY
     basic_string(const _CharT* __s, size_type __n);
     _LIBCPP_INLINE_VISIBILITY
     basic_string(const _CharT* __s, size_type __n, const _Allocator& __a);
     _LIBCPP_INLINE_VISIBILITY
     basic_string(size_type __n, _CharT __c);
 
     template <class = _EnableIf<__is_allocator<_Allocator>::value, nullptr_t> >

 #ifndef _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY
     basic_string& operator=(basic_string&& __str)
         _NOEXCEPT_((__noexcept_move_assign_container<_Allocator, __alloc_traits>::value));
      _LIBCPP_INLINE_VISIBILITY
     basic_string& operator=(initializer_list<value_type> __il) {return assign(__il.begin(), __il.size());}
 #endif
     _LIBCPP_INLINE_VISIBILITY basic_string& operator=(const value_type* __s) {return assign(__s);}
+#if _LIBCPP_STD_VER > 20
+    basic_string& operator=(nullptr_t) = delete;
+#endif
     basic_string& operator=(value_type __c);
 
 #if _LIBCPP_DEBUG_LEVEL == 2
     _LIBCPP_INLINE_VISIBILITY
     iterator begin() _NOEXCEPT
         {return iterator(this, __get_pointer());}
     _LIBCPP_INLINE_VISIBILITY
     const_iterator begin() const _NOEXCEPT

libcxx/include/string_view

class basic_string_view {
       static constexpr size_type npos = size_type(-1);
 
       // 7.3, basic_string_view constructors and assignment operators
       constexpr basic_string_view() noexcept;
       constexpr basic_string_view(const basic_string_view&) noexcept = default;
       basic_string_view& operator=(const basic_string_view&) noexcept = default;
       template<class Allocator>
       constexpr basic_string_view(const charT* str);
+      basic_string_view(nullptr_t) = delete; // C++2b
       constexpr basic_string_view(const charT* str, size_type len);
 
       // 7.4, basic_string_view iterator support
       constexpr const_iterator begin() const noexcept;
       constexpr const_iterator end() const noexcept;
       constexpr const_iterator cbegin() const noexcept;
       constexpr const_iterator cend() const noexcept;
       const_reverse_iterator rbegin() const noexcept;
#if _LIBCPP_STD_VER > 11
     _LIBCPP_ASSERT(__len == 0 || __s != nullptr, "string_view::string_view(_CharT *, size_t): received nullptr");
 #endif
     }
 
     _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
     basic_string_view(const _CharT* __s)
         : __data(__s), __size(_VSTD::__char_traits_length_checked<_Traits>(__s)) {}
 
+#if _LIBCPP_STD_VER > 20
+    basic_string_view(nullptr_t) = delete;
+#endif
+
     // [string.view.iterators], iterators
     _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
     const_iterator begin()  const _NOEXCEPT { return cbegin(); }
 
     _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
     const_iterator end()    const _NOEXCEPT { return cend(); }
 
     _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY

libcxx/test/libcxx/debug/db_string_view.pass.cpp

 // test container debugging
 
 #include <string_view>
 
 #include "test_macros.h"
 #include "debug_mode_helper.h"
 
 void test_null_argument() {
-  EXPECT_DEATH(std::string_view(nullptr));
-  EXPECT_DEATH(std::string_view(NULL));
+  // C++2b prohibits construction of string_view from nullptr_t.
+  const char* nullp = nullptr;
+  const char* null = NULL;
+  (void)nullp;
+  (void)null;
+  EXPECT_DEATH((std::string_view(nullp)));
+  EXPECT_DEATH((std::string_view(null)));
   EXPECT_DEATH(std::string_view(static_cast<const char*>(0)));
   {
     std::string_view v;
-    EXPECT_DEATH(((void)(v == nullptr)));
-    EXPECT_DEATH(((void)(nullptr == v)));
+    EXPECT_DEATH(((void)(v == nullp)));
+    EXPECT_DEATH(((void)(nullp == v)));
   }
 }
 
-int main(int, char**)
-{
+int main(int, char**) {
   test_null_argument();
 
   return 0;
 }

libcxx/test/std/strings/basic.string/string.cons/nullptr.compile.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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// <string>
+
+// basic_string(nullptr_t) = delete; // C++2b
+// basic_string& operator=(nullptr_t) = delete; // C++2b
+
+#include <string>
+#include <type_traits>
+
+static_assert(!std::is_convertible_v<decltype(nullptr), std::string>);
+static_assert(!std::is_constructible_v<std::string, decltype(nullptr)>);
+static_assert(!std::is_assignable_v<std::string, decltype(nullptr)>);

libcxx/test/std/strings/string.view/string.view.cons/nullptr.compile.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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// <string_view>
+
+// basic_string_view(nullptr_t) = delete; // C++2b
+
+#include <string_view>
+#include <type_traits>
+
+static_assert(!std::is_convertible_v<decltype(nullptr), std::string_view>);
+static_assert(!std::is_constructible_v<std::string_view, decltype(nullptr)>);
+static_assert(!std::is_assignable_v<std::string_view, decltype(nullptr)>);