diff --git a/src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs b/src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs index 0577ba7f45a93..f193588dd7d6c 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs @@ -11,7 +11,7 @@ //! Computes moves. use borrowck::*; -use borrowck::gather_loans::move_error::MoveSpanAndPath; +use borrowck::gather_loans::move_error::MovePlace; use borrowck::gather_loans::move_error::{MoveError, MoveErrorCollector}; use borrowck::move_data::*; use rustc::middle::expr_use_visitor as euv; @@ -23,13 +23,67 @@ use rustc::ty::{self, Ty}; use std::rc::Rc; use syntax::ast; use syntax_pos::Span; -use rustc::hir::{self, PatKind}; +use rustc::hir::*; +use rustc::hir::map::Node::*; struct GatherMoveInfo<'tcx> { id: ast::NodeId, kind: MoveKind, cmt: mc::cmt<'tcx>, - span_path_opt: Option + span_path_opt: Option> +} + +/// Represents the kind of pattern +#[derive(Debug, Clone, Copy)] +pub enum PatternSource<'tcx> { + MatchExpr(&'tcx Expr), + LetDecl(&'tcx Local), + Other, +} + +/// Analyzes the context where the pattern appears to determine the +/// kind of hint we want to give. In particular, if the pattern is in a `match` +/// or nested within other patterns, we want to suggest a `ref` binding: +/// +/// let (a, b) = v[0]; // like the `a` and `b` patterns here +/// match v[0] { a => ... } // or the `a` pattern here +/// +/// But if the pattern is the outermost pattern in a `let`, we would rather +/// suggest that the author add a `&` to the initializer: +/// +/// let x = v[0]; // suggest `&v[0]` here +/// +/// In this latter case, this function will return `PatternSource::LetDecl` +/// with a reference to the let +fn get_pattern_source<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, pat: &Pat) -> PatternSource<'tcx> { + + let parent = tcx.hir.get_parent_node(pat.id); + + match tcx.hir.get(parent) { + NodeExpr(ref e) => { + // the enclosing expression must be a `match` or something else + assert!(match e.node { + ExprMatch(..) => true, + _ => return PatternSource::Other, + }); + PatternSource::MatchExpr(e) + } + NodeStmt(ref s) => { + // the enclosing statement must be a `let` or something else + match s.node { + StmtDecl(ref decl, _) => { + match decl.node { + DeclLocal(ref local) => PatternSource::LetDecl(local), + _ => return PatternSource::Other, + } + } + _ => return PatternSource::Other, + } + } + + _ => return PatternSource::Other, + + } } pub fn gather_decl<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, @@ -95,11 +149,15 @@ pub fn gather_move_from_pat<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, move_error_collector: &mut MoveErrorCollector<'tcx>, move_pat: &hir::Pat, cmt: mc::cmt<'tcx>) { + let source = get_pattern_source(bccx.tcx,move_pat); let pat_span_path_opt = match move_pat.node { PatKind::Binding(_, _, ref path1, _) => { - Some(MoveSpanAndPath{span: move_pat.span, - name: path1.node}) - }, + Some(MovePlace { + span: move_pat.span, + name: path1.node, + pat_source: source, + }) + } _ => None, }; let move_info = GatherMoveInfo { @@ -108,6 +166,11 @@ pub fn gather_move_from_pat<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, cmt: cmt, span_path_opt: pat_span_path_opt, }; + + debug!("gather_move_from_pat: move_pat={:?} source={:?}", + move_pat, + source); + gather_move(bccx, move_data, move_error_collector, move_info); } diff --git a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs index ebe2de5840954..9a72f3866a0e1 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs @@ -17,6 +17,7 @@ use rustc::ty; use syntax::ast; use syntax_pos; use errors::DiagnosticBuilder; +use borrowck::gather_loans::gather_moves::PatternSource; pub struct MoveErrorCollector<'tcx> { errors: Vec> @@ -40,12 +41,12 @@ impl<'tcx> MoveErrorCollector<'tcx> { pub struct MoveError<'tcx> { move_from: mc::cmt<'tcx>, - move_to: Option + move_to: Option> } impl<'tcx> MoveError<'tcx> { pub fn with_move_info(move_from: mc::cmt<'tcx>, - move_to: Option) + move_to: Option>) -> MoveError<'tcx> { MoveError { move_from: move_from, @@ -55,30 +56,41 @@ impl<'tcx> MoveError<'tcx> { } #[derive(Clone)] -pub struct MoveSpanAndPath { +pub struct MovePlace<'tcx> { pub span: syntax_pos::Span, pub name: ast::Name, + pub pat_source: PatternSource<'tcx>, } pub struct GroupedMoveErrors<'tcx> { move_from: mc::cmt<'tcx>, - move_to_places: Vec + move_to_places: Vec> } -fn report_move_errors<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, - errors: &Vec>) { +fn report_move_errors<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, errors: &Vec>) { let grouped_errors = group_errors_with_same_origin(errors); for error in &grouped_errors { let mut err = report_cannot_move_out_of(bccx, error.move_from.clone()); let mut is_first_note = true; - for move_to in &error.move_to_places { - err = note_move_destination(err, move_to.span, move_to.name, is_first_note); - is_first_note = false; + match error.move_to_places.get(0) { + Some(&MovePlace { pat_source: PatternSource::LetDecl(_), .. }) => { + // ignore patterns that are found at the top-level of a `let`; + // see `get_pattern_source()` for details + } + _ => { + for move_to in &error.move_to_places { + + err = note_move_destination(err, move_to.span, move_to.name, is_first_note); + is_first_note = false; + } + } } if let NoteClosureEnv(upvar_id) = error.move_from.note { - err.span_label(bccx.tcx.hir.span(upvar_id.var_id), &"captured outer variable"); + err.span_label(bccx.tcx.hir.span(upvar_id.var_id), + &"captured outer variable"); } err.emit(); + } } diff --git a/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs b/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs index d26364efdbc5d..98bb6b14b945c 100644 --- a/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs +++ b/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs @@ -54,7 +54,6 @@ fn c() { _ => {} } let a = vec[0]; //~ ERROR cannot move out - //~^ NOTE to prevent move //~| cannot move out of here } @@ -68,7 +67,6 @@ fn d() { _ => {} } let a = vec[0]; //~ ERROR cannot move out - //~^ NOTE to prevent move //~| cannot move out of here } @@ -84,7 +82,6 @@ fn e() { _ => {} } let a = vec[0]; //~ ERROR cannot move out - //~^ NOTE to prevent move //~| cannot move out of here } diff --git a/src/test/ui/issue-40402-ref-hints/issue-40402-1.rs b/src/test/ui/issue-40402-ref-hints/issue-40402-1.rs new file mode 100644 index 0000000000000..7efa3bd9d5b31 --- /dev/null +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-1.rs @@ -0,0 +1,20 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Check that we do not suggest `ref f` here in the `main()` function. +struct Foo { + pub v: Vec, +} + +fn main() { + let mut f = Foo { v: Vec::new() }; + f.v.push("hello".to_string()); + let e = f.v[0]; +} diff --git a/src/test/ui/issue-40402-ref-hints/issue-40402-1.stderr b/src/test/ui/issue-40402-ref-hints/issue-40402-1.stderr new file mode 100644 index 0000000000000..5e743b6bd3fe7 --- /dev/null +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-1.stderr @@ -0,0 +1,8 @@ +error[E0507]: cannot move out of indexed content + --> $DIR/issue-40402-1.rs:19:13 + | +19 | let e = f.v[0]; + | ^^^^^^ cannot move out of indexed content + +error: aborting due to previous error + diff --git a/src/test/ui/issue-40402-ref-hints/issue-40402-2.rs b/src/test/ui/issue-40402-ref-hints/issue-40402-2.rs new file mode 100644 index 0000000000000..76e038b696e8f --- /dev/null +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-2.rs @@ -0,0 +1,16 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Check that we do suggest `(ref a, ref b)` here, since `a` and `b` +// are nested within a pattern +fn main() { + let x = vec![(String::new(), String::new())]; + let (a, b) = x[0]; +} diff --git a/src/test/ui/issue-40402-ref-hints/issue-40402-2.stderr b/src/test/ui/issue-40402-ref-hints/issue-40402-2.stderr new file mode 100644 index 0000000000000..0060b683bba43 --- /dev/null +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-2.stderr @@ -0,0 +1,11 @@ +error[E0507]: cannot move out of indexed content + --> $DIR/issue-40402-2.rs:15:18 + | +15 | let (a, b) = x[0]; + | - - ^^^^ cannot move out of indexed content + | | | + | | ...and here (use `ref b` or `ref mut b`) + | hint: to prevent move, use `ref a` or `ref mut a` + +error: aborting due to previous error +