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

Stabilize bind_by_move_pattern_guards in Rust 1.39.0 #63118

Merged
merged 7 commits into from
Sep 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
#![feature(link_llvm_intrinsics)]
#![feature(never_type)]
#![feature(nll)]
#![feature(bind_by_move_pattern_guards)]
#![cfg_attr(boostrap_stdarch_ignore_this, feature(bind_by_move_pattern_guards))]
#![feature(exhaustive_patterns)]
#![feature(no_core)]
#![feature(on_unimplemented)]
Expand Down
10 changes: 3 additions & 7 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1345,13 +1345,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// any, and then branches to the arm. Returns the block for the case where
/// the guard fails.
///
/// Note: we check earlier that if there is a guard, there cannot be move
/// bindings (unless feature(bind_by_move_pattern_guards) is used). This
/// isn't really important for the self-consistency of this fn, but the
/// reason for it should be clear: after we've done the assignments, if
/// there were move bindings, further tests would be a use-after-move.
/// bind_by_move_pattern_guards avoids this by only moving the binding once
/// the guard has evaluated to true (see below).
/// Note: we do not check earlier that if there is a guard,
/// there cannot be move bindings. We avoid a use-after-move by only
/// moving the binding once the guard has evaluated to true (see below).
fn bind_and_guard_matched_candidate<'pat>(
&mut self,
candidate: Candidate<'pat, 'tcx>,
Expand Down
85 changes: 7 additions & 78 deletions src/librustc_mir/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,81 +157,6 @@ match x {
See also the error E0303.
"##,

E0008: r##"
Centril marked this conversation as resolved.
Show resolved Hide resolved
Names bound in match arms retain their type in pattern guards. As such, if a
name is bound by move in a pattern, it should also be moved to wherever it is
referenced in the pattern guard code. Doing so however would prevent the name
from being available in the body of the match arm. Consider the following:

```compile_fail,E0008
match Some("hi".to_string()) {
Some(s) if s.len() == 0 => {}, // use s.
_ => {},
}
```

The variable `s` has type `String`, and its use in the guard is as a variable of
type `String`. The guard code effectively executes in a separate scope to the
body of the arm, so the value would be moved into this anonymous scope and
therefore becomes unavailable in the body of the arm.

The problem above can be solved by using the `ref` keyword.

```
match Some("hi".to_string()) {
Some(ref s) if s.len() == 0 => {},
_ => {},
}
```

Though this example seems innocuous and easy to solve, the problem becomes clear
when it encounters functions which consume the value:

```compile_fail,E0008
struct A{}

impl A {
fn consume(self) -> usize {
0
}
}

fn main() {
let a = Some(A{});
match a {
Some(y) if y.consume() > 0 => {}
_ => {}
}
}
```

In this situation, even the `ref` keyword cannot solve it, since borrowed
content cannot be moved. This problem cannot be solved generally. If the value
can be cloned, here is a not-so-specific solution:

```
#[derive(Clone)]
struct A{}

impl A {
fn consume(self) -> usize {
0
}
}

fn main() {
let a = Some(A{});
match a{
Some(ref y) if y.clone().consume() > 0 => {}
_ => {}
}
}
```

If the value will be consumed in the pattern guard, using its clone will not
move its ownership, so the code works.
"##,

E0009: r##"
In a pattern, all values that don't implement the `Copy` trait have to be bound
the same way. The goal here is to avoid binding simultaneously by-move and
Expand Down Expand Up @@ -475,13 +400,15 @@ for item in xs {
"##,

E0301: r##"
#### Note: this error code is no longer emitted by the compiler.

Mutable borrows are not allowed in pattern guards, because matching cannot have
side effects. Side effects could alter the matched object or the environment
on which the match depends in such a way, that the match would not be
exhaustive. For instance, the following would not match any arm if mutable
borrows were allowed:

```compile_fail,E0301
```compile_fail,E0596
match Some(()) {
None => { },
option if option.take().is_none() => {
Expand All @@ -493,13 +420,15 @@ match Some(()) {
"##,

E0302: r##"
#### Note: this error code is no longer emitted by the compiler.

Assignments are not allowed in pattern guards, because matching cannot have
side effects. Side effects could alter the matched object or the environment
on which the match depends in such a way, that the match would not be
exhaustive. For instance, the following would not match any arm if assignments
were allowed:

```compile_fail,E0302
```compile_fail,E0594
match Some(()) {
None => { },
option if { option = None; false } => { },
Expand Down Expand Up @@ -1989,7 +1918,6 @@ 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(bind_by_move_pattern_guards)]
let mut x = Some(0);
match x {
None => (),
Expand Down Expand Up @@ -2451,6 +2379,7 @@ There are some known bugs that trigger this message.

;

// E0008, // cannot bind by-move into a pattern guard
// E0298, // cannot compare constants
// E0299, // mismatched types between arms
// E0471, // constant evaluation error (in pattern)
Expand Down
90 changes: 4 additions & 86 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@ 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;
use rustc::middle::mem_categorization::cmt_;
use rustc::middle::region;
use rustc::session::Session;
use rustc::ty::{self, Ty, TyCtxt};
use rustc::ty::subst::{InternalSubsts, SubstsRef};
Expand All @@ -36,9 +31,7 @@ crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) -> SignalledError {

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),
signalled_error: SignalledError::NoErrorsSeen,
Expand All @@ -53,11 +46,9 @@ fn create_e0004(sess: &Session, sp: Span, error_message: String) -> DiagnosticBu

struct MatchVisitor<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
body_owner: DefId,
tables: &'a ty::TypeckTables<'tcx>,
param_env: ty::ParamEnv<'tcx>,
identity_substs: SubstsRef<'tcx>,
region_scope_tree: &'a region::ScopeTree,
signalled_error: SignalledError,
}

Expand Down Expand Up @@ -151,11 +142,8 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {

// 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 {
if arm.guard.is_some() {
self.signalled_error = SignalledError::SawSomeError;
if !self.tcx.features().bind_by_move_pattern_guards {
check_for_mutation_in_guard(self, &guard);
}
}

// Third, perform some lints.
Expand Down Expand Up @@ -582,19 +570,10 @@ 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 {
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();
} else if !has_guard {
if let Some(_by_ref_span) = by_ref_span {
span_vec.push(p.span);
}
} else if let Some(_by_ref_span) = by_ref_span {
span_vec.push(p.span);
}
};

Expand Down Expand Up @@ -636,67 +615,6 @@ fn check_legality_of_move_bindings(
}
}

/// Ensures that a pattern guard doesn't borrow by mutable reference or assign.
//
// FIXME: this should be done by borrowck.
fn check_for_mutation_in_guard(cx: &MatchVisitor<'_, '_>, guard: &hir::Guard) {
let mut checker = MutationChecker {
cx,
};
match guard {
hir::Guard::If(expr) =>
ExprUseVisitor::new(&mut checker,
cx.tcx,
cx.body_owner,
cx.param_env,
cx.region_scope_tree,
cx.tables,
None).walk_expr(expr),
};
}

struct MutationChecker<'a, 'tcx> {
cx: &'a MatchVisitor<'a, 'tcx>,
}

impl<'a, 'tcx> Delegate<'tcx> for MutationChecker<'a, 'tcx> {
fn matched_pat(&mut self, _: &Pat, _: &cmt_<'_>, _: euv::MatchMode) {}
fn consume(&mut self, _: hir::HirId, _: Span, _: &cmt_<'_>, _: ConsumeMode) {}
fn consume_pat(&mut self, _: &Pat, _: &cmt_<'_>, _: ConsumeMode) {}
fn borrow(&mut self,
_: hir::HirId,
span: Span,
_: &cmt_<'_>,
_: ty::Region<'tcx>,
kind:ty:: BorrowKind,
_: LoanCause) {
match kind {
ty::MutBorrow => {
let mut err = struct_span_err!(self.cx.tcx.sess, span, E0301,
"cannot mutably borrow in a pattern guard");
err.span_label(span, "borrowed mutably in pattern guard");
if self.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();
}
ty::ImmBorrow | ty::UniqueImmBorrow => {}
}
}
fn decl_without_init(&mut self, _: hir::HirId, _: Span) {}
fn mutate(&mut self, _: hir::HirId, span: Span, _: &cmt_<'_>, mode: MutateMode) {
match mode {
MutateMode::JustWrite | MutateMode::WriteAndRead => {
struct_span_err!(self.cx.tcx.sess, span, E0302, "cannot assign in a pattern guard")
.span_label(span, "assignment in pattern guard")
.emit();
}
MutateMode::Init => {}
}
}
}

/// Forbids bindings in `@` patterns. This is necessary for memory safety,
/// because of the way rvalues are handled in the borrow check. (See issue
/// #14587.)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_passes/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

#![feature(in_band_lifetimes)]
#![feature(nll)]
#![feature(bind_by_move_pattern_guards)]
#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]

#![recursion_limit="256"]

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/",
html_playground_url = "https://play.rust-lang.org/")]

#![feature(bind_by_move_pattern_guards)]
#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]
#![feature(rustc_private)]
#![feature(arbitrary_self_types)]
#![feature(box_patterns)]
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@
#![feature(array_error_internals)]
#![feature(asm)]
#![feature(associated_type_bounds)]
#![feature(bind_by_move_pattern_guards)]
#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]
#![feature(box_syntax)]
#![feature(c_variadic)]
#![feature(cfg_target_has_atomic)]
Expand Down
2 changes: 2 additions & 0 deletions src/libsyntax/feature_gate/accepted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ declare_features! (
(accepted, underscore_const_names, "1.37.0", Some(54912), None),
/// Allows free and inherent `async fn`s, `async` blocks, and `<expr>.await` expressions.
(accepted, async_await, "1.39.0", Some(50547), None),
/// Allows mixing bind-by-move in patterns and references to those identifiers in guards.
(accepted, bind_by_move_pattern_guards, "1.39.0", Some(15287), None),

// -------------------------------------------------------------------------
// feature-group-end: accepted features
Expand Down
3 changes: 0 additions & 3 deletions src/libsyntax/feature_gate/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,6 @@ declare_features! (
/// Allows non-builtin attributes in inner attribute position.
(active, custom_inner_attributes, "1.30.0", Some(54726), None),

/// Allows mixing bind-by-move in patterns and references to those identifiers in guards.
(active, bind_by_move_pattern_guards, "1.30.0", Some(15287), None),

/// Allows `impl Trait` in bindings (`let`, `const`, `static`).
(active, impl_trait_in_bindings, "1.30.0", Some(63065), None),

Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/",
test(attr(deny(warnings))))]

#![feature(bind_by_move_pattern_guards)]
#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]
#![feature(box_syntax)]
#![feature(const_fn)]
#![feature(const_transmute)]
Expand Down
2 changes: 0 additions & 2 deletions src/test/mir-opt/match-arm-scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// all of the bindings for that scope.
// * No drop flags are used.

#![feature(nll, bind_by_move_pattern_guards)]

fn complicated_match(cond: bool, items: (bool, bool, String)) -> i32 {
match items {
(false, a, s) | (a, false, s) if if cond { return 3 } else { a } => 1,
Expand Down
13 changes: 0 additions & 13 deletions src/test/ui/bind-by-move/bind-by-move-no-guards.rs

This file was deleted.

11 changes: 0 additions & 11 deletions src/test/ui/bind-by-move/bind-by-move-no-guards.stderr

This file was deleted.

Loading