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

Rewrite the coercion code to be more readable, more sound, and to reborrow when needed. #4591

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/etc/tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ def report_error_name_no(name, no, s):
def report_err(s):
report_error_name_no(fileinput.filename(), fileinput.filelineno(), s)

def report_warn(s):
print("%s:%d: %s" % (fileinput.filename(),
fileinput.filelineno(),
s))

def do_license_check(name, contents):
if not check_license(name, contents):
report_error_name_no(name, 1, "incorrect license")
Expand All @@ -44,6 +49,9 @@ def do_license_check(name, contents):
report_err("FIXME without issue number")
if line.find("TODO") != -1:
report_err("TODO is deprecated; use FIXME")
idx = line.find("// WARN")
if idx != -1:
report_warn("WARN:" + line[idx + len("// WARN"):])
if (line.find('\t') != -1 and
fileinput.filename().find("Makefile") == -1):
report_err("tab character")
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/metadata/tydecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,6 @@ fn parse_ty(st: @pstate, conv: conv_did) -> ty::t {
'f' => {
parse_ty_rust_fn(st, conv)
}
'X' => {
return ty::mk_var(st.tcx, ty::TyVid(parse_int(st) as uint));
}
'Y' => return ty::mk_type(st.tcx),
'C' => {
let proto = parse_proto(st);
Expand Down
15 changes: 2 additions & 13 deletions src/librustc/metadata/tyencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,19 +298,8 @@ fn enc_sty(w: io::Writer, cx: @ctxt, +st: ty::sty) {
ty::ty_fn(ref f) => {
enc_ty_fn(w, cx, (*f));
}
ty::ty_infer(ty::TyVar(id)) => {
w.write_char('X');
w.write_uint(id.to_uint());
}
ty::ty_infer(ty::IntVar(id)) => {
w.write_char('X');
w.write_char('I');
w.write_uint(id.to_uint());
}
ty::ty_infer(ty::FloatVar(id)) => {
w.write_char('X');
w.write_char('F');
w.write_uint(id.to_uint());
ty::ty_infer(_) => {
cx.diag.handler().bug(~"Cannot encode inference variable types");
}
ty::ty_param({idx: id, def_id: did}) => {
w.write_char('p');
Expand Down
42 changes: 42 additions & 0 deletions src/librustc/middle/borrowck/gather_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,11 +591,53 @@ impl gather_loan_ctxt {
}
}

ast::pat_vec(_, Some(tail_pat)) => {
// The `tail_pat` here creates a slice into the
// original vector. This is effectively a borrow of
// the elements of the vector being matched.

let tail_ty = self.tcx().ty(tail_pat);
let (tail_mutbl, tail_r) =
self.vec_slice_info(tail_pat, tail_ty);
let mcx = self.bccx.mc_ctxt();
let cmt_index = mcx.cat_index(tail_pat, cmt);
self.guarantee_valid(cmt_index, tail_mutbl, tail_r);
}

_ => {}
}
}
}

fn vec_slice_info(&self,
pat: @ast::pat,
tail_ty: ty::t) -> (ast::mutability, ty::Region)
{
/*!
*
* In a pattern like [a, b, ..c], normally `c` has slice type,
* but if you have [a, b, ..ref c], then the type of `ref c`
* will be `&&[]`, so to extract the slice details we have
* to recurse through rptrs.
*/

match ty::get(tail_ty).sty {
ty::ty_evec(tail_mt, ty::vstore_slice(tail_r)) => {
(tail_mt.mutbl, tail_r)
}

ty::ty_rptr(_, ref mt) => {
self.vec_slice_info(pat, mt.ty)
}

_ => {
self.tcx().sess.span_bug(
pat.span,
fmt!("Type of tail pattern is not a slice"));
}
}
}

fn pat_is_variant_or_struct(&self, pat: @ast::pat) -> bool {
pat_util::pat_is_variant_or_struct(self.bccx.tcx.def_map, pat)
}
Expand Down
184 changes: 128 additions & 56 deletions src/librustc/middle/borrowck/loan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,35 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

/*!

The `Loan` module deals with borrows of *uniquely mutable* data. We
say that data is uniquely mutable if the current activation (stack
frame) controls the only mutable reference to the data. The most
common way that this can occur is if the current activation owns the
data being borrowed, but it can also occur with `&mut` pointers. The
primary characteristic of uniquely mutable data is that, at any given
time, there is at most one path that can be used to mutate it, and
that path is only accessible from the top stack frame.

Given that some data found at a path P is being borrowed to a borrowed
pointer with mutability M and lifetime L, the job of the code in this
module is to compute the set of *loans* that are necessary to ensure
that (1) the data found at P outlives L and that (2) if M is mutable
then the path P will not be modified directly or indirectly except
through that pointer. A *loan* is the combination of a path P_L, a
mutability M_L, and a lifetime L_L where:

- The path P_L indicates what data has been lent.
- The mutability M_L indicates the access rights on the data:
- const: the data cannot be moved
- immutable/mutable: the data cannot be moved or mutated
- The lifetime L_L indicates the *scope* of the loan.

XXX --- much more needed, don't have time to write this all up now

*/

// ----------------------------------------------------------------------
// Loan(Ex, M, S) = Ls holds if ToAddr(Ex) will remain valid for the entirety
// of the scope S, presuming that the returned set of loans `Ls` are honored.
Expand Down Expand Up @@ -39,7 +68,7 @@ impl borrowck_ctxt {
scope_region: scope_region,
loans: ~[]
};
match lc.loan(cmt, mutbl) {
match lc.loan(cmt, mutbl, true) {
Err(ref e) => Err((*e)),
Ok(()) => {
let LoanContext {loans, _} = move lc;
Expand All @@ -62,46 +91,25 @@ struct LoanContext {
impl LoanContext {
fn tcx(&self) -> ty::ctxt { self.bccx.tcx }

fn issue_loan(&self,
cmt: cmt,
scope_ub: ty::Region,
req_mutbl: ast::mutability) -> bckres<()> {
if self.bccx.is_subregion_of(self.scope_region, scope_ub) {
match req_mutbl {
m_mutbl => {
// We do not allow non-mutable data to be loaned
// out as mutable under any circumstances.
if cmt.mutbl != m_mutbl {
return Err({cmt:cmt,
code:err_mutbl(req_mutbl)});
}
}
m_const | m_imm => {
// However, mutable data can be loaned out as
// immutable (and any data as const). The
// `check_loans` pass will then guarantee that no
// writes occur for the duration of the loan.
}
}
fn loan(&self,
cmt: cmt,
req_mutbl: ast::mutability,
owns_lent_data: bool) -> bckres<()>
{
/*!
*
* The main routine.
*
* # Parameters
*
* - `cmt`: the categorization of the data being borrowed
* - `req_mutbl`: the mutability of the borrowed pointer
* that was created
* - `owns_lent_data`: indicates whether `cmt` owns the
* data that is being lent. See
* discussion in `issue_loan()`.
*/

self.loans.push(Loan {
// Note: cmt.lp must be Some(_) because otherwise this
// loan process does not apply at all.
lp: cmt.lp.get(),
cmt: cmt,
mutbl: req_mutbl
});
return Ok(());
} else {
// The loan being requested lives longer than the data
// being loaned out!
return Err({cmt:cmt,
code:err_out_of_scope(scope_ub,
self.scope_region)});
}
}

fn loan(&self, cmt: cmt, req_mutbl: ast::mutability) -> bckres<()> {
debug!("loan(%s, %s)",
self.bccx.cmt_to_repr(cmt),
self.bccx.mut_to_str(req_mutbl));
Expand All @@ -123,13 +131,14 @@ impl LoanContext {
}
cat_local(local_id) | cat_arg(local_id) | cat_self(local_id) => {
let local_scope_id = self.tcx().region_map.get(local_id);
self.issue_loan(cmt, ty::re_scope(local_scope_id), req_mutbl)
self.issue_loan(cmt, ty::re_scope(local_scope_id), req_mutbl,
owns_lent_data)
}
cat_stack_upvar(cmt) => {
self.loan(cmt, req_mutbl) // NDM correct?
self.loan(cmt, req_mutbl, owns_lent_data)
}
cat_discr(base, _) => {
self.loan(base, req_mutbl)
self.loan(base, req_mutbl, owns_lent_data)
}
cat_comp(cmt_base, comp_field(_, m)) |
cat_comp(cmt_base, comp_index(_, m)) => {
Expand All @@ -139,36 +148,41 @@ impl LoanContext {
// that case, it must also be embedded in an immutable
// location, or else the whole structure could be
// overwritten and the component along with it.
self.loan_stable_comp(cmt, cmt_base, req_mutbl, m)
self.loan_stable_comp(cmt, cmt_base, req_mutbl, m,
owns_lent_data)
}
cat_comp(cmt_base, comp_tuple) |
cat_comp(cmt_base, comp_anon_field) => {
// As above.
self.loan_stable_comp(cmt, cmt_base, req_mutbl, m_imm)
self.loan_stable_comp(cmt, cmt_base, req_mutbl, m_imm,
owns_lent_data)
}
cat_comp(cmt_base, comp_variant(enum_did)) => {
// For enums, the memory is unstable if there are multiple
// variants, because if the enum value is overwritten then
// the memory changes type.
if ty::enum_is_univariant(self.bccx.tcx, enum_did) {
self.loan_stable_comp(cmt, cmt_base, req_mutbl, m_imm)
self.loan_stable_comp(cmt, cmt_base, req_mutbl, m_imm,
owns_lent_data)
} else {
self.loan_unstable_deref(cmt, cmt_base, req_mutbl)
self.loan_unstable_deref(cmt, cmt_base, req_mutbl,
owns_lent_data)
}
}
cat_deref(cmt_base, _, uniq_ptr) => {
// For unique pointers, the memory being pointed out is
// unstable because if the unique pointer is overwritten
// then the memory is freed.
self.loan_unstable_deref(cmt, cmt_base, req_mutbl)
self.loan_unstable_deref(cmt, cmt_base, req_mutbl,
owns_lent_data)
}
cat_deref(cmt_base, _, region_ptr(ast::m_mutbl, region)) => {
// Mutable data can be loaned out as immutable or const. We must
// loan out the base as well as the main memory. For example,
// if someone borrows `*b`, we want to borrow `b` as immutable
// as well.
do self.loan(cmt_base, m_imm).chain |_| {
self.issue_loan(cmt, region, m_const)
do self.loan(cmt_base, m_imm, false).chain |_| {
self.issue_loan(cmt, region, m_const, owns_lent_data)
}
}
cat_deref(_, _, unsafe_ptr) |
Expand All @@ -189,7 +203,8 @@ impl LoanContext {
cmt: cmt,
cmt_base: cmt,
req_mutbl: ast::mutability,
comp_mutbl: ast::mutability) -> bckres<()> {
comp_mutbl: ast::mutability,
owns_lent_data: bool) -> bckres<()> {
// Determine the mutability that the base component must have,
// given the required mutability of the pointer (`req_mutbl`)
// and the declared mutability of the component (`comp_mutbl`).
Expand Down Expand Up @@ -243,10 +258,11 @@ impl LoanContext {
(m_const, _) => m_const // (5)
};

do self.loan(cmt_base, base_mutbl).chain |_ok| {
do self.loan(cmt_base, base_mutbl, owns_lent_data).chain |_ok| {
// can use static for the scope because the base
// determines the lifetime, ultimately
self.issue_loan(cmt, ty::re_static, req_mutbl)
self.issue_loan(cmt, ty::re_static, req_mutbl,
owns_lent_data)
}
}

Expand All @@ -256,13 +272,69 @@ impl LoanContext {
fn loan_unstable_deref(&self,
cmt: cmt,
cmt_base: cmt,
req_mutbl: ast::mutability) -> bckres<()> {
req_mutbl: ast::mutability,
owns_lent_data: bool) -> bckres<()>
{
// Variant components: the base must be immutable, because
// if it is overwritten, the types of the embedded data
// could change.
do self.loan(cmt_base, m_imm).chain |_| {
do self.loan(cmt_base, m_imm, owns_lent_data).chain |_| {
// can use static, as in loan_stable_comp()
self.issue_loan(cmt, ty::re_static, req_mutbl)
self.issue_loan(cmt, ty::re_static, req_mutbl,
owns_lent_data)
}
}

fn issue_loan(&self,
cmt: cmt,
scope_ub: ty::Region,
req_mutbl: ast::mutability,
owns_lent_data: bool) -> bckres<()>
{
// Subtle: the `scope_ub` is the maximal lifetime of `cmt`.
// Therefore, if `cmt` owns the data being lent, then the
// scope of the loan must be less than `scope_ub`, or else the
// data would be freed while the loan is active.
//
// However, if `cmt` does *not* own the data being lent, then
// it is ok if `cmt` goes out of scope during the loan. This
// can occur when you have an `&mut` parameter that is being
// reborrowed.

if !owns_lent_data ||
self.bccx.is_subregion_of(self.scope_region, scope_ub)
{
match req_mutbl {
m_mutbl => {
// We do not allow non-mutable data to be loaned
// out as mutable under any circumstances.
if cmt.mutbl != m_mutbl {
return Err({cmt:cmt,
code:err_mutbl(req_mutbl)});
}
}
m_const | m_imm => {
// However, mutable data can be loaned out as
// immutable (and any data as const). The
// `check_loans` pass will then guarantee that no
// writes occur for the duration of the loan.
}
}

self.loans.push(Loan {
// Note: cmt.lp must be Some(_) because otherwise this
// loan process does not apply at all.
lp: cmt.lp.get(),
cmt: cmt,
mutbl: req_mutbl
});
return Ok(());
} else {
// The loan being requested lives longer than the data
// being loaned out!
return Err({cmt:cmt,
code:err_out_of_scope(scope_ub,
self.scope_region)});
}
}
}
8 changes: 6 additions & 2 deletions src/librustc/middle/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,13 @@ impl borrowck_ctxt {
return @{cat:cat_discr(cmt, match_id),.. *cmt};
}

fn mc_ctxt() -> mem_categorization_ctxt {
mem_categorization_ctxt {tcx: self.tcx,
method_map: self.method_map}
}

fn cat_pattern(cmt: cmt, pat: @ast::pat, op: fn(cmt, @ast::pat)) {
let mc = &mem_categorization_ctxt {tcx: self.tcx,
method_map: self.method_map};
let mc = self.mc_ctxt();
mc.cat_pattern(cmt, pat, op);
}

Expand Down
Loading