Skip to content

librustc: De-mode pattern bindings #4121

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

Closed
wants to merge 1 commit 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
6 changes: 3 additions & 3 deletions src/librustc/driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,12 @@ fn compile_upto(sess: Session, cfg: ast::crate_cfg,
time(time_passes, ~"loop checking", ||
middle::check_loop::check_crate(ty_cx, crate));

time(time_passes, ~"alt checking", ||
middle::check_alt::check_crate(ty_cx, method_map, crate));

time(time_passes, ~"mode computation", ||
middle::mode::compute_modes(ty_cx, method_map, crate));

time(time_passes, ~"alt checking", ||
middle::check_alt::check_crate(ty_cx, method_map, crate));

let last_use_map =
time(time_passes, ~"liveness checking", ||
middle::liveness::check_crate(ty_cx, method_map, crate));
Expand Down
17 changes: 3 additions & 14 deletions src/librustc/middle/borrowck/gather_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,20 +529,9 @@ impl gather_loan_ctxt {
self.guarantee_valid(cmt, mutbl, scope_r);
}
}
ast::bind_by_implicit_ref => {
// Note: there is a discussion of the function of
// cat_discr in the method preserve():
let cmt1 = self.bccx.cat_discr(cmt, alt_id);
let arm_scope = ty::re_scope(arm_id);

// We used to remember the mutability of the location
// that this binding refers to and use it later when
// categorizing the binding. This hack is being
// removed in favor of ref mode bindings.
//
// self.bccx.binding_map.insert(pat.id, cmt1.mutbl);

self.guarantee_valid(cmt1, m_const, arm_scope);
ast::bind_infer => {
// Nothing to do here; this is either a copy or a move;
// thus either way there is nothing to check. Yay!
}
}
}
Expand Down
16 changes: 13 additions & 3 deletions src/librustc/middle/check_alt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,15 +592,25 @@ fn check_legality_of_move_bindings(cx: @AltCheckCtxt,
let mut by_ref_span = None;
let mut any_by_move = false;
for pats.each |pat| {
do pat_bindings(def_map, *pat) |bm, _id, span, _path| {
do pat_bindings(def_map, *pat) |bm, id, span, _path| {
match bm {
bind_by_ref(_) | bind_by_implicit_ref => {
bind_by_ref(_) => {
by_ref_span = Some(span);
}
bind_by_move => {
any_by_move = true;
}
_ => { }
bind_by_value => {}
bind_infer => {
match cx.tcx.value_modes.find(id) {
Some(MoveValue) => any_by_move = true,
Some(CopyValue) | Some(ReadValue) => {}
None => {
cx.tcx.sess.span_bug(span, ~"no mode for pat \
binding");
}
}
}
}
}
}
Expand Down
23 changes: 0 additions & 23 deletions src/librustc/middle/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,27 +767,6 @@ fn check_item_non_camel_case_types(cx: ty::ctxt, it: @ast::item) {
}
}

fn check_pat(tcx: ty::ctxt, pat: @ast::pat) {
debug!("lint check_pat pat=%s", pat_to_str(pat, tcx.sess.intr()));

do pat_bindings(tcx.def_map, pat) |binding_mode, id, span, path| {
match binding_mode {
ast::bind_by_ref(_) | ast::bind_by_value | ast::bind_by_move => {}
ast::bind_by_implicit_ref => {
let pat_ty = ty::node_id_to_type(tcx, id);
let kind = ty::type_kind(tcx, pat_ty);
if !ty::kind_is_safe_for_default_mode(kind) {
tcx.sess.span_lint(
deprecated_pattern, id, id,
span,
fmt!("binding `%s` should use ref or copy mode",
tcx.sess.str_of(path_to_ident(path))));
}
}
}
}
}

fn check_fn(tcx: ty::ctxt, fk: visit::fn_kind, decl: ast::fn_decl,
_body: ast::blk, span: span, id: ast::node_id) {
debug!("lint check_fn fk=%? id=%?", fk, id);
Expand Down Expand Up @@ -904,8 +883,6 @@ fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
check_item(it, tcx),
visit_fn: |fk, decl, body, span, id|
check_fn(tcx, fk, decl, body, span, id),
visit_pat: |pat|
check_pat(tcx, pat),
.. *visit::default_simple_visitor()
});
visit::visit_crate(*crate, (), v);
Expand Down
7 changes: 2 additions & 5 deletions src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,7 @@ impl IrMaps {
Arg(id, _, by_copy) |
Local(LocalInfo {id: id, kind: FromLetNoInitializer, _}) |
Local(LocalInfo {id: id, kind: FromLetWithInitializer, _}) |
Local(LocalInfo {id: id, kind: FromMatch(bind_by_value), _}) |
Local(LocalInfo {id: id, kind: FromMatch(bind_by_ref(_)), _}) |
Local(LocalInfo {id: id, kind: FromMatch(bind_by_move), _}) => {
Local(LocalInfo {id: id, kind: FromMatch(_), _}) => {
let v = match self.last_use_map.find(expr_id) {
Some(v) => v,
None => {
Expand All @@ -405,8 +403,7 @@ impl IrMaps {
(*v).push(id);
}
Arg(_, _, by_ref) |
Arg(_, _, by_val) | Self | ImplicitRet |
Local(LocalInfo {kind: FromMatch(bind_by_implicit_ref), _}) => {
Arg(_, _, by_val) | Self | ImplicitRet => {
debug!("--but it is not owned");
}
}
Expand Down
18 changes: 1 addition & 17 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,28 +636,12 @@ impl &mem_categorization_ctxt {
mutbl:m, ty:expr_ty}
}

ast::def_binding(vid, ast::bind_by_value) |
ast::def_binding(vid, ast::bind_by_move) |
ast::def_binding(vid, ast::bind_by_ref(_)) => {
ast::def_binding(vid, _) => {
// by-value/by-ref bindings are local variables
@{id:id, span:span,
cat:cat_local(vid), lp:Some(@lp_local(vid)),
mutbl:m_imm, ty:expr_ty}
}

ast::def_binding(pid, ast::bind_by_implicit_ref) => {
// implicit-by-ref bindings are "special" since they are
// implicit pointers.

// Technically, the mutability is not always imm, but we
// (choose to be) unsound for the moment since these
// implicit refs are going away and it reduces external
// dependencies.

@{id:id, span:span,
cat:cat_binding(pid), lp:None,
mutbl:m_imm, ty:expr_ty}
}
}
}

Expand Down
43 changes: 33 additions & 10 deletions src/librustc/middle/mode.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use middle::pat_util;
use middle::ty;
use middle::ty::{CopyValue, MoveValue, ReadValue, ValueMode, ctxt};
use middle::typeck::{method_map, method_map_entry};

use core::vec;
use std::map::HashMap;
use syntax::ast::{by_copy, by_move, by_ref, by_val, crate, expr, expr_assign};
use syntax::ast::{expr_addr_of, expr_assign_op, expr_binary, expr_call};
use syntax::ast::{expr_copy, expr_field, expr_index, expr_method_call};
use syntax::ast::{expr_path, expr_swap, expr_unary, node_id, sty_uniq};
use syntax::ast::{sty_value};
use syntax::ast::{box, uniq, deref, not, neg, expr_match, expr_paren};
use syntax::ast::{bind_infer, by_copy, by_move, by_ref, by_val, crate, expr};
use syntax::ast::{expr_addr_of, expr_assign, expr_assign_op, expr_binary};
use syntax::ast::{expr_call, expr_copy, expr_field, expr_index, expr_match};
use syntax::ast::{expr_method_call, expr_paren, expr_path, expr_swap};
use syntax::ast::{expr_unary, node_id, pat, pat_ident, sty_uniq, sty_value};
use syntax::ast::{box, uniq, deref, not, neg};
use syntax::visit;
use syntax::visit::vt;

Expand Down Expand Up @@ -159,18 +160,21 @@ fn compute_modes_for_expr(expr: @expr,
record_mode_for_expr(expr, cx);
}
expr_match(head, ref arms) => {
// We must do this first so that `arms_have_by_move_bindings`
// below knows which bindings are moves.
for arms.each |arm| {
(v.visit_arm)(*arm, cx, v);
}

let by_move_bindings_present =
pat_util::arms_have_by_move_bindings(cx.tcx.def_map, *arms);
pat_util::arms_have_by_move_bindings(cx.tcx, *arms);
if by_move_bindings_present {
// Propagate the current mode flag downward.
visit::visit_expr(expr, cx, v);
} else {
// We aren't moving into any pattern, so this is just a read.
let head_cx = VisitContext { mode: ReadValue, ..cx };
compute_modes_for_expr(head, head_cx, v);
for arms.each |arm| {
(v.visit_arm)(*arm, cx, v);
}
}
}
_ => {
Expand All @@ -181,9 +185,28 @@ fn compute_modes_for_expr(expr: @expr,
}
}

fn compute_modes_for_pat(pat: @pat,
&&cx: VisitContext,
v: vt<VisitContext>) {
match pat.node {
pat_ident(bind_infer, _, _)
if pat_util::pat_is_binding(cx.tcx.def_map, pat) => {
if ty::type_implicitly_moves(cx.tcx, ty::pat_ty(cx.tcx, pat)) {
cx.tcx.value_modes.insert(pat.id, MoveValue);
} else {
cx.tcx.value_modes.insert(pat.id, CopyValue);
}
}
_ => {}
}

visit::visit_pat(pat, cx, v);
}

pub fn compute_modes(tcx: ctxt, method_map: method_map, crate: @crate) {
let visitor = visit::mk_vt(@{
visit_expr: compute_modes_for_expr,
visit_pat: compute_modes_for_pat,
.. *visit::default_visitor()
});
let callee_cx = VisitContext {
Expand Down
21 changes: 16 additions & 5 deletions src/librustc/middle/pat_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use middle::ty::{CopyValue, MoveValue, ReadValue};

use syntax::ast::*;
use syntax::ast_util;
use syntax::ast_util::{path_to_ident, respan, walk_pat};
Expand Down Expand Up @@ -93,16 +95,25 @@ fn pat_binding_ids(dm: resolve::DefMap, pat: @pat) -> ~[node_id] {
return found;
}

fn arms_have_by_move_bindings(dm: resolve::DefMap, +arms: &[arm]) -> bool {
fn arms_have_by_move_bindings(tcx: ty::ctxt, +arms: &[arm]) -> bool {
for arms.each |arm| {
for arm.pats.each |pat| {
let mut found = false;
do pat_bindings(dm, *pat) |binding_mode, _node_id, _span, _path| {
do pat_bindings(tcx.def_map, *pat)
|binding_mode, node_id, span, _path| {
match binding_mode {
bind_by_move => found = true,
bind_by_implicit_ref |
bind_by_ref(*) |
bind_by_value => {}
bind_infer => {
match tcx.value_modes.find(node_id) {
Some(MoveValue) => found = true,
Some(CopyValue) | Some(ReadValue) => {}
None => {
tcx.sess.span_bug(span, ~"pat binding not in \
value mode map");
}
}
}
bind_by_ref(*) | bind_by_value => {}
}
}
if found { return true; }
Expand Down
1 change: 0 additions & 1 deletion src/librustc/middle/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use middle::lang_items::LanguageItems;
use middle::lint::{deny, allow, forbid, level, unused_imports, warn};
use middle::pat_util::{pat_bindings};
use syntax::ast::{_mod, add, arm};
use syntax::ast::{bind_by_ref, bind_by_implicit_ref, bind_by_value};
use syntax::ast::{bitand, bitor, bitxor};
use syntax::ast::{binding_mode, blk, capture_clause, class_ctor, class_dtor};
use syntax::ast::{crate, crate_num, decl_item};
Expand Down
16 changes: 13 additions & 3 deletions src/librustc/middle/trans/alt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ use syntax::ast::def_id;
use syntax::codemap::span;
use syntax::print::pprust::pat_to_str;
use middle::resolve::DefMap;
use middle::ty::{CopyValue, MoveValue, ReadValue};
use back::abi;
use std::map::HashMap;
use dvec::DVec;
Expand Down Expand Up @@ -1394,7 +1395,7 @@ fn trans_alt_inner(scope_cx: block,
// Note that we use the names because each binding will have many ids
// from the various alternatives.
let bindings_map = std::map::HashMap();
do pat_bindings(tcx.def_map, arm.pats[0]) |bm, p_id, _s, path| {
do pat_bindings(tcx.def_map, arm.pats[0]) |bm, p_id, s, path| {
let ident = path_to_ident(path);
let variable_ty = node_id_type(bcx, p_id);
let llvariable_ty = type_of::type_of(bcx.ccx(), variable_ty);
Expand All @@ -1408,9 +1409,18 @@ fn trans_alt_inner(scope_cx: block,
llmatch = alloca(bcx, T_ptr(llvariable_ty));
trmode = TrByValue(is_move, alloca(bcx, llvariable_ty));
}
ast::bind_by_implicit_ref => {
ast::bind_infer => {
// in this case also, the type of the variable will be T,
// but we need to store a *T
let is_move = match tcx.value_modes.find(p_id) {
None => {
tcx.sess.span_bug(s, ~"no value mode");
}
Some(MoveValue) => true,
Some(CopyValue) | Some(ReadValue) => false
};
llmatch = alloca(bcx, T_ptr(llvariable_ty));
trmode = TrByImplicitRef;
trmode = TrByValue(is_move, alloca(bcx, llvariable_ty));
}
ast::bind_by_ref(_) => {
llmatch = alloca(bcx, llvariable_ty);
Expand Down
8 changes: 1 addition & 7 deletions src/librustc/middle/typeck/check/alt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,7 @@ fn check_pat(pcx: pat_ctxt, pat: @ast::pat, expected: ty::t) {
demand::eqtype(fcx, pat.span, region_ty, typ);
}
// otherwise the type of x is the expected type T
ast::bind_by_value => {
demand::eqtype(fcx, pat.span, expected, typ);
}
ast::bind_by_move => {
demand::eqtype(fcx, pat.span, expected, typ);
}
ast::bind_by_implicit_ref => {
ast::bind_by_value | ast::bind_by_move | ast::bind_infer => {
demand::eqtype(fcx, pat.span, expected, typ);
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/libstd/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ pub impl<
Number(v) => v.serialize(s),
String(ref v) => v.serialize(s),
Boolean(v) => v.serialize(s),
List(v) => v.serialize(s),
List(ref v) => v.serialize(s),
Object(ref v) => {
do s.emit_rec || {
let mut idx = 0;
Expand Down Expand Up @@ -927,8 +927,8 @@ impl Json : Eq {
match *other { Boolean(b1) => b0 == b1, _ => false },
Null =>
match *other { Null => true, _ => false },
List(v0) =>
match *other { List(v1) => v0 == v1, _ => false },
List(ref v0) =>
match *other { List(ref v1) => v0 == v1, _ => false },
Object(ref d0) => {
match *other {
Object(ref d1) => {
Expand Down Expand Up @@ -981,10 +981,10 @@ impl Json : Ord {
}
}

List(l0) => {
List(ref l0) => {
match *other {
Number(_) | String(_) | Boolean(_) => false,
List(l1) => l0 < l1,
List(ref l1) => (*l0) < (*l1),
Object(_) | Null => true
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ enum binding_mode {
bind_by_value,
bind_by_move,
bind_by_ref(ast::mutability),
bind_by_implicit_ref
bind_infer
}

impl binding_mode : to_bytes::IterBytes {
Expand All @@ -322,7 +322,7 @@ impl binding_mode : to_bytes::IterBytes {
bind_by_ref(ref m) =>
to_bytes::iter_bytes_2(&2u8, m, lsb0, f),

bind_by_implicit_ref =>
bind_infer =>
3u8.iter_bytes(lsb0, f),
}
}
Expand All @@ -349,9 +349,9 @@ impl binding_mode : cmp::Eq {
_ => false
}
}
bind_by_implicit_ref => {
bind_infer => {
match (*other) {
bind_by_implicit_ref => true,
bind_infer => true,
_ => false
}
}
Expand Down
Loading