[libcxx-commits] [PATCH] D100311: [libc++] Test nodiscard attributes of node-handle::empty().

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision as: Mordante.
Mordante added a comment.

Since this is mandated I think it makes sense to add the test here. IMO it doesn't belong to the extension tests.



================
Comment at: libcxx/test/std/containers/container.node/node_handle.empty.nodiscard.verify.cpp:25
+void test() {
+  Container c;
+  auto n = c.extract(c.begin());
----------------
Quuxplusone wrote:
> I suggest populating this container, to avoid the appearance of trying to extract from an empty container. (Even if this test //is// compile-only, which I think it is, despite the `main` routine.)
> I suggest:
> ```
> template<class Container>
> void test_one(Container c) {
>     auto n = c.extract(c.begin());
>     n.empty();  // expected-warning 8{{ignoring return value of function}}
> }
> 
> void test() {
>     test_one(std::set<int>{1});
>     test_one(std::multiset<int>{1});
>     test_one(std::map<int, int>{{1, 1}});
>     test_one(std::multimap<int, int>{{1, 1}});
>     test_one(std::unordered_set<int>{1});
>     test_one(std::unordered_multiset<int>{1});
>     test_one(std::unordered_map<int, int>{{1, 1}});
>     test_one(std::unordered_multimap<int, int>{{1, 1}});
> }
> ```
> (and no `main` routine).
> 
> However, see my other comment.
This is indeed a compile-time test. It tests whether the expected compiler diagnostics are issues. Basically this is our typical clang unit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100311