This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Remove swap for std::span
ClosedPublic

Authored by jdoerrie on Nov 4 2019, 2:08 PM.

Details

Reviewers
mclow.lists
EricWF
ldionne
Group Reviewers
Restricted Project
Commits
rG416b1560c597: [libcxx] Remove swap for std::span
Summary

This change removes both the member function swap and the free function overload of swap for std::span. While swap is a member and overloaded for every other container in the standard library [1], it is neither a member function nor a free function overload for std::span [2]. Thus the corresponding implementation should be removed.

[1] https://eel.is/c++draft/libraryindex#:swap
[2] https://eel.is/c++draft/span.overview

Diff Detail

Event Timeline

jdoerrie created this revision.Nov 4 2019, 2:08 PM

Friendly ping

miscco added a subscriber: miscco.Feb 16 2020, 10:14 AM

That is indeed the correct thing to do +1

@mclow.lists Can you comment on why you included these functions?

This seems right to me.

ldionne requested changes to this revision.Feb 20 2020, 9:11 AM

Requesting changes just so it shows up in my review queue.

This revision now requires changes to proceed.Feb 20 2020, 9:11 AM
ldionne accepted this revision.May 15 2020, 7:19 AM

@jdoerrie @miscco Let's move forward with this.

This revision is now accepted and ready to land.May 15 2020, 7:19 AM

Please make sure this is rebased on master.

Thanks, Louis! I rebased the diff on the latest master. Could you land the patch for me? I believe I lack the required permissions to do this myself.

What "Author Name <email@domain.com>" should I use for the commit?

"Jan Wilken Dörrie <jdoerrie@google.com>" works for me :)

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2020, 12:03 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript