[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists, ldionne.
Herald added a subscriber: dexonsmith.

In $:\Dev\msvc\src\qa\VC\Libs\libcxx\upstream\test\std\containers\map_allocator_requirement_test_templates.h and $\test\std\containers\set_allocator_requirement_test_templates.h, libcxx tests are asserting that the container's insert operation passes an incorrect type down to allocator::construct. Specifically, given something like:

  {
    CHECKPOINT("Testing C::insert(value_type&)");
    Container c;
    ValueTp v(42, 1);

- cc->expect<const ValueTp&>();** assert(c.insert(v).second); assert(!cc->unchecked()); { DisableAllocationGuard g; ValueTp v2(42, 1); assert(c.insert(v2).second == false); } }

This call to ::insert ends up calling the insert(P&&), not insert(const value_type&), because P is deduced to value_type&, meaning the insert(P&&) overload is an exact match, while insert(const value_type&) requires adding const. See http://eel.is/c++draft/associative#map.modifiers

When the insert(P&&) is called, it delegates to emplace, which only gets Cpp17EmplaceConstructible from the supplied parameters, not Cpp17EmplaceConstructible from add_const_t of the parameters. Thus, libcxx's test is asserting a condition the standard explicitly does not allow at this time. (Though it may be reasonable to fix the standard to make what libcxx is doing okay....)

Unfortunately with the tests fixed to assert the correct allocator::construct call, libc++ will fail these tests. I'm looking for guidance on how libc++ maintainers would like to handle this.


https://reviews.llvm.org/D61364

Files:
  test/std/containers/map_allocator_requirement_test_templates.h
  test/std/containers/set_allocator_requirement_test_templates.h


Index: test/std/containers/set_allocator_requirement_test_templates.h
===================================================================
--- test/std/containers/set_allocator_requirement_test_templates.h
+++ test/std/containers/set_allocator_requirement_test_templates.h
@@ -128,7 +128,7 @@
     CHECKPOINT("Testing C::insert(Iter, Iter) for *Iter = value_type&");
     Container c;
     ValueTp ValueList[] = { ValueTp(1), ValueTp(2) , ValueTp(3) };
-    cc->expect<ValueTp const&>(3);
+    cc->expect<ValueTp&>(3);
     c.insert(std::begin(ValueList), std::end(ValueList));
     assert(!cc->unchecked());
     {
Index: test/std/containers/map_allocator_requirement_test_templates.h
===================================================================
--- test/std/containers/map_allocator_requirement_test_templates.h
+++ test/std/containers/map_allocator_requirement_test_templates.h
@@ -51,7 +51,7 @@
     CHECKPOINT("Testing C::insert(value_type&)");
     Container c;
     ValueTp v(42, 1);
-    cc->expect<const ValueTp&>();
+    cc->expect<ValueTp&>();
     assert(c.insert(v).second);
     assert(!cc->unchecked());
     {
@@ -77,7 +77,7 @@
     CHECKPOINT("Testing C::insert(const value_type&&)");
     Container c;
     const ValueTp v(42, 1);
-    cc->expect<const ValueTp&>();
+    cc->expect<const ValueTp&&>();
     assert(c.insert(std::move(v)).second);
     assert(!cc->unchecked());
     {
@@ -141,7 +141,7 @@
     CHECKPOINT("Testing C::insert(Iter, Iter) for *Iter = value_type&");
     Container c;
     ValueTp ValueList[] = { ValueTp(1, 1), ValueTp(2, 1) , ValueTp(3, 1) };
-    cc->expect<ValueTp const&>(3);
+    cc->expect<ValueTp&>(3);
     c.insert(std::begin(ValueList), std::end(ValueList));
     assert(!cc->unchecked());
     {
@@ -184,7 +184,7 @@
     CHECKPOINT("Testing C::insert(p, value_type&)");
     Container c;
     ValueTp v(42, 1);
-    cc->expect<ValueTp const&>();
+    cc->expect<ValueTp&>();
     It ret = c.insert(c.end(), v);
     assert(ret != c.end());
     assert(c.size() == 1);
@@ -233,7 +233,7 @@
     CHECKPOINT("Testing C::insert(p, const value_type&&)");
     Container c;
     const ValueTp v(42, 1);
-    cc->expect<const ValueTp&>();
+    cc->expect<const ValueTp&&>();
     It ret = c.insert(c.end(), std::move(v));
     assert(ret != c.end());
     assert(c.size() == 1);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61364.197508.patch
Type: text/x-patch
Size: 2335 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190501/c3dd6dfd/attachment-0001.bin>