auto merge of #5966 : alexcrichton/rust/issue-3083, r=graydon

Closes #3083.

This takes a similar approach to #5797 where a set is present on the `tcx` of used mutable definitions. Everything is by default warned about, and analyses must explicitly add mutable definitions to this set so they're not warned about.

Most of this was pretty straightforward, although there was one caveat that I ran into when implementing it. Apparently when the old modes are used (or maybe `legacy_modes`, I'm not sure) some different code paths are taken to cause spurious warnings to be issued which shouldn't be issued. I'm not really sure how modes even worked, so I was having a lot of trouble tracking this down. I figured that because they're a legacy thing that I'd just de-mode the compiler so that the warnings wouldn't be a problem anymore (or at least for the compiler).

Other than that, the entire compiler compiles without warnings of unused mutable variables. To prevent bad warnings, #5965 should be landed (which in turn is waiting on #5963) before landing this. I figured I'd stick it out for review anyway though.

src/libcore/cell.rs

@@ -121,7 +121,7 @@ fn test_with_ref() {
 #[test]
 fn test_with_mut_ref() {
     let good = ~[1, 2, 3];
-    let mut v = ~[1, 2];
+    let v = ~[1, 2];
     let c = Cell(v);
     do c.with_mut_ref() |v| { v.push(3); }
     let v = c.take();

src/libcore/flate.rs

@@ -67,7 +67,7 @@ pub fn deflate_bytes(bytes: &const [u8]) -> ~[u8] {
 pub fn inflate_bytes(bytes: &const [u8]) -> ~[u8] {
     do vec::as_const_buf(bytes) |b, len| {
         unsafe {
-            let mut outsz : size_t = 0;
+            let outsz : size_t = 0;
             let res =
                 rustrt::tinfl_decompress_mem_to_heap(b as *c_void,
                                                      len as size_t,

src/libcore/path.rs

@@ -854,7 +854,7 @@ pub mod windows {
             while i < s.len() {
                 if is_sep(s[i]) {
                     let pre = s.slice(2, i).to_owned();
-                    let mut rest = s.slice(i, s.len()).to_owned();
+                    let rest = s.slice(i, s.len()).to_owned();
                     return Some((pre, rest));
                 }
                 i += 1;

src/libcore/rand.rs

@@ -742,7 +742,7 @@ struct XorShiftState {
 impl Rng for XorShiftState {
     fn next(&self) -> u32 {
         let x = self.x;
-        let mut t = x ^ (x << 11);
+        let t = x ^ (x << 11);
         self.x = self.y;
         self.y = self.z;
         self.z = self.w;

src/libcore/repr.rs

@@ -210,7 +210,7 @@ pub impl ReprVisitor {
     #[inline(always)]
     fn visit_ptr_inner(&self, ptr: *c_void, inner: *TyDesc) -> bool {
         unsafe {
-            let mut u = ReprVisitor(ptr, self.writer);
+            let u = ReprVisitor(ptr, self.writer);
             let v = reflect::MovePtrAdaptor(u);
             visit_tydesc(inner, @v as @TyVisitor);
             true
@@ -667,7 +667,7 @@ pub fn write_repr<T>(writer: @Writer, object: &T) {
     unsafe {
         let ptr = ptr::to_unsafe_ptr(object) as *c_void;
         let tydesc = intrinsic::get_tydesc::<T>();
-        let mut u = ReprVisitor(ptr, writer);
+        let u = ReprVisitor(ptr, writer);
         let v = reflect::MovePtrAdaptor(u);
         visit_tydesc(tydesc, @v as @TyVisitor)
     }

src/libcore/rt/uv/mod.rs

@@ -402,7 +402,7 @@ fn loop_smoke_test() {
 fn idle_new_then_close() {
     do run_in_bare_thread {
         let mut loop_ = Loop::new();
-        let mut idle_watcher = { IdleWatcher::new(&mut loop_) };
+        let idle_watcher = { IdleWatcher::new(&mut loop_) };
         idle_watcher.close();
     }
 }

src/libcore/rt/uv/net.rs

@@ -393,7 +393,7 @@ fn connect_read() {
                 let buf = vec_from_uv_buf(buf);
                 rtdebug!("read cb!");
                 if status.is_none() {
-                    let bytes = buf.unwrap();
+                    let _bytes = buf.unwrap();
                     rtdebug!("%s", bytes.slice(0, nread as uint).to_str());
                 } else {
                     rtdebug!("status after read: %s", status.get().to_str());

src/libcore/rt/uvio.rs

@@ -206,7 +206,7 @@ impl TcpListener for UvTcpListener {
                     let mut server_stream_watcher = server_stream_watcher;
                     let mut loop_ = loop_from_watcher(&server_stream_watcher);
                     let mut client_tcp_watcher = TcpWatcher::new(&mut loop_);
-                    let mut client_tcp_watcher = client_tcp_watcher.as_stream();
+                    let client_tcp_watcher = client_tcp_watcher.as_stream();
                     // XXX: Need's to be surfaced in interface
                     server_stream_watcher.accept(client_tcp_watcher);
                     Some(~UvStream::new(client_tcp_watcher))

src/libcore/run.rs

@@ -343,7 +343,7 @@ pub fn start_program(prog: &str, args: &[~str]) -> @Program {
         fn force_destroy(&mut self) { destroy_repr(&mut self.r, true); }
     }
 
-    let mut repr = ProgRepr {
+    let repr = ProgRepr {
         pid: pid,
         in_fd: pipe_input.out,
         out_file: os::fdopen(pipe_output.in),

src/libcore/str.rs

@@ -673,7 +673,7 @@ pub fn levdistance(s: &str, t: &str) -> uint {
 
         for t.each_chari |j, tc| {
 
-            let mut next = dcol[j + 1];
+            let next = dcol[j + 1];
 
             if sc == tc {
                 dcol[j + 1] = current;
@@ -909,7 +909,7 @@ impl TotalOrd for @str {
 /// Bytewise slice less than
 fn lt(a: &str, b: &str) -> bool {
     let (a_len, b_len) = (a.len(), b.len());
-    let mut end = uint::min(a_len, b_len);
+    let end = uint::min(a_len, b_len);
 
     let mut i = 0;
     while i < end {
@@ -1715,7 +1715,7 @@ pub fn utf16_chars(v: &[u16], f: &fn(char)) {
     let len = vec::len(v);
     let mut i = 0u;
     while (i < len && v[i] != 0u16) {
-        let mut u = v[i];
+        let u = v[i];
 
         if  u <= 0xD7FF_u16 || u >= 0xE000_u16 {
             f(u as char);

src/libcore/task/spawn.rs

@@ -575,7 +575,7 @@ fn spawn_raw_oldsched(opts: TaskOpts, f: ~fn()) {
             };
             assert!(!new_task.is_null());
             // Getting killed after here would leak the task.
-            let mut notify_chan = if opts.notify_chan.is_none() {
+            let notify_chan = if opts.notify_chan.is_none() {
                 None
             } else {
                 Some(opts.notify_chan.swap_unwrap())

src/libcore/unstable/extfmt.rs

@@ -538,7 +538,7 @@ pub mod rt {
     pub fn conv_str(cv: Conv, s: &str, buf: &mut ~str) {
         // For strings, precision is the maximum characters
         // displayed
-        let mut unpadded = match cv.precision {
+        let unpadded = match cv.precision {
           CountImplied => s,
           CountIs(max) => if (max as uint) < str::char_len(s) {
             str::slice(s, 0, max as uint)
@@ -596,7 +596,7 @@ pub mod rt {
     #[deriving(Eq)]
     pub enum PadMode { PadSigned, PadUnsigned, PadNozero, PadFloat }
 
-    pub fn pad(cv: Conv, mut s: &str, head: Option<char>, mode: PadMode,
+    pub fn pad(cv: Conv, s: &str, head: Option<char>, mode: PadMode,
                buf: &mut ~str) {
         let headsize = match head { Some(_) => 1, _ => 0 };
         let uwidth : uint = match cv.width {

src/libcore/vec.rs

@@ -1755,7 +1755,7 @@ impl<T: TotalOrd> TotalOrd for @[T] {
 
 fn lt<T:Ord>(a: &[T], b: &[T]) -> bool {
     let (a_len, b_len) = (a.len(), b.len());
-    let mut end = uint::min(a_len, b_len);
+    let end = uint::min(a_len, b_len);
 
     let mut i = 0;
     while i < end {
@@ -3897,7 +3897,7 @@ mod tests {
 
     #[test]
     fn reversed_mut() {
-        let mut v2 = reversed::<int>(~[10, 20]);
+        let v2 = reversed::<int>(~[10, 20]);
         assert!(v2[0] == 20);
         assert!(v2[1] == 10);
     }

src/librustc/back/link.rs

@@ -273,7 +273,7 @@ pub mod write {
                 let LLVMOptDefault    = 2 as c_int; // -O2, -Os
                 let LLVMOptAggressive = 3 as c_int; // -O3
 
-                let mut CodeGenOptLevel = match opts.optimize {
+                let CodeGenOptLevel = match opts.optimize {
                   session::No => LLVMOptNone,
                   session::Less => LLVMOptLess,
                   session::Default => LLVMOptDefault,
@@ -294,7 +294,7 @@ pub mod write {
                     return;
                 }
 
-                let mut FileType;
+                let FileType;
                 if output_type == output_type_object ||
                        output_type == output_type_exe {
                    FileType = lib::llvm::ObjectFile;
@@ -820,7 +820,7 @@ pub fn link_binary(sess: Session,
     cc_args.push(output.to_str());
     cc_args.push(obj_filename.to_str());
 
-    let mut lib_cmd;
+    let lib_cmd;
     let os = sess.targ_cfg.os;
     if os == session::os_macos {
         lib_cmd = ~"-dynamiclib";

src/librustc/driver/driver.rs

@@ -349,7 +349,7 @@ pub fn compile_upto(sess: Session, cfg: ast::crate_cfg,
                 outputs: Option<@OutputFilenames>)
     -> (@ast::crate, Option<ty::ctxt>) {
     let time_passes = sess.time_passes();
-    let mut crate = time(time_passes, ~"parsing",
+    let crate = time(time_passes, ~"parsing",
                          || parse_input(sess, copy cfg, input) );
     if upto == cu_parse { return (crate, None); }
 

src/librustc/metadata/encoder.rs

@@ -1341,7 +1341,7 @@ pub static metadata_encoding_version : &'static [u8] =
 
 pub fn encode_metadata(parms: EncodeParams, crate: &crate) -> ~[u8] {
     let wr = @io::BytesWriter();
-    let mut stats = Stats {
+    let stats = Stats {
         inline_bytes: 0,
         attr_bytes: 0,
         dep_bytes: 0,

src/librustc/middle/borrowck/check_loans.rs

@@ -367,7 +367,18 @@ pub impl CheckLoanCtxt {
             // are only assigned once
         } else {
             match cmt.mutbl {
-                McDeclared | McInherited => { /*ok*/ }
+                McDeclared | McInherited => {
+                    // Ok, but if this loan is a mutable loan, then mark the
+                    // loan path (if it exists) as being used. This is similar
+                    // to the check performed in loan.rs in issue_loan(). This
+                    // type of use of mutable is different from issuing a loan,
+                    // however.
+                    for cmt.lp.each |lp| {
+                        for lp.node_id().each |&id| {
+                            self.tcx().used_mut_nodes.insert(id);
+                        }
+                    }
+                }
                 McReadOnly | McImmutable => {
                     self.bccx.span_err(
                         ex.span,

src/librustc/middle/borrowck/gather_loans.rs

@@ -305,7 +305,7 @@ pub impl GatherLoanCtxt {
                 let mcx = &mem_categorization_ctxt {
                     tcx: self.tcx(),
                     method_map: self.bccx.method_map};
-                let mut cmt = mcx.cat_expr_autoderefd(expr, autoderefs);
+                let cmt = mcx.cat_expr_autoderefd(expr, autoderefs);
                 debug!("after autoderef, cmt=%s", self.bccx.cmt_to_repr(cmt));
 
                 match autoref.kind {

src/librustc/middle/borrowck/loan.rs

@@ -274,7 +274,17 @@ pub impl LoanContext {
         if !owns_lent_data ||
             self.bccx.is_subregion_of(self.scope_region, scope_ub)
         {
-            if loan_kind.is_take() && !cmt.mutbl.is_mutable() {
+            if cmt.mutbl.is_mutable() {
+                // If this loan is a mutable loan, then mark the loan path (if
+                // it exists) as being used. This is similar to the check
+                // performed in check_loans.rs in check_assignment(), but this
+                // is for a different purpose of having the 'mut' qualifier.
+                for cmt.lp.each |lp| {
+                    for lp.node_id().each |&id| {
+                        self.tcx().used_mut_nodes.insert(id);
+                    }
+                }
+            } else if loan_kind.is_take() {
                 // We do not allow non-mutable data to be "taken"
                 // under any circumstances.
                 return Err(bckerr {

src/librustc/middle/check_match.rs

@@ -481,7 +481,7 @@ pub fn specialize(cx: @MatchCheckCtxt,
                   left_ty: ty::t)
                -> Option<~[@pat]> {
     // Sad, but I can't get rid of this easily
-    let mut r0 = copy *raw_pat(r[0]);
+    let r0 = copy *raw_pat(r[0]);
     match r0 {
         pat{id: pat_id, node: n, span: pat_span} =>
             match n {

src/librustc/middle/lint.rs

@@ -13,6 +13,7 @@ use core::prelude::*;
 use driver::session::Session;
 use driver::session;
 use middle::ty;
+use middle::pat_util;
 use util::ppaux::{ty_to_str};
 
 use core::hashmap::HashMap;
@@ -86,6 +87,7 @@ pub enum lint {
 
     unused_variable,
     dead_assignment,
+    unused_mut,
 }
 
 pub fn level_to_str(lv: level) -> &'static str {
@@ -277,6 +279,13 @@ pub fn get_lint_dict() -> LintDict {
             desc: "detect assignments that will never be read",
             default: warn
         }),
+
+        (~"unused_mut",
+         LintSpec {
+            lint: unused_mut,
+            desc: "detect mut variables which don't need to be mutable",
+            default: warn
+        }),
     ];
     let mut map = HashMap::new();
     do vec::consume(v) |_, (k, v)| {
@@ -499,6 +508,7 @@ fn check_item(i: @ast::item, cx: ty::ctxt) {
     check_item_deprecated_mutable_fields(cx, i);
     check_item_deprecated_drop(cx, i);
     check_item_unused_unsafe(cx, i);
+    check_item_unused_mut(cx, i);
 }
 
 // Take a visitor, and modify it so that it will not proceed past subitems.
@@ -954,6 +964,53 @@ fn check_item_unused_unsafe(cx: ty::ctxt, it: @ast::item) {
     visit::visit_item(it, (), visit);
 }
 
+fn check_item_unused_mut(tcx: ty::ctxt, it: @ast::item) {
+    let check_pat: @fn(@ast::pat) = |p| {
+        let mut used = false;
+        let mut bindings = 0;
+        do pat_util::pat_bindings(tcx.def_map, p) |_, id, _, _| {
+            used = used || tcx.used_mut_nodes.contains(&id);
+            bindings += 1;
+        }
+        if !used {
+            let msg = if bindings == 1 {
+                ~"variable does not need to be mutable"
+            } else {
+                ~"variables do not need to be mutable"
+            };
+            tcx.sess.span_lint(unused_mut, p.id, it.id, p.span, msg);
+        }
+    };
+
+    let visit_fn_decl: @fn(&ast::fn_decl) = |fd| {
+        for fd.inputs.each |arg| {
+            if arg.is_mutbl {
+                check_pat(arg.pat);
+            }
+        }
+    };
+
+    let visit = item_stopping_visitor(
+        visit::mk_simple_visitor(@visit::SimpleVisitor {
+            visit_local: |l| {
+                if l.node.is_mutbl {
+                    check_pat(l.node.pat);
+                }
+            },
+            visit_fn: |_, fd, _, _, _| visit_fn_decl(fd),
+            visit_ty_method: |tm| visit_fn_decl(&tm.decl),
+            visit_struct_method: |sm| visit_fn_decl(&sm.decl),
+            visit_trait_method: |tm| {
+                match *tm {
+                    ast::required(ref tm) => visit_fn_decl(&tm.decl),
+                    ast::provided(m) => visit_fn_decl(&m.decl),
+                }
+            },
+            .. *visit::default_simple_visitor()
+        }));
+    visit::visit_item(it, (), visit);
+}
+
 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);