Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Str and Slice swaps #643

Merged
merged 1 commit into from
Jan 2, 2021
Merged

Remove Str and Slice swaps #643

merged 1 commit into from
Jan 2, 2021

Conversation

dtolnay
Copy link
Owner

@dtolnay dtolnay commented Jan 2, 2021

Both of these types are "is_trivially_move_constructible" and "is_trivially_move_assignable", so the default std::swap would already correctly do exactly the same thing that these are doing.

We keep the custom swap on Box, String, Vec which are not trivially movable, their moves do stuff to the source object to move around ownership.

FYI @capickett

@dtolnay dtolnay merged commit 4f2b30e into master Jan 2, 2021
@dtolnay dtolnay deleted the swap branch January 2, 2021 20:44
@capickett
Copy link
Contributor

A small note here is that this changes means that

rust::Str s1 = "abc";
rust::Str s2 = "def";
s1.swap(s2);

Will now no longer compile. Referencing the STL, it appears std::string_view exposes a member swap https://en.cppreference.com/w/cpp/string/basic_string_view/swap. Perhaps worth keeping the member and implementing as:

void Str::swap(Str &rhs) noexcept {
  std::swap(*this, rhs);
}

@dtolnay
Copy link
Owner Author

dtolnay commented Jan 2, 2021

Thanks! Good catch, I was only paying attention to the "std::swap"-style usage. Thanks for the link, it seems clear that we should have the member functions to match string_view. I've put those back in #646.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants