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

Add feature to enable bind by move pattern guards #54034

Merged
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
29 changes: 28 additions & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1436,10 +1436,37 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.queries.on_disk_cache.serialize(self.global_tcx(), encoder)
}

/// This checks whether one is allowed to have pattern bindings
/// that bind-by-move on a match arm that has a guard, e.g.:
///
/// ```rust
/// match foo { A(inner) if { /* something */ } => ..., ... }
/// ```
///
/// It is separate from check_for_mutation_in_guard_via_ast_walk,
/// because that method has a narrower effect that can be toggled
/// off via a separate `-Z` flag, at least for the short term.
pub fn allow_bind_by_move_patterns_with_guards(self) -> bool {
self.features().bind_by_move_pattern_guards && self.use_mir_borrowck()
}

/// If true, we should use a naive AST walk to determine if match
/// guard could perform bad mutations (or mutable-borrows).
pub fn check_for_mutation_in_guard_via_ast_walk(self) -> bool {
!self.sess.opts.debugging_opts.disable_ast_check_for_mutation_in_guard
// If someone passes the `-Z` flag, they're asking for the footgun.
if self.sess.opts.debugging_opts.disable_ast_check_for_mutation_in_guard {
return false;
}

// If someone requests the feature, then be a little more
// careful and ensure that MIR-borrowck is enabled (which can
// happen via edition selection, via `feature(nll)`, or via an
// appropriate `-Z` flag) before disabling the mutation check.
if self.allow_bind_by_move_patterns_with_guards() {
return false;
}

return true;
}

/// If true, we should use the AST-based borrowck (we may *also* use
Expand Down
28 changes: 19 additions & 9 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,11 +537,15 @@ fn check_legality_of_move_bindings(cx: &MatchVisitor,
"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 {
struct_span_err!(cx.tcx.sess, p.span, E0008,
"cannot bind by-move into a pattern guard")
.span_label(p.span, "moves value into pattern guard")
.emit();
} else if has_guard && !cx.tcx.allow_bind_by_move_patterns_with_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() && cx.tcx.use_mir_borrowck() {
err.help("add #![feature(bind_by_move_pattern_guards)] to the \
crate attributes to enable");
}
err.emit();
} else if let Some(by_ref_span) = by_ref_span {
struct_span_err!(
cx.tcx.sess,
Expand Down Expand Up @@ -613,10 +617,16 @@ impl<'a, 'tcx> Delegate<'tcx> for MutationChecker<'a, 'tcx> {
_: LoanCause) {
match kind {
ty::MutBorrow => {
struct_span_err!(self.cx.tcx.sess, span, E0301,
"cannot mutably borrow in a pattern guard")
.span_label(span, "borrowed mutably in pattern guard")
.emit();
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() &&
self.cx.tcx.use_mir_borrowck()
{
err.help("add #![feature(bind_by_move_pattern_guards)] to the \
crate attributes to enable");
}
err.emit();
}
ty::ImmBorrow | ty::UniqueImmBorrow => {}
}
Expand Down
6 changes: 6 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,12 @@ declare_features! (

// Self struct constructor (RFC 2302)
(active, self_struct_ctor, "1.31.0", Some(51994), None),

// allow mixing of bind-by-move in patterns and references to
// those identifiers in guards, *if* we are using MIR-borrowck
// (aka NLL). Essentially this means you need to be on
// edition:2018 or later.
(active, bind_by_move_pattern_guards, "1.30.0", Some(15287), None),
);

declare_features! (
Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/bind-by-move/bind-by-move-no-guards.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0008]: cannot bind by-move into a pattern guard
--> $DIR/bind-by-move-no-guards.rs:8:14
|
LL | Some(z) if z.recv().unwrap() => { panic!() },
| ^ moves value into pattern guard
|
= help: add #![feature(bind_by_move_pattern_guards)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0008`.
10 changes: 0 additions & 10 deletions src/test/ui/bind-by-move/bind-by-move-no-guards.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
// Copyright 2012 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::sync::mpsc::channel;

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/bind-by-move/bind-by-move-no-guards.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0008]: cannot bind by-move into a pattern guard
--> $DIR/bind-by-move-no-guards.rs:18:14
--> $DIR/bind-by-move-no-guards.rs:8:14
|
LL | Some(z) if z.recv().unwrap() => { panic!() },
| ^ moves value into pattern guard
Expand Down
24 changes: 24 additions & 0 deletions src/test/ui/borrowck/borrowck-mutate-in-guard.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
error[E0302]: cannot assign in a pattern guard
--> $DIR/borrowck-mutate-in-guard.rs:20:25
|
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:22: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:22:41
|
LL | Enum::A(_) if { let y = &mut x; *y = Enum::B(false); false } => 1,
| ^^^^^^^^^^^^^^^^^^^ assignment in pattern guard

error: aborting due to 3 previous errors

Some errors occurred: E0301, E0302.
For more information about an error, try `rustc --explain E0301`.
11 changes: 11 additions & 0 deletions src/test/ui/error-codes/E0008.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0008]: cannot bind by-move into a pattern guard
--> $DIR/E0008.rs:13:14
|
LL | Some(s) if s.len() == 0 => {},
| ^ moves value into pattern guard
|
= help: add #![feature(bind_by_move_pattern_guards)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0008`.
11 changes: 11 additions & 0 deletions src/test/ui/error-codes/E0301.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0301]: cannot mutably borrow in a pattern guard
--> $DIR/E0301.rs:14:19
|
LL | option if option.take().is_none() => {}, //~ ERROR E0301
| ^^^^^^ borrowed mutably in pattern guard
|
= help: add #![feature(bind_by_move_pattern_guards)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0301`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Adaptation of existing ui test (from way back in
// rust-lang/rust#2329), that starts passing with this feature in
// place.

// compile-pass

#![feature(nll)]
#![feature(bind_by_move_pattern_guards)]

use std::sync::mpsc::channel;

fn main() {
let (tx, rx) = channel();
let x = Some(rx);
tx.send(false);
match x {
Some(z) if z.recv().unwrap() => { panic!() },
Some(z) => { assert!(!z.recv().unwrap()); },
None => panic!()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0008]: cannot bind by-move into a pattern guard
--> $DIR/feature-gate.rs:33:16
|
LL | A { a: v } if *v == 42 => v,
| ^ moves value into pattern guard

error: aborting due to previous error

For more information about this error, try `rustc --explain E0008`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: compilation successful
--> $DIR/feature-gate.rs:42:1
|
LL | / fn main() {
LL | | foo(107)
LL | | }
| |_^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: compilation successful
--> $DIR/feature-gate.rs:42:1
|
LL | / fn main() {
LL | | foo(107)
LL | | }
| |_^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: compilation successful
--> $DIR/feature-gate.rs:42:1
|
LL | / fn main() {
LL | | foo(107)
LL | | }
| |_^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0008]: cannot bind by-move into a pattern guard
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I guess it would be nice (when on nightly and with MIR-borrowck enabled) if this issued a diagnostic suggesting the feature gate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(done, see commit c329897)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also be useful to suggest enabling NLL and the feature if not? Perhaps that's a little convoluted though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's necessary; the 2018 edition has NLL enabled anyway (under migrate mode), so I think that will be enough.

--> $DIR/feature-gate.rs:33:16
|
LL | A { a: v } if *v == 42 => v,
| ^ moves value into pattern guard

error: aborting due to previous error

For more information about this error, try `rustc --explain E0008`.
47 changes: 47 additions & 0 deletions src/test/ui/rfc-0107-bind-by-move-pattern-guards/feature-gate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Check that pattern-guards with move-bound variables is only allowed
// with the appropriate set of feature gates. (Note that we require
// the code to opt into MIR-borrowck in *some* way before the feature
// will work; we use the revision system here to enumerate a number of
// ways that opt-in could occur.)

// gate-test-bind_by_move_pattern_guards

// revisions: no_gate gate_and_2015 gate_and_2018 gate_and_znll gate_and_feature_nll

// (We're already testing NLL behavior quite explicitly, no need for compare-mode=nll.)
// ignore-compare-mode-nll

#![feature(rustc_attrs)]

#![cfg_attr(gate_and_2015, feature(bind_by_move_pattern_guards))]
#![cfg_attr(gate_and_2018, feature(bind_by_move_pattern_guards))]
#![cfg_attr(gate_and_znll, feature(bind_by_move_pattern_guards))]
#![cfg_attr(gate_and_feature_nll, feature(bind_by_move_pattern_guards))]

#![cfg_attr(gate_and_feature_nll, feature(nll))]

//[gate_and_2015] edition:2015
//[gate_and_2018] edition:2018
//[gate_and_znll] compile-flags: -Z borrowck=mir

struct A { a: Box<i32> }

fn foo(n: i32) {
let x = A { a: Box::new(n) };
let _y = match x {

A { a: v } if *v == 42 => v,
//[no_gate]~^ ERROR cannot bind by-move into a pattern guard
//[gate_and_2015]~^^ ERROR cannot bind by-move into a pattern guard

_ => Box::new(0)
};
}

#[rustc_error]
fn main() {
foo(107)
}
//[gate_and_2018]~^^^ ERROR compilation successful
//[gate_and_znll]~^^^^ ERROR compilation successful
//[gate_and_feature_nll]~^^^^^ ERROR compilation successful
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#![feature(nll)]
#![feature(bind_by_move_pattern_guards)]

// compile-pass

struct A { a: Box<i32> }

impl A {
fn get(&self) -> i32 { *self.a }
}

fn foo(n: i32) {
let x = A { a: Box::new(n) };
let y = match x {
A { a: v } if *v == 42 => v,
_ => Box::new(0),
};
}

fn bar(n: i32) {
let x = A { a: Box::new(n) };
let y = match x {
A { a: v } if x.get() == 42 => v,
_ => Box::new(0),
};
}

fn baz(n: i32) {
let x = A { a: Box::new(n) };
let y = match x {
A { a: v } if *v.clone() == 42 => v,
_ => Box::new(0),
};
}

fn main() {
foo(107);
bar(107);
baz(107);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![feature(nll)]
#![feature(bind_by_move_pattern_guards)]

enum VecWrapper { A(Vec<i32>) }

fn foo(x: VecWrapper) -> usize {
match x {
VecWrapper::A(v) if { drop(v); false } => 1,
//~^ ERROR cannot move out of borrowed content
VecWrapper::A(v) => v.len()
}
}

fn main() {
foo(VecWrapper::A(vec![107]));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0507]: cannot move out of borrowed content
--> $DIR/rfc-reject-double-move-across-arms.rs:8:36
|
LL | VecWrapper::A(v) if { drop(v); false } => 1,
| ^ cannot move out of borrowed content

error: aborting due to previous error

For more information about this error, try `rustc --explain E0507`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(nll)]
#![feature(bind_by_move_pattern_guards)]

struct A { a: Box<i32> }

fn foo(n: i32) {
let x = A { a: Box::new(n) };
let _y = match x {
A { a: v } if { drop(v); true } => v,
//~^ ERROR cannot move out of borrowed content
_ => Box::new(0),
};
}

fn main() {
foo(107);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0507]: cannot move out of borrowed content
--> $DIR/rfc-reject-double-move-in-first-arm.rs:9:30
|
LL | A { a: v } if { drop(v); true } => v,
| ^ cannot move out of borrowed content

error: aborting due to previous error

For more information about this error, try `rustc --explain E0507`.