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

Place 2.0 change from enum to struct #60913

Merged
merged 23 commits into from
Jul 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d0accad
Migrate from Place enum to Place struct
spastorino Apr 30, 2019
98d2324
Avoid cloning place in LocalAnalyzer visitor
spastorino Jul 1, 2019
e11adb1
Implement Place::as_place_ref
spastorino Jul 2, 2019
438aeb8
Avoid cloning Place in codegen_place
spastorino Jul 2, 2019
1795318
Remove explicit return from last line of fn
spastorino Jul 3, 2019
ec65db0
Remove explicit lifetime
spastorino Jul 3, 2019
46f81fc
Avoid cloning Place in report_use_of_moved_or_uninitialized and friends
spastorino Jul 11, 2019
75c0c8c
Avoid cloning Place in describe_place_for_conflicting_borrow
spastorino Jul 19, 2019
72251d5
Avoid cloning Place in classify_drop_access_kind
spastorino Jul 19, 2019
34e3b70
Avoid cloning Place in append_place_to_string
spastorino Jul 19, 2019
95e727a
Avoid cloning Place in check_access_permissions
spastorino Jul 19, 2019
09014fc
Avoid cloning Place in report_cannot_move_from_static
spastorino Jul 19, 2019
1047079
Avoid cloning Place in report_cannot_move_from_borrowed_content
spastorino Jul 19, 2019
2ffd3c6
Avoid cloning Place in limit_capture_mutability
spastorino Jul 19, 2019
d77653e
Avoid cloning Place in calculate_fake_borrows
spastorino Jul 19, 2019
9b549d3
Avoid cloning Place in gather_init
spastorino Jul 19, 2019
17a465c
Avoid unneeded else branches
spastorino Jul 19, 2019
7b456df
Avoid cloning Place in is_stable
spastorino Jul 19, 2019
2a7d600
Avoid cloning Place in in_projection_structurally
spastorino Jul 19, 2019
b490032
Avoid cloning Place in assign #1
spastorino Jul 19, 2019
7789cbf
Avoid cloning Place in assign #2
spastorino Jul 19, 2019
b59ded8
Avoid cloning Place in visit_rvalue
spastorino Jul 19, 2019
a8ceeeb
Avoid cloning Place in check_and_patch
spastorino Jul 20, 2019
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
141 changes: 101 additions & 40 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1718,11 +1718,11 @@ impl<'tcx> Debug for Statement<'tcx> {
#[derive(
Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable, HashStable,
)]
pub enum Place<'tcx> {
Base(PlaceBase<'tcx>),
pub struct Place<'tcx> {
pub base: PlaceBase<'tcx>,

/// projection out of a place (access a field, deref a pointer, etc)
Projection(Box<Projection<'tcx>>),
pub projection: Option<Box<Projection<'tcx>>>,
}

#[derive(
Expand Down Expand Up @@ -1761,7 +1761,7 @@ impl_stable_hash_for!(struct Static<'tcx> {
Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable, HashStable,
)]
pub struct Projection<'tcx> {
pub base: Place<'tcx>,
pub base: Option<Box<Projection<'tcx>>>,
Copy link
Member

Choose a reason for hiding this comment

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

It is somewhat confusing to have base sometimes be a Projection and sometimes a PlaceBase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes it is, but the entire Projection type will be gone soon, so I deemed it irrelevant

pub elem: PlaceElem<'tcx>,
}

Expand Down Expand Up @@ -1826,8 +1826,17 @@ newtype_index! {
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct PlaceRef<'a, 'tcx> {
pub base: &'a PlaceBase<'tcx>,
pub projection: &'a Option<Box<Projection<'tcx>>>,
}

impl<'tcx> Place<'tcx> {
pub const RETURN_PLACE: Place<'tcx> = Place::Base(PlaceBase::Local(RETURN_PLACE));
pub const RETURN_PLACE: Place<'tcx> = Place {
base: PlaceBase::Local(RETURN_PLACE),
projection: None,
};

pub fn field(self, f: Field, ty: Ty<'tcx>) -> Place<'tcx> {
self.elem(ProjectionElem::Field(f, ty))
Expand All @@ -1853,7 +1862,10 @@ impl<'tcx> Place<'tcx> {
}

pub fn elem(self, elem: PlaceElem<'tcx>) -> Place<'tcx> {
Place::Projection(Box::new(Projection { base: self, elem }))
Place {
base: self.base,
projection: Some(Box::new(Projection { base: self.projection, elem })),
}
}

/// Finds the innermost `Local` from this `Place`, *if* it is either a local itself or
Expand All @@ -1862,54 +1874,77 @@ impl<'tcx> Place<'tcx> {
// FIXME: can we safely swap the semantics of `fn base_local` below in here instead?
pub fn local_or_deref_local(&self) -> Option<Local> {
match self {
Place::Base(PlaceBase::Local(local))
| Place::Projection(box Projection {
base: Place::Base(PlaceBase::Local(local)),
elem: ProjectionElem::Deref,
}) => Some(*local),
Place {
base: PlaceBase::Local(local),
projection: None,
} |
Place {
base: PlaceBase::Local(local),
projection: Some(box Projection {
base: None,
elem: ProjectionElem::Deref,
}),
} => Some(*local),
_ => None,
}
}

/// Finds the innermost `Local` from this `Place`.
pub fn base_local(&self) -> Option<Local> {
let mut place = self;
loop {
match place {
Place::Projection(proj) => place = &proj.base,
Place::Base(PlaceBase::Static(_)) => return None,
Place::Base(PlaceBase::Local(local)) => return Some(*local),
}
}
}

/// Recursively "iterates" over place components, generating a `PlaceBase` and
/// `Projections` list and invoking `op` with a `ProjectionsIter`.
pub fn iterate<R>(
&self,
op: impl FnOnce(&PlaceBase<'tcx>, ProjectionsIter<'_, 'tcx>) -> R,
) -> R {
self.iterate2(&Projections::Empty, op)
Place::iterate_over(&self.base, &self.projection, op)
}

fn iterate2<R>(
&self,
next: &Projections<'_, 'tcx>,
pub fn iterate_over<R>(
place_base: &PlaceBase<'tcx>,
place_projection: &Option<Box<Projection<'tcx>>>,
op: impl FnOnce(&PlaceBase<'tcx>, ProjectionsIter<'_, 'tcx>) -> R,
) -> R {
match self {
Place::Projection(interior) => {
interior.base.iterate2(&Projections::List { projection: interior, next }, op)
fn iterate_over2<'tcx, R>(
place_base: &PlaceBase<'tcx>,
place_projection: &Option<Box<Projection<'tcx>>>,
next: &Projections<'_, 'tcx>,
op: impl FnOnce(&PlaceBase<'tcx>, ProjectionsIter<'_, 'tcx>) -> R,
) -> R {
match place_projection {
None => {
op(place_base, next.iter())
}

Some(interior) => {
iterate_over2(
place_base,
&interior.base,
&Projections::List {
projection: interior,
next,
},
op,
)
}
}
}

Place::Base(base) => op(base, next.iter()),
iterate_over2(place_base, place_projection, &Projections::Empty, op)
}

pub fn as_place_ref(&self) -> PlaceRef<'_, 'tcx> {
Copy link
Contributor

Choose a reason for hiding this comment

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

if it is deemed too wordy, I'm also fine with using as_ref. We can't use the AsRef trait though as that produces a reference

PlaceRef {
base: &self.base,
projection: &self.projection,
}
}
}

impl From<Local> for Place<'_> {
fn from(local: Local) -> Self {
Place::Base(local.into())
Place {
base: local.into(),
projection: None,
}
}
}

Expand All @@ -1919,6 +1954,36 @@ impl From<Local> for PlaceBase<'_> {
}
}

impl<'a, 'tcx> PlaceRef<'a, 'tcx> {
pub fn iterate<R>(
&self,
op: impl FnOnce(&PlaceBase<'tcx>, ProjectionsIter<'_, 'tcx>) -> R,
) -> R {
Place::iterate_over(self.base, self.projection, op)
}

/// Finds the innermost `Local` from this `Place`, *if* it is either a local itself or
/// a single deref of a local.
//
// FIXME: can we safely swap the semantics of `fn base_local` below in here instead?
pub fn local_or_deref_local(&self) -> Option<Local> {
match self {
PlaceRef {
base: PlaceBase::Local(local),
projection: None,
} |
PlaceRef {
base: PlaceBase::Local(local),
projection: Some(box Projection {
base: None,
elem: ProjectionElem::Deref,
}),
} => Some(*local),
_ => None,
}
}
}

/// A linked list of projections running up the stack; begins with the
/// innermost projection and extends to the outermost (e.g., `a.b.c`
/// would have the place `b` with a "next" pointer to `b.c`).
Expand Down Expand Up @@ -3155,18 +3220,14 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {

impl<'tcx> TypeFoldable<'tcx> for Place<'tcx> {
fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
match self {
&Place::Projection(ref p) => Place::Projection(p.fold_with(folder)),
_ => self.clone(),
Place {
base: self.base.clone(),
projection: self.projection.fold_with(folder),
}
}

fn super_visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> bool {
if let &Place::Projection(ref p) = self {
p.visit_with(visitor)
} else {
false
}
self.projection.visit_with(visitor)
}
}

Expand Down
19 changes: 15 additions & 4 deletions src/librustc/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,15 @@ BraceStructTypeFoldableImpl! {
}

impl<'tcx> Place<'tcx> {
pub fn ty<D>(&self, local_decls: &D, tcx: TyCtxt<'tcx>) -> PlaceTy<'tcx>
where
D: HasLocalDecls<'tcx>,
pub fn ty_from<D>(
base: &PlaceBase<'tcx>,
projection: &Option<Box<Projection<'tcx>>>,
local_decls: &D,
tcx: TyCtxt<'tcx>
) -> PlaceTy<'tcx>
where D: HasLocalDecls<'tcx>
{
self.iterate(|place_base, place_projections| {
Place::iterate_over(base, projection, |place_base, place_projections| {
let mut place_ty = place_base.ty(local_decls);

for proj in place_projections {
Expand All @@ -132,6 +136,13 @@ impl<'tcx> Place<'tcx> {
place_ty
})
}

pub fn ty<D>(&self, local_decls: &D, tcx: TyCtxt<'tcx>) -> PlaceTy<'tcx>
where
D: HasLocalDecls<'tcx>,
{
Place::ty_from(&self.base, &self.projection, local_decls, tcx)
}
}

impl<'tcx> PlaceBase<'tcx> {
Expand Down
37 changes: 20 additions & 17 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,11 @@ macro_rules! make_mir_visitor {
}

fn visit_projection(&mut self,
place_base: & $($mutability)? PlaceBase<'tcx>,
place: & $($mutability)? Projection<'tcx>,
context: PlaceContext,
location: Location) {
self.super_projection(place, context, location);
self.super_projection(place_base, place, context, location);
}

fn visit_constant(&mut self,
Expand Down Expand Up @@ -676,19 +677,20 @@ macro_rules! make_mir_visitor {
place: & $($mutability)? Place<'tcx>,
context: PlaceContext,
location: Location) {
match place {
Place::Base(place_base) => {
self.visit_place_base(place_base, context, location);
}
Place::Projection(proj) => {
let context = if context.is_mutating_use() {
PlaceContext::MutatingUse(MutatingUseContext::Projection)
} else {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
};
let mut context = context;

if place.projection.is_some() {
context = if context.is_mutating_use() {
PlaceContext::MutatingUse(MutatingUseContext::Projection)
} else {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
};
}

self.visit_projection(proj, context, location);
}
self.visit_place_base(& $($mutability)? place.base, context, location);
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
spastorino marked this conversation as resolved.
Show resolved Hide resolved

if let Some(box proj) = & $($mutability)? place.projection {
self.visit_projection(& $($mutability)? place.base, proj, context, location);
}
}

Expand All @@ -707,13 +709,14 @@ macro_rules! make_mir_visitor {
}

fn super_projection(&mut self,
place_base: & $($mutability)? PlaceBase<'tcx>,
proj: & $($mutability)? Projection<'tcx>,
context: PlaceContext,
location: Location) {
// this is calling `super_place` in preparation for changing `Place` to be
// a struct with a base and a slice of projections. `visit_place` should only ever
// be called for the outermost place now.
self.super_place(& $($mutability)? proj.base, context, location);
if let Some(box proj_base) = & $($mutability)? proj.base {
self.visit_projection(place_base, proj_base, context, location);
}

match & $($mutability)? proj.elem {
ProjectionElem::Deref => {
}
Expand Down
Loading