Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make #![feature(bind_by_move_pattern_guards)] sound without #[feature(nll)] #63059

Merged
merged 9 commits into from
Aug 3, 2019
2 changes: 1 addition & 1 deletion src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ rustc_queries! {
}

TypeChecking {
query check_match(key: DefId) -> () {
query check_match(key: DefId) -> SignalledError {
cache_on_disk_if { key.is_local() }
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::hir::def::{DefKind, Export};
use crate::hir::{self, TraitCandidate, ItemLocalId, CodegenFnAttrs};
use crate::infer::canonical::{self, Canonical};
use crate::lint;
use crate::middle::borrowck::BorrowCheckResult;
use crate::middle::borrowck::{BorrowCheckResult, SignalledError};
use crate::middle::cstore::{ExternCrate, LinkagePreference, NativeLibrary, ForeignModule};
use crate::middle::cstore::{NativeLibraryKind, DepKind, CrateSource};
use crate::middle::privacy::AccessLevels;
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_ast_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ fn borrowck(tcx: TyCtxt<'_>, owner_def_id: DefId) -> &BorrowCheckResult {

debug!("borrowck(body_owner_def_id={:?})", owner_def_id);

let signalled_error = tcx.check_match(owner_def_id);
if let SignalledError::SawSomeError = signalled_error {
return tcx.arena.alloc(BorrowCheckResult {
signalled_any_error: SignalledError::SawSomeError,
})
}

let owner_id = tcx.hir().as_local_hir_id(owner_def_id).unwrap();

match tcx.hir().get(owner_id) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1989,7 +1989,7 @@ When matching on a variable it cannot be mutated in the match guards, as this
could cause the match to be non-exhaustive:

```compile_fail,E0510
#![feature(nll, bind_by_move_pattern_guards)]
#![feature(bind_by_move_pattern_guards)]
let mut x = Some(0);
match x {
None => (),
Expand Down
58 changes: 34 additions & 24 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use super::_match::WitnessPreference::*;

use super::{Pattern, PatternContext, PatternError, PatternKind};

use rustc::middle::borrowck::SignalledError;
use rustc::middle::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor};
use rustc::middle::expr_use_visitor::{LoanCause, MutateMode};
use rustc::middle::expr_use_visitor as euv;
Expand All @@ -26,21 +27,24 @@ use std::slice;

use syntax_pos::{Span, DUMMY_SP, MultiSpan};

pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: DefId) {
crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) -> SignalledError {
let body_id = if let Some(id) = tcx.hir().as_local_hir_id(def_id) {
tcx.hir().body_owned_by(id)
} else {
return;
return SignalledError::NoErrorsSeen;
};

MatchVisitor {
let mut visitor = MatchVisitor {
tcx,
body_owner: def_id,
tables: tcx.body_tables(body_id),
region_scope_tree: &tcx.region_scope_tree(def_id),
param_env: tcx.param_env(def_id),
identity_substs: InternalSubsts::identity_for_item(tcx, def_id),
}.visit_body(tcx.hir().body(body_id));
signalled_error: SignalledError::NoErrorsSeen,
};
visitor.visit_body(tcx.hir().body(body_id));
visitor.signalled_error
}

fn create_e0004(sess: &Session, sp: Span, error_message: String) -> DiagnosticBuilder<'_> {
Expand All @@ -54,6 +58,7 @@ struct MatchVisitor<'a, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
identity_substs: SubstsRef<'tcx>,
region_scope_tree: &'a region::ScopeTree,
signalled_error: SignalledError,
}

impl<'a, 'tcx> Visitor<'tcx> for MatchVisitor<'a, 'tcx> {
Expand All @@ -64,11 +69,8 @@ impl<'a, 'tcx> Visitor<'tcx> for MatchVisitor<'a, 'tcx> {
fn visit_expr(&mut self, ex: &'tcx hir::Expr) {
intravisit::walk_expr(self, ex);

match ex.node {
hir::ExprKind::Match(ref scrut, ref arms, source) => {
self.check_match(scrut, arms, source);
}
_ => {}
if let hir::ExprKind::Match(ref scrut, ref arms, source) = ex.node {
self.check_match(scrut, arms, source);
}
}

Expand Down Expand Up @@ -130,26 +132,27 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
}

impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
fn check_patterns(&self, has_guard: bool, pats: &[P<Pat>]) {
fn check_patterns(&mut self, has_guard: bool, pats: &[P<Pat>]) {
check_legality_of_move_bindings(self, has_guard, pats);
for pat in pats {
check_legality_of_bindings_in_at_patterns(self, pat);
}
}

fn check_match(
&self,
&mut self,
scrut: &hir::Expr,
arms: &'tcx [hir::Arm],
source: hir::MatchSource)
{
source: hir::MatchSource
) {
for arm in arms {
// First, check legality of move bindings.
self.check_patterns(arm.guard.is_some(), &arm.pats);

// Second, if there is a guard on each arm, make sure it isn't
// assigning or borrowing anything mutably.
if let Some(ref guard) = arm.guard {
self.signalled_error = SignalledError::SawSomeError;
Centril marked this conversation as resolved.
Show resolved Hide resolved
if !self.tcx.features().bind_by_move_pattern_guards {
check_for_mutation_in_guard(self, &guard);
}
Expand Down Expand Up @@ -548,7 +551,7 @@ fn maybe_point_at_variant(

// Legality of move bindings checking
fn check_legality_of_move_bindings(
cx: &MatchVisitor<'_, '_>,
cx: &mut MatchVisitor<'_, '_>,
has_guard: bool,
pats: &[P<Pat>],
) {
Expand All @@ -565,7 +568,12 @@ fn check_legality_of_move_bindings(
})
}
let span_vec = &mut Vec::new();
let check_move = |p: &Pat, sub: Option<&Pat>, span_vec: &mut Vec<Span>| {
let check_move = |
cx: &mut MatchVisitor<'_, '_>,
p: &Pat,
sub: Option<&Pat>,
span_vec: &mut Vec<Span>,
| {
// check legality of moving out of the enum

// x @ Foo(..) is legal, but x @ Foo(y) isn't.
Expand All @@ -574,15 +582,17 @@ fn check_legality_of_move_bindings(
"cannot bind by-move with sub-bindings")
.span_label(p.span, "binds an already bound by-move value by moving it")
.emit();
} else if has_guard && !cx.tcx.features().bind_by_move_pattern_guards {
let mut err = struct_span_err!(cx.tcx.sess, p.span, E0008,
"cannot bind by-move into a pattern guard");
err.span_label(p.span, "moves value into pattern guard");
if cx.tcx.sess.opts.unstable_features.is_nightly_build() {
err.help("add `#![feature(bind_by_move_pattern_guards)]` to the \
crate attributes to enable");
} else if has_guard {
if !cx.tcx.features().bind_by_move_pattern_guards {
let mut err = struct_span_err!(cx.tcx.sess, p.span, E0008,
"cannot bind by-move into a pattern guard");
err.span_label(p.span, "moves value into pattern guard");
if cx.tcx.sess.opts.unstable_features.is_nightly_build() {
err.help("add `#![feature(bind_by_move_pattern_guards)]` to the \
crate attributes to enable");
}
err.emit();
}
err.emit();
} else if let Some(_by_ref_span) = by_ref_span {
span_vec.push(p.span);
}
Expand All @@ -596,7 +606,7 @@ fn check_legality_of_move_bindings(
ty::BindByValue(..) => {
let pat_ty = cx.tables.node_type(p.hir_id);
if !pat_ty.is_copy_modulo_regions(cx.tcx, cx.param_env, pat.span) {
check_move(p, sub.as_ref().map(|p| &**p), span_vec);
check_move(cx, p, sub.as_ref().map(|p| &**p), span_vec);
}
}
_ => {}
Expand Down
18 changes: 9 additions & 9 deletions src/test/ui/borrowck/borrowck-migrate-to-nll.edition.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
warning[E0507]: cannot move out of `foo` in pattern guard
--> $DIR/borrowck-migrate-to-nll.rs:26:18
warning[E0502]: cannot borrow `*block.current` as immutable because it is also borrowed as mutable
--> $DIR/borrowck-migrate-to-nll.rs:28:21
|
LL | (|| { let bar = foo; bar.take() })();
| ^^ ---
| | |
| | move occurs because `foo` has type `&mut std::option::Option<&i32>`, which does not implement the `Copy` trait
| | move occurs due to use in closure
| move out of `foo` occurs here
LL | let x = &mut block;
| ---------- mutable borrow occurs here
LL | let p: &'a u8 = &*block.current;
| ^^^^^^^^^^^^^^^ immutable borrow occurs here
LL | // (use `x` and `p` so enabling NLL doesn't assign overly short lifetimes)
LL | drop(x);
| - mutable borrow later used here
|
= note: variables bound in patterns cannot be moved from until after the end of the pattern guard
= warning: this error has been downgraded to a warning for backwards compatibility with previous releases
= warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future
= note: for more information, try `rustc --explain E0729`
Expand Down
26 changes: 14 additions & 12 deletions src/test/ui/borrowck/borrowck-migrate-to-nll.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// This is a test of the borrowck migrate mode. It leverages #27282, a
// This is a test of the borrowck migrate mode. It leverages #38899, a
// bug that is fixed by NLL: this code is (unsoundly) accepted by
// AST-borrowck, but is correctly rejected by the NLL borrowck.
//
Expand All @@ -18,15 +18,17 @@
//[zflag] run-pass
//[edition] run-pass

fn main() {
match Some(&4) {
None => {},
ref mut foo
if {
(|| { let bar = foo; bar.take() })();
false
} => {},
Some(ref _s) => println!("Note this arm is bogus; the `Some` became `None` in the guard."),
_ => println!("Here is some supposedly unreachable code."),
}
pub struct Block<'a> {
current: &'a u8,
unrelated: &'a u8,
}

fn bump<'a>(mut block: &mut Block<'a>) {
let x = &mut block;
let p: &'a u8 = &*block.current;
// (use `x` and `p` so enabling NLL doesn't assign overly short lifetimes)
drop(x);
drop(p);
}

fn main() {}
18 changes: 9 additions & 9 deletions src/test/ui/borrowck/borrowck-migrate-to-nll.zflag.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
warning[E0507]: cannot move out of `foo` in pattern guard
--> $DIR/borrowck-migrate-to-nll.rs:26:18
warning[E0502]: cannot borrow `*block.current` as immutable because it is also borrowed as mutable
--> $DIR/borrowck-migrate-to-nll.rs:28:21
|
LL | (|| { let bar = foo; bar.take() })();
| ^^ ---
| | |
| | move occurs because `foo` has type `&mut std::option::Option<&i32>`, which does not implement the `Copy` trait
| | move occurs due to use in closure
| move out of `foo` occurs here
LL | let x = &mut block;
| ---------- mutable borrow occurs here
LL | let p: &'a u8 = &*block.current;
| ^^^^^^^^^^^^^^^ immutable borrow occurs here
LL | // (use `x` and `p` so enabling NLL doesn't assign overly short lifetimes)
LL | drop(x);
| - mutable borrow later used here
|
= note: variables bound in patterns cannot be moved from until after the end of the pattern guard
= warning: this error has been downgraded to a warning for backwards compatibility with previous releases
= warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future
= note: for more information, try `rustc --explain E0729`
Expand Down
41 changes: 0 additions & 41 deletions src/test/ui/borrowck/borrowck-mutate-in-guard.nll.stderr

This file was deleted.

8 changes: 2 additions & 6 deletions src/test/ui/borrowck/borrowck-mutate-in-guard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,11 @@ fn foo() -> isize {
match x {
Enum::A(_) if { x = Enum::B(false); false } => 1,
//~^ ERROR cannot assign in a pattern guard
//~| WARN cannot assign `x` in match guard
//~| WARN this error has been downgraded to a warning for backwards compatibility
//~| WARN this represents potential undefined behavior in your code and this warning will
//~| ERROR cannot assign `x` in match guard
Enum::A(_) if { let y = &mut x; *y = Enum::B(false); false } => 1,
//~^ ERROR cannot mutably borrow in a pattern guard
//~| ERROR cannot assign in a pattern guard
//~| WARN cannot mutably borrow `x` in match guard
//~| WARN this error has been downgraded to a warning for backwards compatibility
//~| WARN this represents potential undefined behavior in your code and this warning will
//~| ERROR cannot mutably borrow `x` in match guard
Enum::A(p) => *p,
Enum::B(_) => 2,
}
Expand Down
20 changes: 6 additions & 14 deletions src/test/ui/borrowck/borrowck-mutate-in-guard.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,37 @@ LL | Enum::A(_) if { x = Enum::B(false); false } => 1,
| ^^^^^^^^^^^^^^^^^^ assignment in pattern guard

error[E0301]: cannot mutably borrow in a pattern guard
--> $DIR/borrowck-mutate-in-guard.rs:15:38
--> $DIR/borrowck-mutate-in-guard.rs:13:38
|
LL | Enum::A(_) if { let y = &mut x; *y = Enum::B(false); false } => 1,
| ^ borrowed mutably in pattern guard
|
= help: add `#![feature(bind_by_move_pattern_guards)]` to the crate attributes to enable

error[E0302]: cannot assign in a pattern guard
--> $DIR/borrowck-mutate-in-guard.rs:15:41
--> $DIR/borrowck-mutate-in-guard.rs:13:41
|
LL | Enum::A(_) if { let y = &mut x; *y = Enum::B(false); false } => 1,
| ^^^^^^^^^^^^^^^^^^^ assignment in pattern guard

warning[E0510]: cannot assign `x` in match guard
error[E0510]: cannot assign `x` in match guard
--> $DIR/borrowck-mutate-in-guard.rs:10:25
|
LL | match x {
| - value is immutable in match guard
LL | Enum::A(_) if { x = Enum::B(false); false } => 1,
| ^^^^^^^^^^^^^^^^^^ cannot assign
|
= warning: this error has been downgraded to a warning for backwards compatibility with previous releases
= warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future
= note: for more information, try `rustc --explain E0729`

warning[E0510]: cannot mutably borrow `x` in match guard
--> $DIR/borrowck-mutate-in-guard.rs:15:33
error[E0510]: cannot mutably borrow `x` in match guard
--> $DIR/borrowck-mutate-in-guard.rs:13:33
|
LL | match x {
| - value is immutable in match guard
...
LL | Enum::A(_) if { let y = &mut x; *y = Enum::B(false); false } => 1,
| ^^^^^^ cannot mutably borrow
|
= warning: this error has been downgraded to a warning for backwards compatibility with previous releases
= warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future
= note: for more information, try `rustc --explain E0729`

error: aborting due to 3 previous errors
error: aborting due to 5 previous errors

Some errors have detailed explanations: E0301, E0302, E0510.
For more information about an error, try `rustc --explain E0301`.
13 changes: 13 additions & 0 deletions src/test/ui/borrowck/issue-27282-mutation-in-guard.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn main() {
match Some(&4) {
None => {},
ref mut foo
if {
(|| { let bar = foo; bar.take() })();
//~^ ERROR cannot move out of `foo` in pattern guard
false
} => {},
Some(ref _s) => println!("Note this arm is bogus; the `Some` became `None` in the guard."),
_ => println!("Here is some supposedly unreachable code."),
}
}
Loading