From ab5d16a6b21d1360bb4fed20698aca2ead5f9c85 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 27 Apr 2017 13:26:12 -0700 Subject: [PATCH] Adding documentation, indentation fixes --- src/librustc/hir/map/mod.rs | 10 +------ .../borrowck/gather_loans/gather_moves.rs | 30 +++++++++++++++---- .../borrowck/gather_loans/move_error.rs | 28 ++++++++--------- .../ui/issue-40402-ref-hints/issue-40402-1.rs | 1 + .../issue-40402-1.stderr | 4 +-- .../ui/issue-40402-ref-hints/issue-40402-2.rs | 2 ++ .../issue-40402-2.stderr | 4 +-- 7 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 1948dd909f4..48b8a819fff 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -95,14 +95,6 @@ enum MapEntry<'hir> { RootCrate, } -/// Represents the kind of pattern -#[derive(Debug, Clone, Copy)] -pub enum PatternSource<'hir> { - MatchExpr(&'hir Expr), - LetDecl(&'hir Local), - Other, -} - impl<'hir> Clone for MapEntry<'hir> { fn clone(&self) -> MapEntry<'hir> { *self @@ -645,7 +637,7 @@ impl<'hir> Map<'hir> { Err(id) => id, } } - + /// Returns the nearest enclosing scope. A scope is an item or block. /// FIXME it is not clear to me that all items qualify as scopes - statics /// and associated types probably shouldn't, for example. Behaviour in this diff --git a/src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs b/src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs index 18c9858e8d6..f193588dd7d 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; @@ -25,16 +25,36 @@ use syntax::ast; use syntax_pos::Span; use rustc::hir::*; use rustc::hir::map::Node::*; -use rustc::hir::map::{PatternSource}; struct GatherMoveInfo<'tcx> { id: ast::NodeId, kind: MoveKind, cmt: mc::cmt<'tcx>, - span_path_opt: Option> + span_path_opt: Option> } -/// Returns the kind of the Pattern +/// 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); @@ -132,7 +152,7 @@ pub fn gather_move_from_pat<'a, 'tcx>(bccx: &BorrowckCtxt<'a, '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 { + Some(MovePlace { span: move_pat.span, name: path1.node, pat_source: source, diff --git a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs index f047374cab2..9a72f3866a0 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs @@ -17,7 +17,7 @@ use rustc::ty; use syntax::ast; use syntax_pos; use errors::DiagnosticBuilder; -use rustc::hir::map::PatternSource; +use borrowck::gather_loans::gather_moves::PatternSource; pub struct MoveErrorCollector<'tcx> { errors: Vec> @@ -41,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, @@ -56,7 +56,7 @@ impl<'tcx> MoveError<'tcx> { } #[derive(Clone)] -pub struct MoveSpanAndPath<'tcx> { +pub struct MovePlace<'tcx> { pub span: syntax_pos::Span, pub name: ast::Name, pub pat_source: PatternSource<'tcx>, @@ -64,7 +64,7 @@ pub struct MoveSpanAndPath<'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>) { @@ -72,11 +72,11 @@ fn report_move_errors<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, errors: &Vec {} + 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 { @@ -85,14 +85,14 @@ fn report_move_errors<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, errors: &Vec(errors: &Vec>) -> Vec> { 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 index fd42abc752e..7efa3bd9d5b 100644 --- a/src/test/ui/issue-40402-ref-hints/issue-40402-1.rs +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-1.rs @@ -8,6 +8,7 @@ // 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, } 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 index 03b5beee2a4..5e743b6bd3f 100644 --- a/src/test/ui/issue-40402-ref-hints/issue-40402-1.stderr +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-1.stderr @@ -1,7 +1,7 @@ error[E0507]: cannot move out of indexed content - --> $DIR/issue-40402-1.rs:18:13 + --> $DIR/issue-40402-1.rs:19:13 | -18 | let e = f.v[0]; +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 index 0879614243f..76e038b696e 100644 --- a/src/test/ui/issue-40402-ref-hints/issue-40402-2.rs +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-2.rs @@ -8,6 +8,8 @@ // 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 index ab725900015..0060b683bba 100644 --- a/src/test/ui/issue-40402-ref-hints/issue-40402-2.stderr +++ b/src/test/ui/issue-40402-ref-hints/issue-40402-2.stderr @@ -1,7 +1,7 @@ error[E0507]: cannot move out of indexed content - --> $DIR/issue-40402-2.rs:13:18 + --> $DIR/issue-40402-2.rs:15:18 | -13 | let (a, b) = x[0]; +15 | let (a, b) = x[0]; | - - ^^^^ cannot move out of indexed content | | | | | ...and here (use `ref b` or `ref mut b`)