This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Adds missing forward_list merge tests.
ClosedPublic

Authored by Mordante on Nov 7 2021, 7:06 AM.

Details

Summary

During the review of D112660 it turned out the tests for
std::forward_list::merge are incomplete.

Adds tests for the rvalue reference overloads. The tests are extended to
better test the Effects [forward.list.ops]/25 and Remarks
[forward.list.ops]/27 of the function:

  • x is empty after the merge.
  • Pointers and references to the moved elements of x now refer to those same elements but as members of *this.
  • Iterators referring to the moved elements will continue to refer to their elements, but they now behave as iterators into *this, not into x.
  • The algorithm is stable.

Diff Detail

Event Timeline

Mordante requested review of this revision.Nov 7 2021, 7:06 AM
Mordante created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2021, 7:06 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

LGTM assuming these tests are all basically copied from the std::list tests.

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp
66

I assume this is copied from an existing test, because otherwise typedef T* P; is certainly overkill. ;) No complaints if we're just matching somewhere else, though.

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.pass.cpp
10

Honestly this probably works in c++03 mode, too, doesn't it?

ldionne accepted this revision.Nov 8 2021, 1:24 PM
ldionne added a subscriber: ldionne.

Thanks for increasing our coverage!

This revision is now accepted and ready to land.Nov 8 2021, 1:24 PM
Mordante marked 2 inline comments as done.Nov 9 2021, 11:11 AM
Mordante added inline comments.
libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp
66

It was copied/moved from merge_pred.pass.cpp. I added the P just for consistency, but I agree it's a bit of overkill.

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.pass.cpp
10

No it doesn't forward_list:837

#ifndef _LIBCPP_CXX03_LANG
    _LIBCPP_INLINE_VISIBILITY
    void merge(forward_list&& __x) {merge(__x, __less<value_type>());}
    template <class _Compare>
        _LIBCPP_INLINE_VISIBILITY
        void merge(forward_list&& __x, _Compare __comp)
        {merge(__x, _VSTD::move(__comp));}
#endif // _LIBCPP_CXX03_LANG

But judging by your question and your recent commits I guess I'll test whether this code can be made available in C++03. Obviously that will be in a separate patch.
(At least I will fix the horrible indention.)

This revision was automatically updated to reflect the committed changes.
Mordante marked 2 inline comments as done.
libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp