Add swap method for Cell

[F001]

Addition to #39264

r? @alexcrichton

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[ghost]

The value in other is currently garbage and this will drop it.

You could do mem::forget(other.replace(s)), or this function could just be unsafe { ptr::swap(self.as_ptr(), other.as_ptr()); } (after the pointer equality check of course)

[F001]

Thank you! I'll fix this.

[ghost]

Oh it looks like Cell::as_ptr is still only implemented for T: Copy. Sorry about that.

[alexcrichton]

Thanks for the PR @F001! This seems like reasonable functionality to me. Thoughts @rust-lang/libs?

[BurntSushi]

I think this seems reasonable to me. Should this gets it own feature separate from move_cell though?

[F001]

@whataloadofwhat Thank you for your help! I was very busy yesterday, and submitted the code in a hurry. Even didn't try to compile. That's my fault!

@BurntSushi Is "cell_swap" a reasonable name? Should I also open an issue to track this feature?

[F001]

Off the topic, I'm not sure the methods as_ptr(), get_mut() and default() should still hold the Copy constraint, according to the recent extension of Cell type. @BurntSushi , what do you think of that?

[pitdicker]

I can't see why as_ptr(), get_mut() and default() would require copy, and I also think from() would be okay. But don't take my word for it...

[aturon]

This API seems good. I personally think it can go under the existing feature; it's almost certain to be stabilized together. We can always shard later if need be.

[alexcrichton]

@bors: r+

[bors]

📌 Commit a5e8bbf has been approved by alexcrichton

[eddyb]

@bors rollup

[bors]

⌛ Testing commit a5e8bbf with merge f7c8beb...

[bors]

💔 Test failed - status-travis

[alexcrichton]

@bors: retry * flaky test - #39785

…

On Mon, Feb 13, 2017 at 2:56 AM, bors ***@***.***> wrote: 💔 Test failed - status-travis <https://travis-ci.org/rust-lang/rust/builds/201050096> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#39716 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95Jk8vf1vyvf5UFMkY8YYOiRYPK4Mks5rcBqogaJpZM4L9JuV> .

[RalfJung]

I'm not sure the methods as_ptr(), get_mut() and default() should still hold the Copy constraint, according to the recent extension of Cell type.

I made a PR for this: #39793

[glaebhoerl]

Hey, I was going to write an RFC for this. Apparently it didn't need one?

[aturon]

@glaebhoerl Yeah -- it's hard to draw a firm line here, but once we have a basic API surface (like the updated one for Cell), additions in the same spirit can generally go through a PR. Of course, as with anything else they start unstable and there's the full FCP for stabilization later on.

[RalfJung]

The PR above is is insta-stable, isn't it? It is relaxing trait bounds of methods, AFAIK this can't even be flagged as unstable...

[alexcrichton]

@RalfJung oh in this case the method is tagged #[unstable], so the method should be unstable for awhile

[RalfJung]

One of the three is. The others are not. (I am talking just about my PR here, about what I quoted above -- not about swap, which my PR does not touch.)

[alexcrichton]

Oh sorry, then yeah, those methods (with relaxed bounds) are instantly stable

(the libs team discussed this though and is ok with that)