Skip to content

Commit

Permalink
Rollup merge of rust-lang#73489 - sexxi-goose:init_place_refactor, r=…
Browse files Browse the repository at this point in the history
…nikomatsakis

Refactor hir::Place

For the following code
```rust
let c = || bar(foo.x, foo.x)
```

We generate two different `hir::Place`s for both `foo.x`.
Handling this adds overhead for analysis we need to do for RFC 2229.

We also want to store type information at each Projection to support
analysis as part of the RFC. This resembles what we have for
`mir::Place`

This commit modifies the Place as follows:
- Rename to `PlaceWithHirId`, where there `hir_id` is that of the
expressioin.
- Move any other information that describes the access out to another
struct now called `Place`.
- Removed `Span`, it can be accessed using the [hir
API](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/hir/map/struct.Map.html#method.span)
- Modify `Projection` to be a strucutre of its own, that currently only
contains the `ProjectionKind`.

Adding type information to projections wil be completed as part of rust-lang/project-rfc-2229#5

Closes rust-lang/project-rfc-2229#3
  • Loading branch information
Manishearth committed Jun 19, 2020
2 parents d2272d4 + 3659406 commit a88182f
Show file tree
Hide file tree
Showing 8 changed files with 222 additions and 195 deletions.
29 changes: 16 additions & 13 deletions src/librustc_typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,10 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {

/// Invoked on any adjustments that occur. Checks that if this is a region pointer being
/// dereferenced, the lifetime of the pointer includes the deref expr.
fn constrain_adjustments(&mut self, expr: &hir::Expr<'_>) -> mc::McResult<mc::Place<'tcx>> {
fn constrain_adjustments(
&mut self,
expr: &hir::Expr<'_>,
) -> mc::McResult<mc::PlaceWithHirId<'tcx>> {
debug!("constrain_adjustments(expr={:?})", expr);

let mut place = self.with_mc(|mc| mc.cat_expr_unadjusted(expr))?;
Expand Down Expand Up @@ -480,12 +483,12 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {

fn check_safety_of_rvalue_destructor_if_necessary(
&mut self,
place: &mc::Place<'tcx>,
place_with_id: &mc::PlaceWithHirId<'tcx>,
span: Span,
) {
if let mc::PlaceBase::Rvalue = place.base {
if place.projections.is_empty() {
let typ = self.resolve_type(place.ty);
if let mc::PlaceBase::Rvalue = place_with_id.place.base {
if place_with_id.place.projections.is_empty() {
let typ = self.resolve_type(place_with_id.place.ty);
let body_id = self.body_id;
let _ = dropck::check_drop_obligations(self, typ, span, body_id);
}
Expand Down Expand Up @@ -570,7 +573,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {

/// Link lifetimes of any ref bindings in `root_pat` to the pointers found
/// in the discriminant, if needed.
fn link_pattern(&self, discr_cmt: mc::Place<'tcx>, root_pat: &hir::Pat<'_>) {
fn link_pattern(&self, discr_cmt: mc::PlaceWithHirId<'tcx>, root_pat: &hir::Pat<'_>) {
debug!("link_pattern(discr_cmt={:?}, root_pat={:?})", discr_cmt, root_pat);
ignore_err!(self.with_mc(|mc| {
mc.cat_pattern(discr_cmt, root_pat, |sub_cmt, hir::Pat { kind, span, hir_id }| {
Expand All @@ -591,7 +594,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {
fn link_autoref(
&self,
expr: &hir::Expr<'_>,
expr_cmt: &mc::Place<'tcx>,
expr_cmt: &mc::PlaceWithHirId<'tcx>,
autoref: &adjustment::AutoBorrow<'tcx>,
) {
debug!("link_autoref(autoref={:?}, expr_cmt={:?})", autoref, expr_cmt);
Expand All @@ -612,7 +615,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {
span: Span,
id: hir::HirId,
mutbl: hir::Mutability,
cmt_borrowed: &mc::Place<'tcx>,
cmt_borrowed: &mc::PlaceWithHirId<'tcx>,
) {
debug!(
"link_region_from_node_type(id={:?}, mutbl={:?}, cmt_borrowed={:?})",
Expand All @@ -635,12 +638,12 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {
span: Span,
borrow_region: ty::Region<'tcx>,
borrow_kind: ty::BorrowKind,
borrow_place: &mc::Place<'tcx>,
borrow_place: &mc::PlaceWithHirId<'tcx>,
) {
let origin = infer::DataBorrowed(borrow_place.ty, span);
self.type_must_outlive(origin, borrow_place.ty, borrow_region);
let origin = infer::DataBorrowed(borrow_place.place.ty, span);
self.type_must_outlive(origin, borrow_place.place.ty, borrow_region);

for pointer_ty in borrow_place.deref_tys() {
for pointer_ty in borrow_place.place.deref_tys() {
debug!(
"link_region(borrow_region={:?}, borrow_kind={:?}, pointer_ty={:?})",
borrow_region, borrow_kind, borrow_place
Expand All @@ -656,7 +659,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {
_ => assert!(pointer_ty.is_box(), "unexpected built-in deref type {}", pointer_ty),
}
}
if let mc::PlaceBase::Upvar(upvar_id) = borrow_place.base {
if let mc::PlaceBase::Upvar(upvar_id) = borrow_place.place.base {
self.link_upvar_region(span, borrow_region, upvar_id);
}
}
Expand Down
57 changes: 34 additions & 23 deletions src/librustc_typeck/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,13 @@ struct InferBorrowKind<'a, 'tcx> {
impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
fn adjust_upvar_borrow_kind_for_consume(
&mut self,
place: &mc::Place<'tcx>,
place_with_id: &mc::PlaceWithHirId<'tcx>,
mode: euv::ConsumeMode,
) {
debug!("adjust_upvar_borrow_kind_for_consume(place={:?}, mode={:?})", place, mode);
debug!(
"adjust_upvar_borrow_kind_for_consume(place_with_id={:?}, mode={:?})",
place_with_id, mode
);

// we only care about moves
match mode {
Expand All @@ -284,7 +287,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
}

let tcx = self.fcx.tcx;
let upvar_id = if let PlaceBase::Upvar(upvar_id) = place.base {
let upvar_id = if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base {
upvar_id
} else {
return;
Expand All @@ -296,22 +299,22 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
self.adjust_closure_kind(
upvar_id.closure_expr_id,
ty::ClosureKind::FnOnce,
place.span,
tcx.hir().span(place_with_id.hir_id),
var_name(tcx, upvar_id.var_path.hir_id),
);

self.adjust_upvar_captures.insert(upvar_id, ty::UpvarCapture::ByValue);
}

/// Indicates that `place` is being directly mutated (e.g., assigned
/// Indicates that `place_with_id` is being directly mutated (e.g., assigned
/// to). If the place is based on a by-ref upvar, this implies that
/// the upvar must be borrowed using an `&mut` borrow.
fn adjust_upvar_borrow_kind_for_mut(&mut self, place: &mc::Place<'tcx>) {
debug!("adjust_upvar_borrow_kind_for_mut(place={:?})", place);
fn adjust_upvar_borrow_kind_for_mut(&mut self, place_with_id: &mc::PlaceWithHirId<'tcx>) {
debug!("adjust_upvar_borrow_kind_for_mut(place_with_id={:?})", place_with_id);

if let PlaceBase::Upvar(upvar_id) = place.base {
if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base {
let mut borrow_kind = ty::MutBorrow;
for pointer_ty in place.deref_tys() {
for pointer_ty in place_with_id.place.deref_tys() {
match pointer_ty.kind {
// Raw pointers don't inherit mutability.
ty::RawPtr(_) => return,
Expand All @@ -323,20 +326,28 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
_ => (),
}
}
self.adjust_upvar_deref(upvar_id, place.span, borrow_kind);
self.adjust_upvar_deref(
upvar_id,
self.fcx.tcx.hir().span(place_with_id.hir_id),
borrow_kind,
);
}
}

fn adjust_upvar_borrow_kind_for_unique(&mut self, place: &mc::Place<'tcx>) {
debug!("adjust_upvar_borrow_kind_for_unique(place={:?})", place);
fn adjust_upvar_borrow_kind_for_unique(&mut self, place_with_id: &mc::PlaceWithHirId<'tcx>) {
debug!("adjust_upvar_borrow_kind_for_unique(place_with_id={:?})", place_with_id);

if let PlaceBase::Upvar(upvar_id) = place.base {
if place.deref_tys().any(ty::TyS::is_unsafe_ptr) {
if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base {
if place_with_id.place.deref_tys().any(ty::TyS::is_unsafe_ptr) {
// Raw pointers don't inherit mutability.
return;
}
// for a borrowed pointer to be unique, its base must be unique
self.adjust_upvar_deref(upvar_id, place.span, ty::UniqueImmBorrow);
self.adjust_upvar_deref(
upvar_id,
self.fcx.tcx.hir().span(place_with_id.hir_id),
ty::UniqueImmBorrow,
);
}
}

Expand Down Expand Up @@ -453,26 +464,26 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
}

impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
fn consume(&mut self, place: &mc::Place<'tcx>, mode: euv::ConsumeMode) {
debug!("consume(place={:?},mode={:?})", place, mode);
self.adjust_upvar_borrow_kind_for_consume(place, mode);
fn consume(&mut self, place_with_id: &mc::PlaceWithHirId<'tcx>, mode: euv::ConsumeMode) {
debug!("consume(place_with_id={:?},mode={:?})", place_with_id, mode);
self.adjust_upvar_borrow_kind_for_consume(place_with_id, mode);
}

fn borrow(&mut self, place: &mc::Place<'tcx>, bk: ty::BorrowKind) {
debug!("borrow(place={:?}, bk={:?})", place, bk);
fn borrow(&mut self, place_with_id: &mc::PlaceWithHirId<'tcx>, bk: ty::BorrowKind) {
debug!("borrow(place_with_id={:?}, bk={:?})", place_with_id, bk);

match bk {
ty::ImmBorrow => {}
ty::UniqueImmBorrow => {
self.adjust_upvar_borrow_kind_for_unique(place);
self.adjust_upvar_borrow_kind_for_unique(place_with_id);
}
ty::MutBorrow => {
self.adjust_upvar_borrow_kind_for_mut(place);
self.adjust_upvar_borrow_kind_for_mut(place_with_id);
}
}
}

fn mutate(&mut self, assignee_place: &mc::Place<'tcx>) {
fn mutate(&mut self, assignee_place: &mc::PlaceWithHirId<'tcx>) {
debug!("mutate(assignee_place={:?})", assignee_place);

self.adjust_upvar_borrow_kind_for_mut(assignee_place);
Expand Down
64 changes: 36 additions & 28 deletions src/librustc_typeck/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
pub use self::ConsumeMode::*;

// Export these here so that Clippy can use them.
pub use mc::{Place, PlaceBase, Projection};
pub use mc::{PlaceBase, PlaceWithHirId, Projection};

use rustc_hir as hir;
use rustc_hir::def::Res;
Expand All @@ -25,13 +25,13 @@ use rustc_span::Span;
pub trait Delegate<'tcx> {
// The value found at `place` is either copied or moved, depending
// on mode.
fn consume(&mut self, place: &mc::Place<'tcx>, mode: ConsumeMode);
fn consume(&mut self, place_with_id: &mc::PlaceWithHirId<'tcx>, mode: ConsumeMode);

// The value found at `place` is being borrowed with kind `bk`.
fn borrow(&mut self, place: &mc::Place<'tcx>, bk: ty::BorrowKind);
fn borrow(&mut self, place_with_id: &mc::PlaceWithHirId<'tcx>, bk: ty::BorrowKind);

// The path at `place` is being assigned to.
fn mutate(&mut self, assignee_place: &mc::Place<'tcx>);
// The path at `place_with_id` is being assigned to.
fn mutate(&mut self, assignee_place: &mc::PlaceWithHirId<'tcx>);
}

#[derive(Copy, Clone, PartialEq, Debug)]
Expand Down Expand Up @@ -113,11 +113,11 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
self.mc.tcx()
}

fn delegate_consume(&mut self, place: &Place<'tcx>) {
debug!("delegate_consume(place={:?})", place);
fn delegate_consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>) {
debug!("delegate_consume(place_with_id={:?})", place_with_id);

let mode = copy_or_move(&self.mc, place);
self.delegate.consume(place, mode);
let mode = copy_or_move(&self.mc, place_with_id);
self.delegate.consume(place_with_id, mode);
}

fn consume_exprs(&mut self, exprs: &[hir::Expr<'_>]) {
Expand All @@ -129,22 +129,22 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
pub fn consume_expr(&mut self, expr: &hir::Expr<'_>) {
debug!("consume_expr(expr={:?})", expr);

let place = return_if_err!(self.mc.cat_expr(expr));
self.delegate_consume(&place);
let place_with_id = return_if_err!(self.mc.cat_expr(expr));
self.delegate_consume(&place_with_id);
self.walk_expr(expr);
}

fn mutate_expr(&mut self, expr: &hir::Expr<'_>) {
let place = return_if_err!(self.mc.cat_expr(expr));
self.delegate.mutate(&place);
let place_with_id = return_if_err!(self.mc.cat_expr(expr));
self.delegate.mutate(&place_with_id);
self.walk_expr(expr);
}

fn borrow_expr(&mut self, expr: &hir::Expr<'_>, bk: ty::BorrowKind) {
debug!("borrow_expr(expr={:?}, bk={:?})", expr, bk);

let place = return_if_err!(self.mc.cat_expr(expr));
self.delegate.borrow(&place, bk);
let place_with_id = return_if_err!(self.mc.cat_expr(expr));
self.delegate.borrow(&place_with_id, bk);

self.walk_expr(expr)
}
Expand Down Expand Up @@ -384,7 +384,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {

// Select just those fields of the `with`
// expression that will actually be used
match with_place.ty.kind {
match with_place.place.ty.kind {
ty::Adt(adt, substs) if adt.is_struct() => {
// Consume those fields of the with expression that are needed.
for (f_index, with_field) in adt.non_enum_variant().fields.iter().enumerate() {
Expand Down Expand Up @@ -422,14 +422,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
// process.
fn walk_adjustment(&mut self, expr: &hir::Expr<'_>) {
let adjustments = self.mc.tables.expr_adjustments(expr);
let mut place = return_if_err!(self.mc.cat_expr_unadjusted(expr));
let mut place_with_id = return_if_err!(self.mc.cat_expr_unadjusted(expr));
for adjustment in adjustments {
debug!("walk_adjustment expr={:?} adj={:?}", expr, adjustment);
match adjustment.kind {
adjustment::Adjust::NeverToAny | adjustment::Adjust::Pointer(_) => {
// Creating a closure/fn-pointer or unsizing consumes
// the input and stores it into the resulting rvalue.
self.delegate_consume(&place);
self.delegate_consume(&place_with_id);
}

adjustment::Adjust::Deref(None) => {}
Expand All @@ -441,14 +441,15 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
// this is an autoref of `x`.
adjustment::Adjust::Deref(Some(ref deref)) => {
let bk = ty::BorrowKind::from_mutbl(deref.mutbl);
self.delegate.borrow(&place, bk);
self.delegate.borrow(&place_with_id, bk);
}

adjustment::Adjust::Borrow(ref autoref) => {
self.walk_autoref(expr, &place, autoref);
self.walk_autoref(expr, &place_with_id, autoref);
}
}
place = return_if_err!(self.mc.cat_expr_adjusted(expr, place, &adjustment));
place_with_id =
return_if_err!(self.mc.cat_expr_adjusted(expr, place_with_id, &adjustment));
}
}

Expand All @@ -458,7 +459,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
fn walk_autoref(
&mut self,
expr: &hir::Expr<'_>,
base_place: &mc::Place<'tcx>,
base_place: &mc::PlaceWithHirId<'tcx>,
autoref: &adjustment::AutoBorrow<'tcx>,
) {
debug!(
Expand All @@ -479,7 +480,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
}
}

fn walk_arm(&mut self, discr_place: &Place<'tcx>, arm: &hir::Arm<'_>) {
fn walk_arm(&mut self, discr_place: &PlaceWithHirId<'tcx>, arm: &hir::Arm<'_>) {
self.walk_pat(discr_place, &arm.pat);

if let Some(hir::Guard::If(ref e)) = arm.guard {
Expand All @@ -491,12 +492,12 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {

/// Walks a pat that occurs in isolation (i.e., top-level of fn argument or
/// let binding, and *not* a match arm or nested pat.)
fn walk_irrefutable_pat(&mut self, discr_place: &Place<'tcx>, pat: &hir::Pat<'_>) {
fn walk_irrefutable_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
self.walk_pat(discr_place, pat);
}

/// The core driver for walking a pattern
fn walk_pat(&mut self, discr_place: &Place<'tcx>, pat: &hir::Pat<'_>) {
fn walk_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
debug!("walk_pat(discr_place={:?}, pat={:?})", discr_place, pat);

let tcx = self.tcx();
Expand Down Expand Up @@ -569,7 +570,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
closure_hir_id: hir::HirId,
closure_span: Span,
var_id: hir::HirId,
) -> mc::McResult<mc::Place<'tcx>> {
) -> mc::McResult<mc::PlaceWithHirId<'tcx>> {
// Create the place for the variable being borrowed, from the
// perspective of the creator (parent) of the closure.
let var_ty = self.mc.node_ty(var_id)?;
Expand All @@ -579,7 +580,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {

fn copy_or_move<'a, 'tcx>(
mc: &mc::MemCategorizationContext<'a, 'tcx>,
place: &Place<'tcx>,
place_with_id: &PlaceWithHirId<'tcx>,
) -> ConsumeMode {
if !mc.type_is_copy_modulo_regions(place.ty, place.span) { Move } else { Copy }
if !mc.type_is_copy_modulo_regions(
place_with_id.place.ty,
mc.tcx().hir().span(place_with_id.hir_id),
) {
Move
} else {
Copy
}
}
Loading

0 comments on commit a88182f

Please sign in to comment.