Skip to content

Commit 7af9ff3

Browse files
committed
introduce #![feature(move_ref_pattern)]
1 parent cef6894 commit 7af9ff3

File tree

4 files changed

+125
-73
lines changed

4 files changed

+125
-73
lines changed

src/librustc_errors/diagnostic_builder.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,25 @@ impl<'a> DiagnosticBuilder<'a> {
186186
/// all, and you just supplied a `Span` to create the diagnostic,
187187
/// then the snippet will just include that `Span`, which is
188188
/// called the primary span.
189-
pub fn span_label<T: Into<String>>(&mut self, span: Span, label: T) -> &mut Self {
189+
pub fn span_label(&mut self, span: Span, label: impl Into<String>) -> &mut Self {
190190
self.0.diagnostic.span_label(span, label);
191191
self
192192
}
193193

194+
/// Labels all the given spans with the provided label.
195+
/// See `span_label` for more information.
196+
pub fn span_labels(
197+
&mut self,
198+
spans: impl IntoIterator<Item = Span>,
199+
label: impl AsRef<str>,
200+
) -> &mut Self {
201+
let label = label.as_ref();
202+
for span in spans {
203+
self.0.diagnostic.span_label(span, label);
204+
}
205+
self
206+
}
207+
194208
forward!(pub fn note_expected_found(
195209
&mut self,
196210
expected_label: &dyn fmt::Display,

src/librustc_feature/active.rs

+4
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,10 @@ declare_features! (
538538
/// For example, you can write `x @ Some(y)`.
539539
(active, bindings_after_at, "1.41.0", Some(65490), None),
540540

541+
/// Allows patterns with concurrent by-move and by-ref bindings.
542+
/// For example, you can write `Foo(a, ref b)` where `a` is by-move and `b` is by-ref.
543+
(active, move_ref_pattern, "1.42.0", Some(68354), None),
544+
541545
/// Allows `impl const Trait for T` syntax.
542546
(active, const_trait_impl, "1.42.0", Some(67792), None),
543547

src/librustc_mir_build/hair/pattern/check_match.rs

+105-72
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ use rustc_session::lint::builtin::BINDINGS_WITH_VARIANT_NAME;
1616
use rustc_session::lint::builtin::{IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS};
1717
use rustc_session::parse::feature_err;
1818
use rustc_session::Session;
19-
use rustc_span::symbol::sym;
20-
use rustc_span::{MultiSpan, Span};
19+
use rustc_span::{sym, Span};
2120
use syntax::ast::Mutability;
2221

2322
use std::slice;
@@ -114,8 +113,10 @@ impl PatCtxt<'_, '_> {
114113

115114
impl<'tcx> MatchVisitor<'_, 'tcx> {
116115
fn check_patterns(&mut self, has_guard: bool, pat: &Pat<'_>) {
117-
check_legality_of_move_bindings(self, has_guard, pat);
118-
check_borrow_conflicts_in_at_patterns(self, pat);
116+
if !self.tcx.features().move_ref_pattern {
117+
check_legality_of_move_bindings(self, has_guard, pat);
118+
}
119+
pat.walk_always(|pat| check_borrow_conflicts_in_at_patterns(self, pat));
119120
if !self.tcx.features().bindings_after_at {
120121
check_legality_of_bindings_in_at_patterns(self, pat);
121122
}
@@ -559,6 +560,11 @@ fn maybe_point_at_variant(ty: Ty<'_>, patterns: &[super::Pat<'_>]) -> Vec<Span>
559560
covered
560561
}
561562

563+
/// Check if a by-value binding is by-value. That is, check if the binding's type is not `Copy`.
564+
fn is_binding_by_move(cx: &MatchVisitor<'_, '_>, hir_id: HirId, span: Span) -> bool {
565+
!cx.tables.node_type(hir_id).is_copy_modulo_regions(cx.tcx, cx.param_env, span)
566+
}
567+
562568
/// Check the legality of legality of by-move bindings.
563569
fn check_legality_of_move_bindings(cx: &mut MatchVisitor<'_, '_>, has_guard: bool, pat: &Pat<'_>) {
564570
let sess = cx.tcx.sess;
@@ -589,8 +595,7 @@ fn check_legality_of_move_bindings(cx: &mut MatchVisitor<'_, '_>, has_guard: boo
589595
pat.walk_always(|p| {
590596
if let hir::PatKind::Binding(.., sub) = &p.kind {
591597
if let Some(ty::BindByValue(_)) = tables.extract_binding_mode(sess, p.hir_id, p.span) {
592-
let pat_ty = tables.node_type(p.hir_id);
593-
if !pat_ty.is_copy_modulo_regions(cx.tcx, cx.param_env, pat.span) {
598+
if is_binding_by_move(cx, p.hir_id, p.span) {
594599
check_move(p, sub.as_deref());
595600
}
596601
}
@@ -599,11 +604,11 @@ fn check_legality_of_move_bindings(cx: &mut MatchVisitor<'_, '_>, has_guard: boo
599604

600605
// Found some bad by-move spans, error!
601606
if !by_move_spans.is_empty() {
602-
let mut err = struct_span_err!(
603-
sess,
604-
MultiSpan::from_spans(by_move_spans.clone()),
605-
E0009,
606-
"cannot bind by-move and by-ref in the same pattern",
607+
let mut err = feature_err(
608+
&sess.parse_sess,
609+
sym::move_ref_pattern,
610+
by_move_spans.clone(),
611+
"binding by-move and by-ref in the same pattern is unstable",
607612
);
608613
for span in by_ref_spans.iter() {
609614
err.span_label(*span, "by-ref pattern here");
@@ -615,81 +620,109 @@ fn check_legality_of_move_bindings(cx: &mut MatchVisitor<'_, '_>, has_guard: boo
615620
}
616621
}
617622

618-
/// Check that there are no borrow conflicts in `binding @ subpat` patterns.
623+
/// Check that there are no borrow or move conflicts in `binding @ subpat` patterns.
619624
///
620625
/// For example, this would reject:
621626
/// - `ref x @ Some(ref mut y)`,
622-
/// - `ref mut x @ Some(ref y)`
623-
/// - `ref mut x @ Some(ref mut y)`.
627+
/// - `ref mut x @ Some(ref y)`,
628+
/// - `ref mut x @ Some(ref mut y)`,
629+
/// - `ref mut? x @ Some(y)`, and
630+
/// - `x @ Some(ref mut? y)`.
624631
///
625632
/// This analysis is *not* subsumed by NLL.
626633
fn check_borrow_conflicts_in_at_patterns(cx: &MatchVisitor<'_, '_>, pat: &Pat<'_>) {
627-
let tab = cx.tables;
628-
let sess = cx.tcx.sess;
629-
// Get the mutability of `p` if it's by-ref.
630-
let extract_binding_mut = |hir_id, span| match tab.extract_binding_mode(sess, hir_id, span)? {
631-
ty::BindByValue(_) => None,
632-
ty::BindByReference(m) => Some(m),
634+
// Extract `sub` in `binding @ sub`.
635+
let (name, sub) = match &pat.kind {
636+
hir::PatKind::Binding(.., name, Some(sub)) => (*name, sub),
637+
_ => return,
633638
};
634-
pat.walk_always(|pat| {
635-
// Extract `sub` in `binding @ sub`.
636-
let (name, sub) = match &pat.kind {
637-
hir::PatKind::Binding(.., name, Some(sub)) => (*name, sub),
638-
_ => return,
639-
};
639+
let binding_span = pat.span.with_hi(name.span.hi());
640640

641-
// Extract the mutability.
642-
let mut_outer = match extract_binding_mut(pat.hir_id, pat.span) {
643-
None => return,
644-
Some(m) => m,
645-
};
641+
let tables = cx.tables;
642+
let sess = cx.tcx.sess;
646643

647-
// We now have `ref $mut_outer binding @ sub` (semantically).
648-
// Recurse into each binding in `sub` and find mutability conflicts.
649-
let mut conflicts_mut_mut = Vec::new();
650-
let mut conflicts_mut_ref = Vec::new();
651-
sub.each_binding(|_, hir_id, span, _| {
652-
if let Some(mut_inner) = extract_binding_mut(hir_id, span) {
653-
match (mut_outer, mut_inner) {
654-
(Mutability::Not, Mutability::Not) => {}
655-
(Mutability::Mut, Mutability::Mut) => conflicts_mut_mut.push(span),
656-
_ => conflicts_mut_ref.push(span),
644+
// Get the binding move, extract the mutability if by-ref.
645+
let mut_outer = match tables.extract_binding_mode(sess, pat.hir_id, pat.span) {
646+
Some(ty::BindByValue(_)) if is_binding_by_move(cx, pat.hir_id, pat.span) => {
647+
// We have `x @ pat` where `x` is by-move. Reject all borrows in `pat`.
648+
let mut conflicts_ref = Vec::new();
649+
sub.each_binding(|_, hir_id, span, _| {
650+
match tables.extract_binding_mode(sess, hir_id, span) {
651+
Some(ty::BindByValue(_)) if is_binding_by_move(cx, hir_id, span) => {
652+
sess.delay_span_bug(span, "by-move in subpat unchecked by borrowck");
653+
}
654+
Some(ty::BindByValue(_)) | None => {}
655+
Some(ty::BindByReference(_)) => conflicts_ref.push(span),
657656
}
657+
});
658+
if !conflicts_ref.is_empty() {
659+
let occurs_because = format!(
660+
"move occurs because `{}` has type `{}` which does implement the `Copy` trait",
661+
name,
662+
tables.node_type(pat.hir_id),
663+
);
664+
sess.struct_span_err(pat.span, &format!("borrow of moved value: `{}`", name))
665+
.span_label(binding_span, "value moved here")
666+
.span_label(binding_span, occurs_because)
667+
.span_labels(conflicts_ref, "value borrowed here after move")
668+
.emit();
658669
}
659-
});
670+
return;
671+
}
672+
Some(ty::BindByValue(_)) | None => return,
673+
Some(ty::BindByReference(m)) => m,
674+
};
660675

661-
// Report errors if any.
662-
let binding_span = pat.span.with_hi(name.span.hi());
663-
if !conflicts_mut_mut.is_empty() {
664-
// Report mutability conflicts for e.g. `ref mut x @ Some(ref mut y)`.
665-
let msg = &format!("cannot borrow `{}` as mutable more than once at a time", name);
666-
let mut err = sess.struct_span_err(pat.span, msg);
667-
err.span_label(binding_span, "first mutable borrow occurs here");
668-
for sp in conflicts_mut_mut {
669-
err.span_label(sp, "another mutable borrow occurs here");
670-
}
671-
for sp in conflicts_mut_ref {
672-
err.span_label(sp, "also borrowed as immutable here");
673-
}
674-
err.emit();
675-
} else if !conflicts_mut_ref.is_empty() {
676-
// Report mutability conflicts for e.g. `ref x @ Some(ref mut y)` or the converse.
677-
let (primary, also) = match mut_outer {
678-
Mutability::Mut => ("mutable", "immutable"),
679-
Mutability::Not => ("immutable", "mutable"),
680-
};
681-
let msg = &format!(
682-
"cannot borrow `{}` as {} because it is also borrowed as {}",
683-
name, also, primary,
684-
);
685-
let mut err = sess.struct_span_err(pat.span, msg);
686-
err.span_label(binding_span, &format!("{} borrow occurs here", primary));
687-
for sp in conflicts_mut_ref {
688-
err.span_label(sp, &format!("{} borrow occurs here", also));
689-
}
690-
err.emit();
676+
// We now have `ref $mut_outer binding @ sub` (semantically).
677+
// Recurse into each binding in `sub` and find mutability or move conflicts.
678+
let mut conflicts_move = Vec::new();
679+
let mut conflicts_mut_mut = Vec::new();
680+
let mut conflicts_mut_ref = Vec::new();
681+
sub.each_binding(|_, hir_id, span, _| match tables.extract_binding_mode(sess, hir_id, span) {
682+
Some(ty::BindByReference(mut_inner)) => match (mut_outer, mut_inner) {
683+
(Mutability::Not, Mutability::Not) => {} // Both sides are `ref`.
684+
(Mutability::Mut, Mutability::Mut) => conflicts_mut_mut.push(span), // 2x `ref mut`.
685+
_ => conflicts_mut_ref.push(span), // `ref` + `ref mut` in either direction.
686+
},
687+
Some(ty::BindByValue(_)) if is_binding_by_move(cx, hir_id, span) => {
688+
conflicts_move.push(span) // `ref mut?` + by-move conflict.
691689
}
690+
Some(ty::BindByValue(_)) | None => {} // `ref mut?` + by-copy is fine.
692691
});
692+
693+
// Report errors if any.
694+
if !conflicts_mut_mut.is_empty() {
695+
// Report mutability conflicts for e.g. `ref mut x @ Some(ref mut y)`.
696+
let msg = &format!("cannot borrow `{}` as mutable more than once at a time", name);
697+
sess.struct_span_err(pat.span, msg)
698+
.span_label(binding_span, "first mutable borrow occurs here")
699+
.span_labels(conflicts_mut_mut, "another mutable borrow occurs here")
700+
.span_labels(conflicts_mut_ref, "also borrowed as immutable here")
701+
.span_labels(conflicts_move, "also moved here")
702+
.emit();
703+
} else if !conflicts_mut_ref.is_empty() {
704+
// Report mutability conflicts for e.g. `ref x @ Some(ref mut y)` or the converse.
705+
let (primary, also) = match mut_outer {
706+
Mutability::Mut => ("mutable", "immutable"),
707+
Mutability::Not => ("immutable", "mutable"),
708+
};
709+
let msg = &format!(
710+
"cannot borrow `{}` as {} because it is also borrowed as {}",
711+
name, also, primary,
712+
);
713+
sess.struct_span_err(pat.span, msg)
714+
.span_label(binding_span, format!("{} borrow occurs here", primary))
715+
.span_labels(conflicts_mut_ref, format!("{} borrow occurs here", also))
716+
.span_labels(conflicts_move, "also moved here")
717+
.emit();
718+
} else if !conflicts_move.is_empty() {
719+
// Report by-ref and by-move conflicts, e.g. `ref x @ y`.
720+
let msg = &format!("cannot move out of `{}` because it is borrowed", name);
721+
sess.struct_span_err(pat.span, msg)
722+
.span_label(binding_span, format!("borrow of `{}` occurs here", name))
723+
.span_labels(conflicts_move, format!("move out of `{}` occurs here", name))
724+
.emit();
725+
}
693726
}
694727

695728
/// Forbids bindings in `@` patterns. This used to be is necessary for memory safety,

src/librustc_span/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,7 @@ symbols! {
455455
module,
456456
module_path,
457457
more_struct_aliases,
458+
move_ref_pattern,
458459
move_val_init,
459460
movbe_target_feature,
460461
mul_with_overflow,

0 commit comments

Comments
 (0)