Skip to content

Commit

Permalink
auto merge of #10514 : sfackler/rust/mut, r=cmr
Browse files Browse the repository at this point in the history
This is based off of @blake2-ppc's work on #9429. That PR bitrotted and I haven't been able to contact the original author so I decided to take up the cause.

Overview
======
`Mut` encapsulates a mutable, non-nullable slot. The `Cell` type is currently used to do this, but `Cell` is much more commonly used as a workaround for the inability to move values into non-once functions. `Mut` provides a more robust API.

`Mut` duplicates the semantics of borrowed pointers with enforcement at runtime instead of compile time.
```rust
let x = Mut::new(0);

{
    // make some immutable borrows
    let p = x.borrow();
    let y = *p.get() + 10;

    // multiple immutable borrows are allowed simultaneously
    let p2 = x.borrow();

    // this would throw a runtime failure
    // let p_mut = x.borrow_mut();
}

// now we can mutably borrow
let p = x.borrow_mut();
*p.get() = 10;
```
`borrow` returns a `Ref` type and `borrow_mut` returns a `RefMut` type, both of which are simple smart pointer types with a single method, `get`, which returns a reference to the wrapped data.

This also allows `RcMut<T>` to be deleted, as it can be replaced with `Rc<Mut<T>>`.

Changes
======
I've done things a little bit differently than the original proposal.

* I've added `try_borrow` and `try_borrow_mut` methods that return `Option<Ref<T>>` and `Option<RefMut<T>>` respectively instead of failing on a borrow check failure. I'm not totally sure when that'd be useful, but I don't see any reason to not put them in and @cmr requested them.
* `ReadPtr` and `WritePtr` have been renamed to `Ref` and `RefMut` respectively, as `Ref` is to `ref foo` and `RefMut` is to `ref mut foo` as `Mut` is to `mut foo`.
* `get` on `MutRef` now takes `&self` instead of `&mut self` for consistency with `&mut`. As @alexcrichton pointed, out this violates soundness by allowing aliasing `&mut` references.
* `Cell` is being left as is. It solves a different problem than `Mut` is designed to solve.
* There are no longer methods implemented for `Mut<Option<T>>`. Since `Cell` isn't going away, there's less of a need for these, and I didn't feel like they provided a huge benefit, especially as that kind of `impl` is very uncommon in the standard library.

Open Questions
============
* `Cell` should now be used exclusively for movement into closures. Should this be enforced by reducing its API to `new` and `take`? It seems like this use case will be completely going away once the transition to `proc` and co. finishes.
* Should there be `try_map` and `try_map_mut` methods along with `map` and `map_mut`?
  • Loading branch information
bors committed Nov 24, 2013
2 parents 6cbc57c + bdfaf04 commit 33375a3
Show file tree
Hide file tree
Showing 12 changed files with 437 additions and 360 deletions.
28 changes: 13 additions & 15 deletions src/librustc/middle/typeck/infer/region_inference/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use middle::graph::{Direction, NodeIndex};
use util::common::indenter;
use util::ppaux::{Repr};

use std::cell::Cell;
use std::hashmap::{HashMap, HashSet};
use std::uint;
use std::vec;
Expand Down Expand Up @@ -106,16 +105,15 @@ pub struct RegionVarBindings {
undo_log: ~[UndoLogEntry],

// This contains the results of inference. It begins as an empty
// cell and only acquires a value after inference is complete.
// We use a cell vs a mutable option to circumvent borrowck errors.
values: Cell<~[VarValue]>,
// option and only acquires a value after inference is complete.
values: Option<~[VarValue]>,
}

pub fn RegionVarBindings(tcx: ty::ctxt) -> RegionVarBindings {
RegionVarBindings {
tcx: tcx,
var_origins: ~[],
values: Cell::new_empty(),
values: None,
constraints: HashMap::new(),
lubs: HashMap::new(),
glbs: HashMap::new(),
Expand Down Expand Up @@ -226,7 +224,7 @@ impl RegionVarBindings {
constraint: Constraint,
origin: SubregionOrigin) {
// cannot add constraints once regions are resolved
assert!(self.values.is_empty());
assert!(self.values.is_none());

debug!("RegionVarBindings: add_constraint({:?})", constraint);

Expand All @@ -242,7 +240,7 @@ impl RegionVarBindings {
sub: Region,
sup: Region) {
// cannot add constraints once regions are resolved
assert!(self.values.is_empty());
assert!(self.values.is_none());

debug!("RegionVarBindings: make_subregion({:?}, {:?})", sub, sup);
match (sub, sup) {
Expand Down Expand Up @@ -277,7 +275,7 @@ impl RegionVarBindings {
b: Region)
-> Region {
// cannot add constraints once regions are resolved
assert!(self.values.is_empty());
assert!(self.values.is_none());

debug!("RegionVarBindings: lub_regions({:?}, {:?})", a, b);
match (a, b) {
Expand All @@ -300,7 +298,7 @@ impl RegionVarBindings {
b: Region)
-> Region {
// cannot add constraints once regions are resolved
assert!(self.values.is_empty());
assert!(self.values.is_none());

debug!("RegionVarBindings: glb_regions({:?}, {:?})", a, b);
match (a, b) {
Expand All @@ -319,14 +317,14 @@ impl RegionVarBindings {
}

pub fn resolve_var(&mut self, rid: RegionVid) -> ty::Region {
if self.values.is_empty() {
self.tcx.sess.span_bug(
let v = match self.values {
None => self.tcx.sess.span_bug(
self.var_origins[rid.to_uint()].span(),
format!("Attempt to resolve region variable before values have \
been computed!"));
}
been computed!")),
Some(ref values) => values[rid.to_uint()]
};

let v = self.values.with_ref(|values| values[rid.to_uint()]);
debug!("RegionVarBindings: resolve_var({:?}={})={:?}",
rid, rid.to_uint(), v);
match v {
Expand Down Expand Up @@ -482,7 +480,7 @@ impl RegionVarBindings {
debug!("RegionVarBindings: resolve_regions()");
let mut errors = opt_vec::Empty;
let v = self.infer_variable_values(&mut errors);
self.values.put_back(v);
self.values = Some(v);
errors
}
}
Expand Down
Loading

0 comments on commit 33375a3

Please sign in to comment.