rust-lang / rust-lang/rust

We should decide whether take_mut is sound

Open
#161,168 15 comments 6 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

C-discussion I-lang-nominated I-lang-radar P-lang-drag-2 T-lang
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

The take_mut and replace_with crates have been around since forever and, despite being unmaintained, has tons of downloads. I am sure variants of this pattern have been copied into many other places; at least one such copy exists in the standard library. However, take_mut is in a soundness conflict with other patterns, such as partial_borrow and the WontDrop from this Zulip comment. Both of those rely on mutable references being "pinned" in some sense, while wanting to avoid the overhead of Pin to make that pinning explicit in the type system.

This means the soundness status of take_mut is undecided. (Note that even if take_mut is unsound, some instances of the pattern may be sound, so e.g. the use in the standard library may still be fine. It just means the safety argument for why that code is fine needs to be more complicated than it currently is.) I think it is harmful to leave the ecosystem in limbo about whether this pattern is sound. We should officially decide one way or the other, by blessing one of the APIs as sound. The most direct way of doing that would be having something in the standard library, but we could also consider having a collection of "operations we consider sound but not common enough to have in the standard library" somewhere. Not sure where.

My personal preference would be to say that take_mut/replace_with are sound. I consider this "natural" since in a formal model of Rust (such as RustBelt), if you don't do anything special for mutable references, this is what will happen. It falls out of the fact that all types are movable. We have pinning any maybe eventually !Move types to properly indicate when something should not be movable.

This has previously been suggested in https://github.com/rust-lang/rfcs/pull/1736 and was closed (10 years ago) because it was considered a footgun to have such implicit aborts in the standard library. We seem more ready to accept implicit aborts these days, such as around extern "C" functions. I think not resolving this soundness conflict is a much worse footgun.

I am not sure which team would be in charge of such a decision, but I suspect both lang and libs since this is about a decision on the safety contract of a built-in type (lang) that is best exposed via a std API (libs).
Cc @rust-lang/lang @rust-lang/libs @rust-lang/libs-api

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the soundness discussion in the linked unsafe-code-guidelines issue and the existing use in library/alloc/src/collections/btree/mem.rs. Compare the take_mut, replace_with, partial_borrow, and WontDrop arguments, then review the context from RFC 1736 and the linked team discussions. Done means the relevant teams reach and document an official soundness decision and an appropriate place or API for communicating it.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.