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

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

> 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. [...]

Agreed.

> 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.

Do you know *why* the tests are failing with libc++? I see this overload for `insert` and it seems like it should be a better match?

  template <class _Pp,
            class = typename enable_if<is_constructible<value_type, _Pp>::value>::type>
      _LIBCPP_INLINE_VISIBILITY
      pair<iterator, bool> insert(_Pp&& __p);

I think we should just fix libc++ to do the right thing (according to the standard).

Separately, I'm not sure it's worth changing the Standard to ensure that the `T const&` overload is selected -- I don't see a huge benefit here.


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

https://reviews.llvm.org/D61364