-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
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 98d2324
Avoid cloning place in LocalAnalyzer visitor
spastorino e11adb1
Implement Place::as_place_ref
spastorino 438aeb8
Avoid cloning Place in codegen_place
spastorino 1795318
Remove explicit return from last line of fn
spastorino ec65db0
Remove explicit lifetime
spastorino 46f81fc
Avoid cloning Place in report_use_of_moved_or_uninitialized and friends
spastorino 75c0c8c
Avoid cloning Place in describe_place_for_conflicting_borrow
spastorino 72251d5
Avoid cloning Place in classify_drop_access_kind
spastorino 34e3b70
Avoid cloning Place in append_place_to_string
spastorino 95e727a
Avoid cloning Place in check_access_permissions
spastorino 09014fc
Avoid cloning Place in report_cannot_move_from_static
spastorino 1047079
Avoid cloning Place in report_cannot_move_from_borrowed_content
spastorino 2ffd3c6
Avoid cloning Place in limit_capture_mutability
spastorino d77653e
Avoid cloning Place in calculate_fake_borrows
spastorino 9b549d3
Avoid cloning Place in gather_init
spastorino 17a465c
Avoid unneeded else branches
spastorino 7b456df
Avoid cloning Place in is_stable
spastorino 2a7d600
Avoid cloning Place in in_projection_structurally
spastorino b490032
Avoid cloning Place in assign #1
spastorino 7789cbf
Avoid cloning Place in assign #2
spastorino b59ded8
Avoid cloning Place in visit_rvalue
spastorino a8ceeeb
Avoid cloning Place in check_and_patch
spastorino File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
|
@@ -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>>>, | ||
pub elem: PlaceElem<'tcx>, | ||
} | ||
|
||
|
@@ -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)) | ||
|
@@ -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 | ||
|
@@ -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> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if it is deemed too wordy, I'm also fine with using |
||
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, | ||
} | ||
} | ||
} | ||
|
||
|
@@ -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`). | ||
|
@@ -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) | ||
} | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 aProjection
and sometimes aPlaceBase
.There was a problem hiding this comment.
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