Skip to content

Commit

Permalink
Auto merge of #53147 - ashtneoi:dont-suggest-ref, r=estebank
Browse files Browse the repository at this point in the history
For move errors, suggest match ergonomics instead of `ref`

Partially fixes issue #52423. Also makes errors and suggestions more consistent between move-from-place and move-from-value errors.

Limitations:
- Only the first pattern in a match arm can have a "consider removing this borrow operator" suggestion.
- Suggestions don't always compile as-is (see the TODOs in the test for details).

Sorry for the really long test. I wanted to make sure I handled every case I could think of, and it turned out there were a lot of them.

Questions:
- Is there any particular applicability I should set on those suggestions?
- Are the notes about the `Copy` trait excessive?
  • Loading branch information
bors committed Aug 16, 2018
2 parents 996e26c + 0023dd9 commit 142bb27
Show file tree
Hide file tree
Showing 49 changed files with 3,104 additions and 279 deletions.
17 changes: 11 additions & 6 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,8 @@ pub enum BorrowKind {

/// Data must be immutable but not aliasable. This kind of borrow
/// cannot currently be expressed by the user and is used only in
/// implicit closure bindings. It is needed when you the closure
/// is borrowing or mutating a mutable referent, e.g.:
/// implicit closure bindings. It is needed when the closure is
/// borrowing or mutating a mutable referent, e.g.:
///
/// let x: &mut isize = ...;
/// let y = || *x += 5;
Expand All @@ -443,7 +443,7 @@ pub enum BorrowKind {
/// let y = (&mut Env { &x }, fn_ptr); // Closure is pair of env and fn
/// fn fn_ptr(env: &mut Env) { **env.x += 5; }
///
/// This is then illegal because you cannot mutate a `&mut` found
/// This is then illegal because you cannot mutate an `&mut` found
/// in an aliasable location. To solve, you'd have to translate with
/// an `&mut` borrow:
///
Expand Down Expand Up @@ -523,6 +523,8 @@ pub struct VarBindingForm<'tcx> {
/// (b) it gives a way to separate this case from the remaining cases
/// for diagnostics.
pub opt_match_place: Option<(Option<Place<'tcx>>, Span)>,
/// Span of the pattern in which this variable was bound.
pub pat_span: Span,
}

#[derive(Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
Expand All @@ -540,7 +542,8 @@ CloneTypeFoldableAndLiftImpls! { BindingForm<'tcx>, }
impl_stable_hash_for!(struct self::VarBindingForm<'tcx> {
binding_mode,
opt_ty_info,
opt_match_place
opt_match_place,
pat_span
});

mod binding_form_impl {
Expand Down Expand Up @@ -673,7 +676,7 @@ pub struct LocalDecl<'tcx> {
/// ROOT SCOPE
/// │{ argument x: &str }
/// │
/// │ │{ #[allow(unused_mut] } // this is actually split into 2 scopes
/// │ │{ #[allow(unused_mut)] } // this is actually split into 2 scopes
/// │ │ // in practice because I'm lazy.
/// │ │
/// │ │← x.source_info.scope
Expand Down Expand Up @@ -710,6 +713,7 @@ impl<'tcx> LocalDecl<'tcx> {
binding_mode: ty::BindingMode::BindByValue(_),
opt_ty_info: _,
opt_match_place: _,
pat_span: _,
}))) => true,

// FIXME: might be able to thread the distinction between
Expand All @@ -729,6 +733,7 @@ impl<'tcx> LocalDecl<'tcx> {
binding_mode: ty::BindingMode::BindByValue(_),
opt_ty_info: _,
opt_match_place: _,
pat_span: _,
}))) => true,

Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf)) => true,
Expand Down Expand Up @@ -906,7 +911,7 @@ pub enum TerminatorKind<'tcx> {

/// Drop the Place and assign the new value over it. This ensures
/// that the assignment to `P` occurs *even if* the destructor for
/// place unwinds. Its semantics are best explained by by the
/// place unwinds. Its semantics are best explained by the
/// elaboration:
///
/// ```
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
ty::BindByReference(..) => {
let let_span = self.tcx.hir.span(node_id);
let suggestion = suggest_ref_mut(self.tcx, let_span);
if let Some((let_span, replace_str)) = suggestion {
if let Some(replace_str) = suggestion {
db.span_suggestion(
let_span,
"use a mutable reference instead",
Expand Down
Loading

0 comments on commit 142bb27

Please sign in to comment.