diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 53458fd875673..1a2e36d081b87 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1450,8 +1450,10 @@ fn print_pat(s: ps, &&pat: @ast::pat) { word_nbsp(s, ~"ref"); print_mutability(s, mutbl); } + ast::bind_by_move => { + word_nbsp(s, ~"move"); + } ast::bind_by_implicit_ref | - ast::bind_by_move | // this is totally the wrong thing ast::bind_by_value => {} } print_path(s, path, true); diff --git a/src/rustc/middle/lint.rs b/src/rustc/middle/lint.rs index 7b7e163d38c42..1a26072532a88 100644 --- a/src/rustc/middle/lint.rs +++ b/src/rustc/middle/lint.rs @@ -52,7 +52,11 @@ enum lint { vecs_implicitly_copyable, deprecated_mode, deprecated_pattern, - non_camel_case_types + non_camel_case_types, + + // FIXME(#3266)--make liveness warnings lintable + // unused_variable, + // dead_assignment } fn level_to_str(lv: level) -> ~str { @@ -134,7 +138,19 @@ fn get_lint_dict() -> lint_dict { (~"non_camel_case_types", @{lint: non_camel_case_types, desc: ~"types, variants and traits must have camel case names", - default: allow}) + default: allow}), + + /* FIXME(#3266)--make liveness warnings lintable + (~"unused_variable", + @{lint: unused_variable, + desc: ~"detect variables which are not used in any way", + default: warn}), + + (~"dead_assignment", + @{lint: dead_assignment, + desc: ~"detect assignments that will never be read", + default: warn}), + */ ]; hash_from_strs(v) } diff --git a/src/rustc/middle/liveness.rs b/src/rustc/middle/liveness.rs index 4f15e3b1a9aa8..ee8c6c2baeca2 100644 --- a/src/rustc/middle/liveness.rs +++ b/src/rustc/middle/liveness.rs @@ -140,7 +140,8 @@ fn check_crate(tcx: ty::ctxt, let visitor = visit::mk_vt(@{ visit_fn: visit_fn, visit_local: visit_local, - visit_expr: visit_expr + visit_expr: visit_expr, + visit_arm: visit_arm, with *visit::default_visitor() }); @@ -191,11 +192,17 @@ enum RelevantDef { RelevantVar(node_id), RelevantSelf } type CaptureInfo = {ln: LiveNode, is_move: bool, rv: RelevantDef}; +enum LocalKind { + FromMatch(binding_mode), + FromLetWithInitializer, + FromLetNoInitializer +} + struct LocalInfo { id: node_id; ident: ident; is_mutbl: bool; - initialized: bool; + kind: LocalKind; } enum VarKind { @@ -209,7 +216,11 @@ enum VarKind { fn relevant_def(def: def) -> option { match def { def_self(_) => some(RelevantSelf), - def_arg(nid, _) | def_local(nid, _) => some(RelevantVar(nid)), + + def_binding(nid, _) | + def_arg(nid, _) | + def_local(nid, _) => some(RelevantVar(nid)), + _ => none } } @@ -329,7 +340,16 @@ impl IrMaps { match vk { Arg(id, name, by_move) | Arg(id, name, by_copy) | - Local(LocalInfo {id:id, ident:name, _}) => { + Local(LocalInfo {id:id, ident:name, + kind: FromLetNoInitializer, _}) | + Local(LocalInfo {id:id, ident:name, + kind: FromLetWithInitializer, _}) | + Local(LocalInfo {id:id, ident:name, + kind: FromMatch(bind_by_value), _}) | + Local(LocalInfo {id:id, ident:name, + kind: FromMatch(bind_by_ref(_)), _}) | + Local(LocalInfo {id:id, ident:name, + kind: FromMatch(bind_by_move), _}) => { let v = match self.last_use_map.find(expr_id) { some(v) => v, none => { @@ -342,7 +362,8 @@ impl IrMaps { (*v).push(id); } Arg(_, _, by_ref) | Arg(_, _, by_mutbl_ref) | - Arg(_, _, by_val) | Self | Field(_) | ImplicitRet => { + Arg(_, _, by_val) | Self | Field(_) | ImplicitRet | + Local(LocalInfo {kind: FromMatch(bind_by_implicit_ref), _}) => { debug!("--but it is not owned"); } } @@ -396,7 +417,8 @@ fn visit_fn(fk: visit::fn_kind, decl: fn_decl, body: blk, let check_vt = visit::mk_vt(@{ visit_fn: check_fn, visit_local: check_local, - visit_expr: check_expr + visit_expr: check_expr, + visit_arm: check_arm, with *visit::default_visitor() }); check_vt.visit_block(body, lsets, check_vt); @@ -419,16 +441,39 @@ fn visit_local(local: @local, &&self: @IrMaps, vt: vt<@IrMaps>) { debug!("adding local variable %d", p_id); let name = ast_util::path_to_ident(path); self.add_live_node_for_node(p_id, VarDefNode(sp)); + let kind = match local.node.init { + some(_) => FromLetWithInitializer, + none => FromLetNoInitializer + }; self.add_variable(Local(LocalInfo { - id: p_id, - ident: name, - is_mutbl: local.node.is_mutbl, - initialized: local.node.init.is_some() + id: p_id, + ident: name, + is_mutbl: local.node.is_mutbl, + kind: kind })); } visit::visit_local(local, self, vt); } +fn visit_arm(arm: arm, &&self: @IrMaps, vt: vt<@IrMaps>) { + let def_map = self.tcx.def_map; + for arm.pats.each |pat| { + do pat_util::pat_bindings(def_map, pat) |bm, p_id, sp, path| { + debug!("adding local variable %d from match with bm %?", + p_id, bm); + let name = ast_util::path_to_ident(path); + self.add_live_node_for_node(p_id, VarDefNode(sp)); + self.add_variable(Local(LocalInfo { + id: p_id, + ident: name, + is_mutbl: false, + kind: FromMatch(bm) + })); + } + } + visit::visit_arm(arm, self, vt); +} + fn visit_expr(expr: @expr, &&self: @IrMaps, vt: vt<@IrMaps>) { match expr.node { // live nodes required for uses or definitions of variables: @@ -612,6 +657,30 @@ impl Liveness { } } + fn arm_pats_bindings(pats: &[@pat], f: fn(LiveNode, Variable, span)) { + // only consider the first pattern; any later patterns must have + // the same bindings, and we also consider the first pattern to be + // the "authoratative" set of ids + if !pats.is_empty() { + self.pat_bindings(pats[0], f) + } + } + + fn define_bindings_in_pat(pat: @pat, succ: LiveNode) -> LiveNode { + self.define_bindings_in_arm_pats([pat], succ) + } + + fn define_bindings_in_arm_pats(pats: &[@pat], + succ: LiveNode) -> LiveNode { + let mut succ = succ; + do self.arm_pats_bindings(pats) |ln, var, _sp| { + self.init_from_succ(ln, succ); + self.define(ln, var); + succ = ln; + } + succ + } + fn idx(ln: LiveNode, var: Variable) -> uint { *ln * self.ir.num_vars + *var } @@ -894,13 +963,8 @@ impl Liveness { // once at the func header but otherwise equivalent. let opt_init = local.node.init.map(|i| i.expr ); - let mut succ = self.propagate_through_opt_expr(opt_init, succ); - do self.pat_bindings(local.node.pat) |ln, var, _sp| { - self.init_from_succ(ln, succ); - self.define(ln, var); - succ = ln; - } - succ + let succ = self.propagate_through_opt_expr(opt_init, succ); + self.define_bindings_in_pat(local.node.pat, succ) } fn propagate_through_exprs(exprs: ~[@expr], @@ -1003,10 +1067,12 @@ impl Liveness { self.init_empty(ln, succ); let mut first_merge = true; for arms.each |arm| { + let body_succ = + self.propagate_through_block(arm.body, succ); + let guard_succ = + self.propagate_through_opt_expr(arm.guard, body_succ); let arm_succ = - self.propagate_through_opt_expr( - arm.guard, - self.propagate_through_block(arm.body, succ)); + self.define_bindings_in_arm_pats(arm.pats, guard_succ); self.merge_from_succ(ln, arm_succ, first_merge); first_merge = false; }; @@ -1409,6 +1475,13 @@ fn check_local(local: @local, &&self: @Liveness, vt: vt<@Liveness>) { visit::visit_local(local, self, vt); } +fn check_arm(arm: arm, &&self: @Liveness, vt: vt<@Liveness>) { + do self.arm_pats_bindings(arm.pats) |ln, var, sp| { + self.warn_about_unused(sp, ln, var); + } + visit::visit_arm(arm, self, vt); +} + fn check_expr(expr: @expr, &&self: @Liveness, vt: vt<@Liveness>) { match expr.node { expr_path(_) => { @@ -1798,10 +1871,12 @@ impl @Liveness { }; if is_assigned { + // FIXME(#3266)--make liveness warnings lintable self.tcx.sess.span_warn( sp, fmt!("variable `%s` is assigned to, \ but never used", name)); } else { + // FIXME(#3266)--make liveness warnings lintable self.tcx.sess.span_warn( sp, fmt!("unused variable: `%s`", name)); } @@ -1814,6 +1889,7 @@ impl @Liveness { fn warn_about_dead_assign(sp: span, ln: LiveNode, var: Variable) { if self.live_on_exit(ln, var).is_none() { for self.should_warn(var).each |name| { + // FIXME(#3266)--make liveness warnings lintable self.tcx.sess.span_warn( sp, fmt!("value assigned to `%s` is never read", name)); diff --git a/src/test/compile-fail/borrowck-issue-2657-1.rs b/src/test/compile-fail/borrowck-issue-2657-1.rs index 9bfc4cc1d0d05..29a05ce5410a0 100644 --- a/src/test/compile-fail/borrowck-issue-2657-1.rs +++ b/src/test/compile-fail/borrowck-issue-2657-1.rs @@ -1,7 +1,7 @@ fn main() { let x = some(~1); match x { //~ NOTE loan of immutable local variable granted here - some(ref y) => { + some(ref _y) => { let _a <- x; //~ ERROR moving out of immutable local variable prohibited due to outstanding loan } _ => {} diff --git a/src/test/compile-fail/borrowck-pat-reassign-sometimes-binding.rs b/src/test/compile-fail/borrowck-pat-reassign-sometimes-binding.rs index e0d96fb9e267c..9eb8a620da9d5 100644 --- a/src/test/compile-fail/borrowck-pat-reassign-sometimes-binding.rs +++ b/src/test/compile-fail/borrowck-pat-reassign-sometimes-binding.rs @@ -8,7 +8,7 @@ fn main() { // fact no outstanding loan of x! x = some(0); } - some(ref i) => { + some(ref _i) => { x = some(1); //~ ERROR assigning to mutable local variable prohibited due to outstanding loan } } diff --git a/src/test/compile-fail/liveness-unused.rs b/src/test/compile-fail/liveness-unused.rs index f778a8da32329..c0a3b8821da89 100644 --- a/src/test/compile-fail/liveness-unused.rs +++ b/src/test/compile-fail/liveness-unused.rs @@ -29,11 +29,24 @@ fn f3b() { fn f4() { match some(3) { some(i) => { + //~^ WARNING unused variable: `i` } none => {} } } +enum tri { + a(int), b(int), c(int) +} + +fn f4b() -> int { + match a(3) { + a(i) | b(i) | c(i) => { + i + } + } +} + // leave this in here just to trigger compile-fail: struct r { let x: (); diff --git a/src/test/run-pass/option-unwrap.rs b/src/test/run-pass/option-unwrap.rs new file mode 100644 index 0000000000000..bbb8c3eb5c99d --- /dev/null +++ b/src/test/run-pass/option-unwrap.rs @@ -0,0 +1,26 @@ +struct dtor { + x: @mut int; + + drop { + // abuse access to shared mutable state to write this code + *self.x -= 1; + } +} + +fn unwrap(+o: option) -> T { + match move o { + some(move v) => v, + none => fail + } +} + +fn main() { + let x = @mut 1; + + { + let b = some(dtor { x:x }); + let c = unwrap(b); + } + + assert *x == 0; +} \ No newline at end of file