[libc++] Add tests for std::string default constructor and destructor

[nilayvaish]

Minor comments, otherwise looks good to me.

libcxx/test/std/strings/basic.string/string.cons/default.pass.cpp

40

Why is this commented out? Same question for the other file as well.

libcxx/test/std/strings/basic.string/string.cons/dtor.pass.cpp

33–35

Seems unused.

[philnik]

libcxx/test/std/strings/basic.string/string.cons/default.pass.cpp

40

std::string isn't constexpr yet. This will be uncommented when std::string is made constexpr (D110598 if you want to review it :P).

libcxx/test/std/strings/basic.string/string.cons/dtor.pass.cpp

33–35

AFAICT this is supposed to be that way. The comment above says that it checks that the address of the destructors constructor can be taken.

[var-const]

libcxx/test/std/strings/basic.string/string.cons/default.pass.cpp

13

This comment is no longer entirely true, because now this file also tests that the default constructor works and creates an empty string. I'd suggest moving it closer to the static assertions and rephrasing to something like Test the noexcept specification, which is a conforming extension..

13

If this is an extension, it seems like the test should use LIBCPP_STATIC_ASSERT (unless the other implementations just happen to do the same extension). If this test passes with GCC and MSVC, I wonder if the extension in question is fully tested?

40

Can you please add a TODO to uncomment this line once that patch lands? In general, I think we should always add a TODO or at least a comment when committing commented out code.

libcxx/test/std/strings/basic.string/string.cons/dtor.pass.cpp

40

Nit: can you use LIBCPP_STATIC_ASSERT?

[philnik]

• Address comments

libcxx/test/std/strings/basic.string/string.cons/default.pass.cpp

40

I think a TODO doesn't make a lot of sense here. This commented out line exists in almost every single file in strings/basic.string. It just makes it harder to remove all the residue mechanically.

[ldionne]

LGTM, I think all comments have been addressed but please confirm with Costa before shipping, just to be sure.

libcxx/test/std/strings/basic.string/string.cons/default.pass.cpp

40

I agree with Costa in the general case, however in this specific instance the string tests have this commented out bit consistently, so I think this is fine as-is. Also we know we're going to uncomment it in the very short term.

[var-const]

libcxx/test/std/strings/basic.string/string.cons/default.pass.cpp

13

Very optional nit: two blank lines in a row, I'd remove one.

libcxx/test/std/strings/basic.string/string.cons/default.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>
+
+// basic_string() noexcept(is_nothrow_default_constructible<allocator_type>::value);
+
+// This tests a conforming extension
+
+#include <cassert>
+#include <string>
+
+#include "test_macros.h"
+#include "test_allocator.h"
+
+#if TEST_STD_VER >= 11
+static_assert(std::is_nothrow_default_constructible<std::string>::value, "");
+static_assert(std::is_nothrow_default_constructible<
+                std::basic_string<char, std::char_traits<char>, test_allocator<char>>>::value, "");
+static_assert(!std::is_nothrow_default_constructible<
+                std::basic_string<char, std::char_traits<char>, limited_allocator<char, 10>>>::value, "");
+#endif
+
+bool test() {
+  std::string str;
+  assert(str.empty());
+
+  return true;
+}
+
+int main(int, char**)
+{
+  test();
+#if TEST_STD_VER > 17
+  // static_assert(test());
+#endif
+
+  return 0;
+}

libcxx/test/std/strings/basic.string/string.cons/default_noexcept.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
-
-// <string>
-
-// basic_string()
-//        noexcept(is_nothrow_default_constructible<allocator_type>::value);
-
-// This tests a conforming extension
-
-#include <string>
-#include <cassert>
-
-#include "test_macros.h"
-#include "test_allocator.h"
-
-int main(int, char**)
-{
-    {
-        typedef std::string C;
-        static_assert(std::is_nothrow_default_constructible<C>::value, "");
-    }
-    {
-        typedef std::basic_string<char, std::char_traits<char>, test_allocator<char>> C;
-        static_assert(std::is_nothrow_default_constructible<C>::value, "");
-    }
-    {
-        typedef std::basic_string<char, std::char_traits<char>, limited_allocator<char, 10>> C;
-        static_assert(!std::is_nothrow_default_constructible<C>::value, "");
-    }
-
-  return 0;
-}

libcxx/test/std/strings/basic.string/string.cons/dtor.pass.cpp

struct throwing_alloc
     throwing_alloc(const throwing_alloc&);
     T *allocate(size_t);
     ~throwing_alloc() noexcept(false);
 };
 
 // Test that it's possible to take the address of basic_string's destructors
 // by creating globals which will register their destructors with cxa_atexit.
 std::string s;
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
 std::wstring ws;
 #endif
 
-int main(int, char**)
-{
-    {
-        typedef std::string C;
-        static_assert(std::is_nothrow_destructible<C>::value, "");
-    }
-    {
-        typedef std::basic_string<char, std::char_traits<char>, test_allocator<char>> C;
-        static_assert(std::is_nothrow_destructible<C>::value, "");
-    }
-#if defined(_LIBCPP_VERSION)
-    {
-        typedef std::basic_string<char, std::char_traits<char>, throwing_alloc<char>> C;
-        static_assert(!std::is_nothrow_destructible<C>::value, "");
-    }
-#endif // _LIBCPP_VERSION
+static_assert(std::is_nothrow_destructible<std::string>::value, "");
+static_assert(std::is_nothrow_destructible<
+                std::basic_string<char, std::char_traits<char>, test_allocator<char>>>::value, "");
+#if defined(_LIBCPP_VERSION)
+static_assert(!std::is_nothrow_destructible<
+                std::basic_string<char, std::char_traits<char>, throwing_alloc<char>>>::value, "");
+#endif // _LIBCPP_VERSION
+
+bool test() {
+  test_allocator_statistics alloc_stats;
+    std::basic_string<char, std::char_traits<char>, test_allocator<char>> str2((test_allocator<char>(&alloc_stats)));
+    str2 = "long long string so no SSO";
+    assert(alloc_stats.alloc_count == 1);
+  assert(alloc_stats.alloc_count == 0);
+
+  return true;
+
+int main(int, char**)
+  test();
+#if TEST_STD_VER > 17
+  // static_assert(test());
+#endif
 
   return 0;
 }