Skip to content

allow deref patterns to move out of boxes #140022

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
51 changes: 30 additions & 21 deletions compiler/rustc_hir_typeck/src/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,13 +1000,15 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
// determines whether to borrow *at the level of the deref pattern* rather than
// borrowing the bound place (since that inner place is inside the temporary that
// stores the result of calling `deref()`/`deref_mut()` so can't be captured).
// Deref patterns on boxes don't borrow, so we ignore them here.
// HACK: this could be a fake pattern corresponding to a deref inserted by match
// ergonomics, in which case `pat.hir_id` will be the id of the subpattern.
let mutable = self.cx.typeck_results().pat_has_ref_mut_binding(subpattern);
let mutability =
if mutable { hir::Mutability::Mut } else { hir::Mutability::Not };
let bk = ty::BorrowKind::from_mutbl(mutability);
self.delegate.borrow_mut().borrow(place, discr_place.hir_id, bk);
if let hir::ByRef::Yes(mutability) =
self.cx.typeck_results().deref_pat_borrow_mode(place.place.ty(), subpattern)
{
let bk = ty::BorrowKind::from_mutbl(mutability);
self.delegate.borrow_mut().borrow(place, discr_place.hir_id, bk);
}
}
PatKind::Never => {
// A `!` pattern always counts as an immutable read of the discriminant,
Expand Down Expand Up @@ -1691,18 +1693,19 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
place_with_id = match adjust.kind {
adjustment::PatAdjust::BuiltinDeref => self.cat_deref(pat.hir_id, place_with_id)?,
adjustment::PatAdjust::OverloadedDeref => {
// This adjustment corresponds to an overloaded deref; it borrows the scrutinee to
// call `Deref::deref` or `DerefMut::deref_mut`. Invoke the callback before setting
// `place_with_id` to the temporary storing the result of the deref.
// This adjustment corresponds to an overloaded deref; unless it's on a box, it
// borrows the scrutinee to call `Deref::deref` or `DerefMut::deref_mut`. Invoke
// the callback before setting `place_with_id` to the temporary storing the
// result of the deref.
// HACK(dianne): giving the callback a fake deref pattern makes sure it behaves the
// same as it would if this were an explicit deref pattern.
// same as it would if this were an explicit deref pattern (including for boxes).
op(&place_with_id, &hir::Pat { kind: PatKind::Deref(pat), ..*pat })?;
let target_ty = match adjusts.peek() {
Some(&&next_adjust) => next_adjust.source,
// At the end of the deref chain, we get `pat`'s scrutinee.
None => self.pat_ty_unadjusted(pat)?,
};
self.pat_deref_temp(pat.hir_id, pat, target_ty)?
self.pat_deref_place(pat.hir_id, place_with_id, pat, target_ty)?
}
};
}
Expand Down Expand Up @@ -1810,7 +1813,7 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
}
PatKind::Deref(subpat) => {
let ty = self.pat_ty_adjusted(subpat)?;
let place = self.pat_deref_temp(pat.hir_id, subpat, ty)?;
let place = self.pat_deref_place(pat.hir_id, place_with_id, subpat, ty)?;
self.cat_pattern(place, subpat, op)?;
}

Expand Down Expand Up @@ -1863,21 +1866,27 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
Ok(())
}

/// Represents the place of the temp that stores the scrutinee of a deref pattern's interior.
fn pat_deref_temp(
/// Represents the place matched on by a deref pattern's interior.
fn pat_deref_place(
&self,
hir_id: HirId,
base_place: PlaceWithHirId<'tcx>,
inner: &hir::Pat<'_>,
target_ty: Ty<'tcx>,
) -> Result<PlaceWithHirId<'tcx>, Cx::Error> {
let mutable = self.cx.typeck_results().pat_has_ref_mut_binding(inner);
let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not };
let re_erased = self.cx.tcx().lifetimes.re_erased;
let ty = Ty::new_ref(self.cx.tcx(), re_erased, target_ty, mutability);
// A deref pattern stores the result of `Deref::deref` or `DerefMut::deref_mut` ...
let base = self.cat_rvalue(hir_id, ty);
// ... and the inner pattern matches on the place behind that reference.
self.cat_deref(hir_id, base)
match self.cx.typeck_results().deref_pat_borrow_mode(base_place.place.ty(), inner) {
// Deref patterns on boxes are lowered using a built-in deref.
hir::ByRef::No => self.cat_deref(hir_id, base_place),
// For other types, we create a temporary to match on.
hir::ByRef::Yes(mutability) => {
let re_erased = self.cx.tcx().lifetimes.re_erased;
let ty = Ty::new_ref(self.cx.tcx(), re_erased, target_ty, mutability);
// A deref pattern stores the result of `Deref::deref` or `DerefMut::deref_mut` ...
let base = self.cat_rvalue(hir_id, ty);
// ... and the inner pattern matches on the place behind that reference.
self.cat_deref(hir_id, base)
}
}
}

fn is_multivariant_adt(&self, ty: Ty<'tcx>, span: Span) -> bool {
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,12 @@ pub enum PatKind<'tcx> {
/// Deref pattern, written `box P` for now.
DerefPattern {
subpattern: Box<Pat<'tcx>>,
mutability: hir::Mutability,
/// Whether the pattern scrutinee needs to be borrowed in order to call `Deref::deref` or
/// `DerefMut::deref_mut`, and if so, which. This is `ByRef::No` for deref patterns on
/// boxes; they are lowered using a built-in deref rather than a method call, thus they
/// don't borrow the scrutinee.
#[type_visitable(ignore)]
borrow: ByRef,
},

/// One of the following:
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_middle/src/ty/typeck_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,21 @@ impl<'tcx> TypeckResults<'tcx> {
has_ref_mut
}

/// How should a deref pattern find the place for its inner pattern to match on?
///
/// In most cases, if the pattern recursively contains a `ref mut` binding, we find the inner
/// pattern's scrutinee by calling `DerefMut::deref_mut`, and otherwise we call `Deref::deref`.
/// However, for boxes we can use a built-in deref instead, which doesn't borrow the scrutinee;
/// in this case, we return `ByRef::No`.
pub fn deref_pat_borrow_mode(&self, pointer_ty: Ty<'_>, inner: &hir::Pat<'_>) -> ByRef {
if pointer_ty.is_box() {
ByRef::No
} else {
let mutable = self.pat_has_ref_mut_binding(inner);
ByRef::Yes(if mutable { Mutability::Mut } else { Mutability::Not })
}
}

/// For a given closure, returns the iterator of `ty::CapturedPlace`s that are captured
/// by the closure.
pub fn closure_min_captures_flattened(
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_mir_build/src/builder/matches/match_pair.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::sync::Arc;

use rustc_hir::ByRef;
use rustc_middle::mir::*;
use rustc_middle::thir::*;
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
Expand Down Expand Up @@ -260,7 +261,13 @@ impl<'tcx> MatchPairTree<'tcx> {
None
}

PatKind::Deref { ref subpattern } => {
PatKind::Deref { ref subpattern }
| PatKind::DerefPattern { ref subpattern, borrow: ByRef::No } => {
if cfg!(debug_assertions) && matches!(pattern.kind, PatKind::DerefPattern { .. }) {
// Only deref patterns on boxes can be lowered using a built-in deref.
debug_assert!(pattern.ty.is_box());
}

MatchPairTree::for_pattern(
place_builder.deref(),
subpattern,
Expand All @@ -271,7 +278,7 @@ impl<'tcx> MatchPairTree<'tcx> {
None
}

PatKind::DerefPattern { ref subpattern, mutability } => {
PatKind::DerefPattern { ref subpattern, borrow: ByRef::Yes(mutability) } => {
// Create a new temporary for each deref pattern.
// FIXME(deref_patterns): dedup temporaries to avoid multiple `deref()` calls?
let temp = cx.temp(
Expand Down
11 changes: 4 additions & 7 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
let kind = match adjust.kind {
PatAdjust::BuiltinDeref => PatKind::Deref { subpattern: thir_pat },
PatAdjust::OverloadedDeref => {
let mutable = self.typeck_results.pat_has_ref_mut_binding(pat);
let mutability =
if mutable { hir::Mutability::Mut } else { hir::Mutability::Not };
PatKind::DerefPattern { subpattern: thir_pat, mutability }
let borrow = self.typeck_results.deref_pat_borrow_mode(adjust.source, pat);
PatKind::DerefPattern { subpattern: thir_pat, borrow }
}
};
Box::new(Pat { span, ty: adjust.source, kind })
Expand Down Expand Up @@ -308,9 +306,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
}

hir::PatKind::Deref(subpattern) => {
let mutable = self.typeck_results.pat_has_ref_mut_binding(subpattern);
let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not };
PatKind::DerefPattern { subpattern: self.lower_pattern(subpattern), mutability }
let borrow = self.typeck_results.deref_pat_borrow_mode(ty, subpattern);
PatKind::DerefPattern { subpattern: self.lower_pattern(subpattern), borrow }
}
hir::PatKind::Ref(subpattern, _) => {
// Track the default binding mode for the Rust 2024 migration suggestion.
Expand Down
15 changes: 14 additions & 1 deletion src/doc/unstable-book/src/language-features/deref-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ The tracking issue for this feature is: [#87121]
------------------------

> **Note**: This feature is incomplete. In the future, it is meant to supersede
> [`box_patterns`](./box-patterns.md) and [`string_deref_patterns`](./string-deref-patterns.md).
> [`box_patterns`] and [`string_deref_patterns`].

This feature permits pattern matching on [smart pointers in the standard library] through their
`Deref` target types, either implicitly or with explicit `deref!(_)` patterns (the syntax of which
Expand Down Expand Up @@ -54,6 +54,17 @@ if let [b] = &mut *v {
assert_eq!(v, [Box::new(Some(2))]);
```

Like [`box_patterns`], deref patterns may move out of boxes:

```rust
# #![feature(deref_patterns)]
# #![allow(incomplete_features)]
struct NoCopy;
// Match exhaustiveness analysis is not yet implemented.
let deref!(x) = Box::new(NoCopy) else { unreachable!() };
drop::<NoCopy>(x);
```

Additionally, when `deref_patterns` is enabled, string literal patterns may be written where `str`
is expected. Likewise, byte string literal patterns may be written where `[u8]` or `[u8; _]` is
expected. This lets them be used in `deref!(_)` patterns:
Expand All @@ -75,4 +86,6 @@ match *"test" {

Implicit deref pattern syntax is not yet supported for string or byte string literals.

[`box_patterns`]: ./box-patterns.md
[`string_deref_patterns`]: ./string-deref-patterns.md
[smart pointers in the standard library]: https://doc.rust-lang.org/std/ops/trait.DerefPure.html#implementors
22 changes: 12 additions & 10 deletions tests/ui/pattern/deref-patterns/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#![feature(deref_patterns)]
#![allow(incomplete_features)]

use std::rc::Rc;

#[cfg(explicit)]
fn simple_vec(vec: Vec<u32>) -> u32 {
match vec {
Expand Down Expand Up @@ -53,37 +55,37 @@ fn nested_vec(vecvec: Vec<Vec<u32>>) -> u32 {

#[cfg(explicit)]
fn ref_mut(val: u32) -> u32 {
let mut b = Box::new(0u32);
let mut b = vec![0u32];
match &mut b {
deref!(_x) if false => unreachable!(),
deref!(x) => {
deref!([_x]) if false => unreachable!(),
deref!([x]) => {
*x = val;
}
_ => unreachable!(),
}
let deref!(x) = &b else { unreachable!() };
let deref!([x]) = &b else { unreachable!() };
*x
}

#[cfg(implicit)]
fn ref_mut(val: u32) -> u32 {
let mut b = Box::new((0u32,));
let mut b = vec![0u32];
match &mut b {
(_x,) if false => unreachable!(),
(x,) => {
[_x] if false => unreachable!(),
[x] => {
*x = val;
}
_ => unreachable!(),
}
let (x,) = &b else { unreachable!() };
let [x] = &b else { unreachable!() };
*x
}

#[cfg(explicit)]
#[rustfmt::skip]
fn or_and_guard(tuple: (u32, u32)) -> u32 {
let mut sum = 0;
let b = Box::new(tuple);
let b = Rc::new(tuple);
match b {
deref!((x, _) | (_, x)) if { sum += x; false } => {},
_ => {},
Expand All @@ -95,7 +97,7 @@ fn or_and_guard(tuple: (u32, u32)) -> u32 {
#[rustfmt::skip]
fn or_and_guard(tuple: (u32, u32)) -> u32 {
let mut sum = 0;
let b = Box::new(tuple);
let b = Rc::new(tuple);
match b {
(x, _) | (_, x) if { sum += x; false } => {},
_ => {},
Expand Down
20 changes: 10 additions & 10 deletions tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use std::rc::Rc;

struct Struct;

fn cant_move_out_box(b: Box<Struct>) -> Struct {
fn cant_move_out_vec(b: Vec<Struct>) -> Struct {
match b {
//~^ ERROR: cannot move out of a shared reference
deref!(x) => x,
_ => unreachable!(),
//~^ ERROR: cannot move out of type `[Struct]`, a non-copy slice
deref!([x]) => x,
_ => panic!(),
}
}

Expand All @@ -21,16 +21,16 @@ fn cant_move_out_rc(rc: Rc<Struct>) -> Struct {
}
}

struct Container(Struct);

fn cant_move_out_box_implicit(b: Box<Container>) -> Struct {
fn cant_move_out_vec_implicit(b: Vec<Struct>) -> Struct {
match b {
//~^ ERROR: cannot move out of a shared reference
Container(x) => x,
_ => unreachable!(),
//~^ ERROR: cannot move out of type `[Struct]`, a non-copy slice
[x] => x,
_ => panic!(),
}
}

struct Container(Struct);

fn cant_move_out_rc_implicit(rc: Rc<Container>) -> Struct {
match rc {
//~^ ERROR: cannot move out of a shared reference
Expand Down
41 changes: 21 additions & 20 deletions tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
error[E0507]: cannot move out of a shared reference
error[E0508]: cannot move out of type `[Struct]`, a non-copy slice
--> $DIR/cant_move_out_of_pattern.rs:9:11
|
LL | match b {
| ^
| ^ cannot move out of here
LL |
LL | deref!(x) => x,
| -
| |
| data moved here
| move occurs because `x` has type `Struct`, which does not implement the `Copy` trait
LL | deref!([x]) => x,
| -
| |
| data moved here
| move occurs because `x` has type `Struct`, which does not implement the `Copy` trait
|
help: consider borrowing the pattern binding
|
LL | deref!(ref x) => x,
| +++
LL | deref!([ref x]) => x,
| +++

error[E0507]: cannot move out of a shared reference
--> $DIR/cant_move_out_of_pattern.rs:17:11
Expand All @@ -32,22 +32,22 @@ help: consider borrowing the pattern binding
LL | deref!(ref x) => x,
| +++

error[E0507]: cannot move out of a shared reference
--> $DIR/cant_move_out_of_pattern.rs:27:11
error[E0508]: cannot move out of type `[Struct]`, a non-copy slice
--> $DIR/cant_move_out_of_pattern.rs:25:11
|
LL | match b {
| ^
| ^ cannot move out of here
LL |
LL | Container(x) => x,
| -
| |
| data moved here
| move occurs because `x` has type `Struct`, which does not implement the `Copy` trait
LL | [x] => x,
| -
| |
| data moved here
| move occurs because `x` has type `Struct`, which does not implement the `Copy` trait
|
help: consider borrowing the pattern binding
|
LL | Container(ref x) => x,
| +++
LL | [ref x] => x,
| +++

error[E0507]: cannot move out of a shared reference
--> $DIR/cant_move_out_of_pattern.rs:35:11
Expand All @@ -68,4 +68,5 @@ LL | Container(ref x) => x,

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0507`.
Some errors have detailed explanations: E0507, E0508.
For more information about an error, try `rustc --explain E0507`.
Loading
Loading