Skip to content

Commit

Permalink
Auto merge of #49870 - pnkfelix:issue-27282-immut-borrow-all-pat-ids-…
Browse files Browse the repository at this point in the history
…in-guards, r=nikomatsakis

Immutably and implicitly borrow all pattern ids for their guards (NLL only)

This is an important piece of #27282.

It applies only to NLL mode. It is a change to MIR codegen that is currently toggled on only when NLL is turned on. It thus affect MIR-borrowck but not the earlier static analyses (such as the type checker).

This change makes it so that any pattern bindings of type T for a match arm will map to a `&T` within the context of the guard expression for that arm, but will continue to map to a `T` in the context of the arm body.

To avoid surfacing this type distinction in the user source code (which would be a severe change to the language and would also require far more revision to the compiler internals), any occurrence of such an identifier in the guard expression will automatically get a deref op applied to it.

So an input like:
```rust
let place = (1, Foo::new());
match place {
  (1, foo) if inspect(foo) => feed(foo),
  ...
}
```
will be treated as if it were really something like:
 ```rust
let place = (1, Foo::new());
match place {
    (1, Foo { .. }) if { let tmp1 = &place.1; inspect(*tmp1) }
                    => { let tmp2 = place.1; feed(tmp2) },
    ...
}
```

And an input like:
```rust
let place = (2, Foo::new());
match place {
    (2, ref mut foo) if inspect(foo) => feed(foo),
    ...
}
```
will be treated as if it were really something like:

```rust
let place = (2, Foo::new());
match place {
    (2, Foo { .. }) if { let tmp1 = & &mut place.1; inspect(*tmp1) }
                    => { let tmp2 = &mut place.1; feed(tmp2) },
    ...
}
```

In short, any pattern binding will always look like *some* kind of `&T` within the guard at least in terms of how the MIR-borrowck views it, and this will ensure that guard expressions cannot mutate their the match inputs via such bindings. (It also ensures that guard expressions can at most *copy* values from such bindings; non-Copy things cannot be moved via these pattern bindings in guard expressions, since one cannot move out of a `&T`.)
  • Loading branch information
bors committed May 4, 2018
2 parents 0bfe307 + 930e76e commit 91db9dc
Show file tree
Hide file tree
Showing 10 changed files with 505 additions and 92 deletions.
8 changes: 8 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.borrowck_mode().use_mir()
}

/// If true, pattern variables for use in guards on match arms
/// will be bound as references to the data, and occurrences of
/// those variables in the guard expression will implicitly
/// dereference those bindings. (See rust-lang/rust#27282.)
pub fn all_pat_vars_are_implicit_refs_within_guards(self) -> bool {
self.borrowck_mode().use_mir()
}

/// If true, we should enable two-phase borrows checks. This is
/// done with either `-Ztwo-phase-borrows` or with
/// `#![feature(nll)]`.
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
| (RegionKind::ReClosureBound(_), _)
| (RegionKind::ReCanonical(_), _)
| (RegionKind::ReErased, _) => {
span_bug!(drop_span, "region does not make sense in this context");
span_bug!(drop_span, "region {:?} does not make sense in this context",
borrow.region);
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions src/librustc_mir/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
// except according to those terms.

use build::{BlockAnd, BlockAndExtension, Builder};
use build::ForGuard::OutsideGuard;
use build::matches::ArmHasGuard;
use hair::*;
use rustc::mir::*;
use rustc::hir;
Expand Down Expand Up @@ -113,7 +115,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// Declare the bindings, which may create a visibility scope.
let remainder_span = remainder_scope.span(this.hir.tcx(),
&this.hir.region_scope_tree);
let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern);
let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern,
ArmHasGuard(false));

// Evaluate the initializer, if present.
if let Some(init) = initializer {
Expand All @@ -135,8 +138,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}

this.visit_bindings(&pattern, &mut |this, _, _, node, span, _| {
this.storage_live_binding(block, node, span);
this.schedule_drop_for_binding(node, span);
this.storage_live_binding(block, node, span, OutsideGuard);
this.schedule_drop_for_binding(node, span, OutsideGuard);
})
}

Expand Down
15 changes: 13 additions & 2 deletions src/librustc_mir/build/expr/as_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//! See docs in build/expr/mod.rs

use build::{BlockAnd, BlockAndExtension, Builder};
use build::ForGuard::{OutsideGuard, WithinGuard};
use build::expr::category::Category;
use hair::*;
use rustc::mir::*;
Expand Down Expand Up @@ -86,8 +87,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
block.and(Place::Local(Local::new(1)))
}
ExprKind::VarRef { id } => {
let index = this.var_indices[&id];
block.and(Place::Local(index))
let place = if this.is_bound_var_in_guard(id) {
let index = this.var_local_id(id, WithinGuard);
if this.hir.tcx().all_pat_vars_are_implicit_refs_within_guards() {
Place::Local(index).deref()
} else {
Place::Local(index)
}
} else {
let index = this.var_local_id(id, OutsideGuard);
Place::Local(index)
};
block.and(place)
}
ExprKind::StaticRef { id } => {
block.and(Place::Static(Box::new(Static { def_id: id, ty: expr.ty })))
Expand Down
Loading

0 comments on commit 91db9dc

Please sign in to comment.