[libcxx-commits] [PATCH] D109298: [libc++][test] Add tuple trivial destructor test

[Joe Loser via Phabricator via libcxx-commits]

jloser marked 2 inline comments as done.
jloser added inline comments.


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp:40
+    std::tuple<TrackDtor> tuple{TrackDtor(&count)};
+    assert(count == 1);
+  }
----------------
Mordante wrote:
> 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?
I like that idea and find it helps clarify the intent of the test to help future readers.


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/dtor.pass.cpp:51
+static_assert(!std::is_trivially_destructible<std::tuple<std::string>>::value, "");
+static_assert(!std::is_trivially_destructible<std::tuple<int, std::string>>::value, "");
+
----------------
Mordante wrote:
> Maybe move this to the other `static_assert` at line 33.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109298/new/

https://reviews.llvm.org/D109298