[libcxx-commits] [PATCH] D57903: [libcxx] Add missing checks to tests for the move w/allocator constructors of associative containers.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: test/std/containers/associative/map/map.cons/move_alloc.pass.cpp:181
             LIBCPP_ASSERT(m1.empty());
-            assert(Counter_base::gConstructed == num+6);
+            assert(Counter_base::gConstructed == num+6+(m1.empty()?0:3));
 
----------------
amakc11 wrote:
> ldionne wrote:
> > Technically, I think the container could also have just 2 elements remaining, no? IOW, what you mean is really that `num+6 <= Counter_base::gConstructed <= num+6+3`?
> This assumption looks very strange to me. The `std::move()` either deletes everything in the source container or preserves everything. Any intermediate solution when this function deletes any random(??) number of elements looks unreasonable. However, if you have any specific reasons for implementing (and testing) such intermediate solution, please, provide them. Please, also consult Marshall who is the author of these tests.
Well.. the standard says this (http://eel.is/c++draft/lib.types.movedfrom):

> Objects of types defined in the C++ standard library may be moved from ([class.copy.ctor]). Move operations may be explicitly specified or implicitly generated. Unless otherwise specified, such moved-from objects shall be placed in a valid but unspecified state.

I couldn't find something that said that `std::map` behaved differently, therefore as far as the Standard is concerned, a moved-from map must be left in a valid-but-unspecified state. @mclow.lists can double-check that I'm not mistaken here.

If I'm right, then leaving the moved-from map completely full, leaving it empty, or leaving any number of elements in it are all valid implementations. Therefore your patch is not really improving the test suite, it's just making the specific implementation you're testing pass. Instead, I think `Counter_base::gConstructed <= num+6+m1.size()` would be better.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D57903