[libc++][test] Add tuple trivial destructor test

[jloser]

Summary

There is only compile-time tests in dtor.pass.cpp, so it could be made a
dtor.compile.pass.cpp. Instead, add a runtime test for testing the trivial
destructor behavior for tuple.

[Mordante]

LGTM!

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

48–56

While you're at it, I would be nice if you can remove the unneeded whitespace in > >.

[jloser]

[NFC] Remove extra whitespace from test

[ldionne]

Instead of moving this to a .compile.pass.cpp, I'd like to add an actual runtime test that the dtor does the right thing. Something like this:

struct TrackDtor {
  bool* dtorCalled_;
  constexpr explicit TrackDtor(bool* dtorCalled) : dtorCalled_(dtorCalled) { }
  TEST_CONSTEXPR_CXX20 ~TrackDtor() { *dtorCalled_ = true; }
};

TEST_CONSTEXPR_CXX20 bool test() {
  bool called = false;
  {
    std::tuple<TrackDtor> tuple(TrackDtor(&called));
    assert(!called);
  }
  assert(called);

  return true;
}

int main(int, char**) {
  test();
#if TEST_STD_VER > 20
  static_assert(test());
#endif
  return 0;
}

[jloser]

Add runtime test

[jloser]

ping @ldionne @Mordante

[jloser]

Mark dtor constexpr in C++20 test

[philnik]

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

34

Can this test be constexpr in C++20?

48–52

I think I'd put the static_asserts outside the main.

[jloser]

Move static_asserts out of main
Mark test() function as constexpr in C++20 mode

[jloser]

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

34

Yep, just made it so.

48–52

Sure, I don't feel strongly. Moved them outside.

[Mordante]

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

40

This differs from @ldionne's suggestion. I wonder whether we should make sure the destruction of the temporary shouldn't increase the counter. Something like:

struct TrackDtor {
   int* count_;
   constexpr explicit TrackDtor(int* count) : count_(count) {}
   constexpr TrackDtor(TrackDtor&& that) : count_(that.count_) { that.count_ = nullptr}
   TEST_CONSTEXPR_CXX20 ~TrackDtor() { if(coount_) ++*count_; }
};

WDYT?

51

Maybe move this to the other static_assert at line 33.

[jloser]

Modify TrackDtor per Mordante's suggestion to make the intent of the test a bit clearer

[jloser]

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

40

I like that idea and find it helps clarify the intent of the test to help future readers.

51

I don't feel strongly; I moved them up above the test() function though and also added a static_assert(test()) while we're at it.

[Mordante]

Thanks for the update! LGTM modulo one nit.

[philnik]

LGTM with tests passing. I think @ldionne's requests have been addressed, so I think you can land this.

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

48–55

[jloser]

Only static_assert(test()) in C++20 or later

[jloser]

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

48–55

I assume you mean > 17 since it's constexpr in C++20. Fixed.

[philnik]

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

48–55

Oops. Yes I meant > 17.

[jloser]

Fix C++11 CI build

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp

 
 #include <tuple>
 #include <string>
 #include <cassert>
 #include <type_traits>
 
 #include "test_macros.h"
 
+struct TrackDtor {
+  int* count_;
+  constexpr explicit TrackDtor(int* count) : count_(count) {}
+  TEST_CONSTEXPR_CXX20 ~TrackDtor() { ++*count_; }
+};
+
+bool test() {
+  int count = 0;
+  {
+    static_assert(!std::is_trivially_destructible<TrackDtor>::value);
+    std::tuple<TrackDtor> tuple{TrackDtor(&count)};
+    assert(count == 1);
+  }
+  assert(count == 2);
+
+  return true;
+}
+
 int main(int, char**)
 {
-  static_assert(std::is_trivially_destructible<
-      std::tuple<> >::value, "");
-  static_assert(std::is_trivially_destructible<
-      std::tuple<void*> >::value, "");
-  static_assert(std::is_trivially_destructible<
-      std::tuple<int, float> >::value, "");
-  static_assert(!std::is_trivially_destructible<
-      std::tuple<std::string> >::value, "");
-  static_assert(!std::is_trivially_destructible<
-      std::tuple<int, std::string> >::value, "");
+  static_assert(std::is_trivially_destructible<std::tuple<>>::value, "");
+  static_assert(std::is_trivially_destructible<std::tuple<void*>>::value, "");
+  static_assert(std::is_trivially_destructible<std::tuple<int, float>>::value, "");
+  static_assert(!std::is_trivially_destructible<std::tuple<std::string>>::value, "");
+  static_assert(!std::is_trivially_destructible<std::tuple<int, std::string>>::value, "");
+
+  test();
 
   return 0;
 }