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.

Sign up for GitHub

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

Jump to bottom

Use PlaceRef projection abstractions more consistently in rustc_mir #80865

Merged
merged 1 commit into from
Jan 18, 2021
Merged

Use PlaceRef projection abstractions more consistently in rustc_mir #80865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 6 additions & 10 deletions compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1604,28 +1604,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

fn classify_drop_access_kind(&self, place: PlaceRef<'tcx>) -> StorageDeadOrDrop<'tcx> {
let tcx = self.infcx.tcx;
match place.projection {
[] => StorageDeadOrDrop::LocalStorageDead,
[proj_base @ .., elem] => {
match place.last_projection() {
None => StorageDeadOrDrop::LocalStorageDead,
Some((place_base, elem)) => {
// FIXME(spastorino) make this iterate
let base_access = self.classify_drop_access_kind(PlaceRef {
local: place.local,
projection: proj_base,
});
let base_access = self.classify_drop_access_kind(place_base);
match elem {
ProjectionElem::Deref => match base_access {
StorageDeadOrDrop::LocalStorageDead
| StorageDeadOrDrop::BoxedStorageDead => {
assert!(
Place::ty_from(place.local, proj_base, self.body, tcx).ty.is_box(),
place_base.ty(self.body, tcx).ty.is_box(),
"Drop of value behind a reference or raw pointer"
);
StorageDeadOrDrop::BoxedStorageDead
}
StorageDeadOrDrop::Destructor(_) => base_access,
},
ProjectionElem::Field(..) | ProjectionElem::Downcast(..) => {
let base_ty = Place::ty_from(place.local, proj_base, self.body, tcx).ty;
let base_ty = place_base.ty(self.body, tcx).ty;
match base_ty.kind() {
ty::Adt(def, _) if def.has_dtor(tcx) => {
// Report the outermost adt with a destructor
Expand All @@ -1640,7 +1637,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
_ => base_access,
}
}

ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Index(_) => base_access,
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
self.describe_field(PlaceRef { local, projection: proj_base }, field)
}
ProjectionElem::Downcast(_, variant_index) => {
let base_ty =
Place::ty_from(place.local, place.projection, self.body, self.infcx.tcx).ty;
let base_ty = place.ty(self.body, self.infcx.tcx).ty;
self.describe_field_from_ty(&base_ty, field, Some(*variant_index))
}
ProjectionElem::Field(_, field_type) => {
Expand Down Expand Up @@ -473,7 +472,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

// If we didn't find an overloaded deref or index, then assume it's a
// built in deref and check the type of the base.
let base_ty = Place::ty_from(deref_base.local, deref_base.projection, self.body, tcx).ty;
let base_ty = deref_base.ty(self.body, tcx).ty;
if base_ty.is_unsafe_ptr() {
BorrowedContentSource::DerefRawPointer
} else if base_ty.is_mutable_ptr() {
Expand Down
16 changes: 7 additions & 9 deletions compiler/rustc_mir/src/borrow_check/path_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,27 +147,25 @@ pub(crate) fn is_upvar_field_projection(
place_ref: PlaceRef<'tcx>,
body: &Body<'tcx>,
) -> Option<Field> {
let mut place_projection = place_ref.projection;
let mut place_ref = place_ref;
let mut by_ref = false;

if let [proj_base @ .., ProjectionElem::Deref] = place_projection {
place_projection = proj_base;
if let Some((place_base, ProjectionElem::Deref)) = place_ref.last_projection() {
place_ref = place_base;
by_ref = true;
}

match place_projection {
[base @ .., ProjectionElem::Field(field, _ty)] => {
let base_ty = Place::ty_from(place_ref.local, base, body, tcx).ty;

match place_ref.last_projection() {
Some((place_base, ProjectionElem::Field(field, _ty))) => {
let base_ty = place_base.ty(body, tcx).ty;
if (base_ty.is_closure() || base_ty.is_generator())
&& (!by_ref || upvars[field.index()].by_ref)
{
Some(*field)
Some(field)
} else {
None
}
}

_ => None,
}
}
10 changes: 3 additions & 7 deletions compiler/rustc_mir/src/dataflow/move_paths/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,14 +518,10 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {

// Check if we are assigning into a field of a union, if so, lookup the place
// of the union so it is marked as initialized again.
if let [proj_base @ .., ProjectionElem::Field(_, _)] = place.projection {
if let ty::Adt(def, _) =
Place::ty_from(place.local, proj_base, self.builder.body, self.builder.tcx)
.ty
.kind()
{
if let Some((place_base, ProjectionElem::Field(_, _))) = place.last_projection() {
if let ty::Adt(def, _) = place_base.ty(self.builder.body, self.builder.tcx).ty.kind() {
if def.is_union() {
place = PlaceRef { local: place.local, projection: proj_base }
place = place_base;
}
}
}
Expand Down
24 changes: 10 additions & 14 deletions compiler/rustc_mir/src/transform/check_consts/qualifs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,10 @@ where

Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => {
// Special-case reborrows to be more like a copy of the reference.
if let &[ref proj_base @ .., ProjectionElem::Deref] = place.projection.as_ref() {
let base_ty = Place::ty_from(place.local, proj_base, cx.body, cx.tcx).ty;
if let Some((place_base, ProjectionElem::Deref)) = place.as_ref().last_projection() {
let base_ty = place_base.ty(cx.body, cx.tcx).ty;
if let ty::Ref(..) = base_ty.kind() {
return in_place::<Q, _>(
cx,
in_local,
PlaceRef { local: place.local, projection: proj_base },
);
return in_place::<Q, _>(cx, in_local, place_base);
}
}

Expand Down Expand Up @@ -209,9 +205,9 @@ where
Q: Qualif,
F: FnMut(Local) -> bool,
{
let mut projection = place.projection;
while let &[ref proj_base @ .., proj_elem] = projection {
match proj_elem {
let mut place = place;
while let Some((place_base, elem)) = place.last_projection() {
match elem {
ProjectionElem::Index(index) if in_local(index) => return true,

ProjectionElem::Deref
Expand All @@ -222,16 +218,16 @@ where
| ProjectionElem::Index(_) => {}
}

let base_ty = Place::ty_from(place.local, proj_base, cx.body, cx.tcx);
let proj_ty = base_ty.projection_ty(cx.tcx, proj_elem).ty;
let base_ty = place_base.ty(cx.body, cx.tcx);
let proj_ty = base_ty.projection_ty(cx.tcx, elem).ty;
if !Q::in_any_value_of_ty(cx, proj_ty) {
return false;
}

projection = proj_base;
place = place_base;
}

assert!(projection.is_empty());
assert!(place.projection.is_empty());
in_local(place.local)
}

Expand Down
39 changes: 19 additions & 20 deletions compiler/rustc_mir/src/transform/check_consts/validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1007,27 +1007,26 @@ fn place_as_reborrow(
body: &Body<'tcx>,
place: Place<'tcx>,
) -> Option<&'a [PlaceElem<'tcx>]> {
Copy link
Member

Choose a reason for hiding this comment

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

This looks to me like the return type should be changed to Option<PlaceRef>... but that's a larger change, no need to do it in this PR.

place.projection.split_last().and_then(|(outermost, inner)| {
if outermost != &ProjectionElem::Deref {
return None;
}

// A borrow of a `static` also looks like `&(*_1)` in the MIR, but `_1` is a `const`
// that points to the allocation for the static. Don't treat these as reborrows.
oliviacrain marked this conversation as resolved.
Show resolved Hide resolved
if body.local_decls[place.local].is_ref_to_static() {
return None;
}

// Ensure the type being derefed is a reference and not a raw pointer.
//
// This is sufficient to prevent an access to a `static mut` from being marked as a
// reborrow, even if the check above were to disappear.
let inner_ty = Place::ty_from(place.local, inner, body, tcx).ty;
match inner_ty.kind() {
ty::Ref(..) => Some(inner),
_ => None,
match place.as_ref().last_projection() {
Some((place_base, ProjectionElem::Deref)) => {
// A borrow of a `static` also looks like `&(*_1)` in the MIR, but `_1` is a `const`
// that points to the allocation for the static. Don't treat these as reborrows.
if body.local_decls[place_base.local].is_ref_to_static() {
None
} else {
// Ensure the type being derefed is a reference and not a raw pointer.
//
// This is sufficient to prevent an access to a `static mut` from being marked as a
// reborrow, even if the check above were to disappear.
let inner_ty = place_base.ty(body, tcx).ty;
match inner_ty.kind() {
ty::Ref(..) => Some(place_base.projection),
_ => None,
}
}
}
})
_ => None,
}
}

fn is_int_bool_or_char(ty: Ty<'_>) -> bool {
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_mir/src/transform/check_unsafety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,17 +407,13 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
place: Place<'tcx>,
is_mut_use: bool,
) {
let mut cursor = place.projection.as_ref();
while let &[ref proj_base @ .., elem] = cursor {
cursor = proj_base;

for (place_base, elem) in place.iter_projections().rev() {
match elem {
// Modifications behind a dereference don't affect the value of
// the pointer.
ProjectionElem::Deref => return,
ProjectionElem::Field(..) => {
let ty =
Place::ty_from(place.local, proj_base, &self.body.local_decls, self.tcx).ty;
let ty = place_base.ty(&self.body.local_decls, self.tcx).ty;
if let ty::Adt(def, _) = ty.kind() {
if self.tcx.layout_scalar_valid_range(def.did)
!= (Bound::Unbounded, Bound::Unbounded)
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_mir/src/transform/instcombine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,11 +277,9 @@ impl OptimizationFinder<'b, 'tcx> {
impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
if let Rvalue::Ref(_, _, place) = rvalue {
if let PlaceRef { local, projection: &[ref proj_base @ .., ProjectionElem::Deref] } =
place.as_ref()
{
if let Some((place_base, ProjectionElem::Deref)) = place.as_ref().last_projection() {
// The dereferenced place must have type `&_`.
let ty = Place::ty_from(local, proj_base, self.body, self.tcx).ty;
let ty = place_base.ty(self.body, self.tcx).ty;
if let ty::Ref(_, _, Mutability::Not) = ty.kind() {
self.optimizations.and_stars.insert(location);
}
Expand Down
96 changes: 50 additions & 46 deletions compiler/rustc_mir/src/transform/promote_consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,43 +445,50 @@ impl<'tcx> Validator<'_, 'tcx> {
}

fn validate_place(&self, place: PlaceRef<'tcx>) -> Result<(), Unpromotable> {
match place {
PlaceRef { local, projection: [] } => self.validate_local(local),
PlaceRef { local, projection: [proj_base @ .., elem] } => {
match place.last_projection() {
None => self.validate_local(place.local),
Some((place_base, elem)) => {
// Validate topmost projection, then recurse.
match *elem {
match elem {
ProjectionElem::Deref => {
let mut promotable = false;
// This is a special treatment for cases like *&STATIC where STATIC is a
// global static variable.
// This pattern is generated only when global static variables are directly
// accessed and is qualified for promotion safely.
if let TempState::Defined { location, .. } = self.temps[local] {
let def_stmt =
self.body[location.block].statements.get(location.statement_index);
if let Some(Statement {
kind:
StatementKind::Assign(box (_, Rvalue::Use(Operand::Constant(c)))),
..
}) = def_stmt
// The `is_empty` predicate is introduced to exclude the case
// where the projection operations are [ .field, * ].
// The reason is because promotion will be illegal if field
// accesses precede the dereferencing.
// Discussion can be found at
// https://github.com/rust-lang/rust/pull/74945#discussion_r463063247
// There may be opportunity for generalization, but this needs to be
// accounted for.
if place_base.projection.is_empty() {
// This is a special treatment for cases like *&STATIC where STATIC is a
// global static variable.
// This pattern is generated only when global static variables are directly
// accessed and is qualified for promotion safely.
if let TempState::Defined { location, .. } =
self.temps[place_base.local]
{
if let Some(did) = c.check_static_ptr(self.tcx) {
// Evaluating a promoted may not read statics except if it got
// promoted from a static (this is a CTFE check). So we
// can only promote static accesses inside statics.
if let Some(hir::ConstContext::Static(..)) = self.const_kind {
// The `is_empty` predicate is introduced to exclude the case
// where the projection operations are [ .field, * ].
// The reason is because promotion will be illegal if field
// accesses precede the dereferencing.
// Discussion can be found at
// https://github.com/rust-lang/rust/pull/74945#discussion_r463063247
// There may be opportunity for generalization, but this needs to be
// accounted for.
if proj_base.is_empty()
&& !self.tcx.is_thread_local_static(did)
let def_stmt = self.body[location.block]
.statements
.get(location.statement_index);
if let Some(Statement {
kind:
StatementKind::Assign(box (
_,
Rvalue::Use(Operand::Constant(c)),
)),
..
}) = def_stmt
{
if let Some(did) = c.check_static_ptr(self.tcx) {
// Evaluating a promoted may not read statics except if it got
// promoted from a static (this is a CTFE check). So we
// can only promote static accesses inside statics.
if let Some(hir::ConstContext::Static(..)) = self.const_kind
{
promotable = true;
if !self.tcx.is_thread_local_static(did) {
promotable = true;
}
}
}
}
Expand All @@ -502,8 +509,7 @@ impl<'tcx> Validator<'_, 'tcx> {
}

ProjectionElem::Field(..) => {
let base_ty =
Place::ty_from(place.local, proj_base, self.body, self.tcx).ty;
let base_ty = place_base.ty(self.body, self.tcx).ty;
if let Some(def) = base_ty.ty_adt_def() {
// No promotion of union field accesses.
if def.is_union() {
Expand All @@ -513,7 +519,7 @@ impl<'tcx> Validator<'_, 'tcx> {
}
}

self.validate_place(PlaceRef { local: place.local, projection: proj_base })
self.validate_place(place_base)
}
}
}
Expand Down Expand Up @@ -660,13 +666,11 @@ impl<'tcx> Validator<'_, 'tcx> {
Rvalue::AddressOf(_, place) => {
// We accept `&raw *`, i.e., raw reborrows -- creating a raw pointer is
// no problem, only using it is.
if let [proj_base @ .., ProjectionElem::Deref] = place.projection.as_ref() {
let base_ty = Place::ty_from(place.local, proj_base, self.body, self.tcx).ty;
if let Some((place_base, ProjectionElem::Deref)) = place.as_ref().last_projection()
{
let base_ty = place_base.ty(self.body, self.tcx).ty;
if let ty::Ref(..) = base_ty.kind() {
return self.validate_place(PlaceRef {
local: place.local,
projection: proj_base,
});
return self.validate_place(place_base);
}
}
return Err(Unpromotable);
Expand All @@ -675,12 +679,12 @@ impl<'tcx> Validator<'_, 'tcx> {
Rvalue::Ref(_, kind, place) => {
// Special-case reborrows to be more like a copy of the reference.
let mut place_simplified = place.as_ref();
if let [proj_base @ .., ProjectionElem::Deref] = &place_simplified.projection {
let base_ty =
Place::ty_from(place_simplified.local, proj_base, self.body, self.tcx).ty;
if let Some((place_base, ProjectionElem::Deref)) =
place_simplified.last_projection()
{
let base_ty = place_base.ty(self.body, self.tcx).ty;
if let ty::Ref(..) = base_ty.kind() {
place_simplified =
PlaceRef { local: place_simplified.local, projection: proj_base };
place_simplified = place_base;
}
}

Expand Down