Skip to content

Always use lazy qualif getters during const-checking #68804

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

Merged
merged 4 commits into from
Feb 4, 2020
Merged

Always use lazy qualif getters during const-checking #68804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49ef3da
Take `FnMut` instead of `Fn` in `Qualif` methods
ecstatic-morse Feb 3, 2020
4c5ca44
Pass correct closure type to `Qualif` methods
ecstatic-morse Feb 3, 2020
110a7e2
Eliminate "eager" qualif getter
ecstatic-morse Feb 3, 2020
21a040e
Treat `Rvalue::AddressOf` the same as `Rvalue::Ref`
ecstatic-morse Feb 3, 2020
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
18 changes: 9 additions & 9 deletions src/librustc_mir/transform/check_consts/qualifs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub trait Qualif {

fn in_projection_structurally(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
place: PlaceRef<'_, 'tcx>,
) -> bool {
if let [proj_base @ .., elem] = place.projection {
Expand Down Expand Up @@ -66,15 +66,15 @@ pub trait Qualif {

fn in_projection(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
place: PlaceRef<'_, 'tcx>,
) -> bool {
Self::in_projection_structurally(cx, per_local, place)
}

fn in_place(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
place: PlaceRef<'_, 'tcx>,
) -> bool {
match place {
Expand All @@ -85,7 +85,7 @@ pub trait Qualif {

fn in_operand(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
operand: &Operand<'tcx>,
) -> bool {
match *operand {
Expand Down Expand Up @@ -126,7 +126,7 @@ pub trait Qualif {

fn in_rvalue_structurally(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
match *rvalue {
Expand Down Expand Up @@ -170,15 +170,15 @@ pub trait Qualif {

fn in_rvalue(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
Self::in_rvalue_structurally(cx, per_local, rvalue)
}

fn in_call(
cx: &ConstCx<'_, 'tcx>,
_per_local: &impl Fn(Local) -> bool,
_per_local: &mut impl FnMut(Local) -> bool,
_callee: &Operand<'tcx>,
_args: &[Operand<'tcx>],
return_ty: Ty<'tcx>,
Expand Down Expand Up @@ -208,7 +208,7 @@ impl Qualif for HasMutInterior {

fn in_rvalue(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
match *rvalue {
Expand Down Expand Up @@ -249,7 +249,7 @@ impl Qualif for NeedsDrop {

fn in_rvalue(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
per_local: &mut impl FnMut(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
if let Rvalue::Aggregate(ref kind, _) = *rvalue {
Expand Down
14 changes: 10 additions & 4 deletions src/librustc_mir/transform/check_consts/resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,13 @@ where
return_place: &mir::Place<'tcx>,
) {
let return_ty = return_place.ty(*self.item.body, self.item.tcx).ty;
let qualif =
Q::in_call(self.item, &|l| self.qualifs_per_local.contains(l), func, args, return_ty);
let qualif = Q::in_call(
self.item,
&mut |l| self.qualifs_per_local.contains(l),
func,
args,
return_ty,
);
if !return_place.is_indirect() {
self.assign_qualif_direct(return_place, qualif);
}
Expand Down Expand Up @@ -105,7 +110,7 @@ where
rvalue: &mir::Rvalue<'tcx>,
location: Location,
) {
let qualif = Q::in_rvalue(self.item, &|l| self.qualifs_per_local.contains(l), rvalue);
let qualif = Q::in_rvalue(self.item, &mut |l| self.qualifs_per_local.contains(l), rvalue);
if !place.is_indirect() {
self.assign_qualif_direct(place, qualif);
}
Expand All @@ -120,7 +125,8 @@ where
// here; that occurs in `apply_call_return_effect`.

if let mir::TerminatorKind::DropAndReplace { value, location: dest, .. } = kind {
let qualif = Q::in_operand(self.item, &|l| self.qualifs_per_local.contains(l), value);
let qualif =
Q::in_operand(self.item, &mut |l| self.qualifs_per_local.contains(l), value);
if !dest.is_indirect() {
self.assign_qualif_direct(dest, qualif);
}
Expand Down
55 changes: 16 additions & 39 deletions src/librustc_mir/transform/check_consts/validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl Qualifs<'a, 'mir, 'tcx> {
/// Returns `true` if `local` is `NeedsDrop` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary
fn needs_drop_lazy_seek(&mut self, local: Local, location: Location) -> bool {
fn needs_drop(&mut self, local: Local, location: Location) -> bool {
if !self.needs_drop.in_any_value_of_ty.contains(local) {
return false;
}
Expand All @@ -76,7 +76,7 @@ impl Qualifs<'a, 'mir, 'tcx> {
/// Returns `true` if `local` is `HasMutInterior` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary.
fn has_mut_interior_lazy_seek(&mut self, local: Local, location: Location) -> bool {
fn has_mut_interior(&mut self, local: Local, location: Location) -> bool {
if !self.has_mut_interior.in_any_value_of_ty.contains(local) {
return false;
}
Expand All @@ -86,17 +86,6 @@ impl Qualifs<'a, 'mir, 'tcx> {
|| self.indirectly_mutable(local, location)
}

/// Returns `true` if `local` is `HasMutInterior`, but requires the `has_mut_interior` and
/// `indirectly_mutable` cursors to be updated beforehand.
fn has_mut_interior_eager_seek(&self, local: Local) -> bool {
if !self.has_mut_interior.in_any_value_of_ty.contains(local) {
return false;
}

self.has_mut_interior.cursor.get().contains(local)
|| self.indirectly_mutable.get().contains(local)
}

fn in_return_place(&mut self, item: &Item<'_, 'tcx>) -> ConstQualifs {
// Find the `Return` terminator if one exists.
//
Expand All @@ -120,8 +109,8 @@ impl Qualifs<'a, 'mir, 'tcx> {
let return_loc = item.body.terminator_loc(return_block);

ConstQualifs {
needs_drop: self.needs_drop_lazy_seek(RETURN_PLACE, return_loc),
has_mut_interior: self.has_mut_interior_lazy_seek(RETURN_PLACE, return_loc),
needs_drop: self.needs_drop(RETURN_PLACE, return_loc),
has_mut_interior: self.has_mut_interior(RETURN_PLACE, return_loc),
}
}
}
Expand Down Expand Up @@ -244,23 +233,6 @@ impl Validator<'a, 'mir, 'tcx> {
self.check_op_spanned(ops::StaticAccess, span)
}
}

fn check_immutable_borrow_like(&mut self, location: Location, place: &Place<'tcx>) {
// FIXME: Change the `in_*` methods to take a `FnMut` so we don't have to manually
// seek the cursors beforehand.
self.qualifs.has_mut_interior.cursor.seek_before(location);
self.qualifs.indirectly_mutable.seek(location);

let borrowed_place_has_mut_interior = HasMutInterior::in_place(
&self.item,
&|local| self.qualifs.has_mut_interior_eager_seek(local),
place.as_ref(),
);

if borrowed_place_has_mut_interior {
self.check_op(ops::CellBorrow);
}
}
}

impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
Expand Down Expand Up @@ -366,12 +338,17 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
Rvalue::AddressOf(Mutability::Mut, _) => self.check_op(ops::MutAddressOf),

Rvalue::Ref(_, BorrowKind::Shared, ref place)
| Rvalue::Ref(_, BorrowKind::Shallow, ref place) => {
self.check_immutable_borrow_like(location, place)
}

Rvalue::AddressOf(Mutability::Not, ref place) => {
self.check_immutable_borrow_like(location, place)
| Rvalue::Ref(_, BorrowKind::Shallow, ref place)
| Rvalue::AddressOf(Mutability::Not, ref place) => {
let borrowed_place_has_mut_interior = HasMutInterior::in_place(
&self.item,
&mut |local| self.qualifs.has_mut_interior(local, location),
place.as_ref(),
);

if borrowed_place_has_mut_interior {
self.check_op(ops::CellBorrow);
}
}

Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) => {
Expand Down Expand Up @@ -571,7 +548,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
let needs_drop = if let Some(local) = dropped_place.as_local() {
// Use the span where the local was declared as the span of the drop error.
err_span = self.body.local_decls[local].source_info.span;
self.qualifs.needs_drop_lazy_seek(local, location)
self.qualifs.needs_drop(local, location)
} else {
true
};
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/promote_consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ impl<'tcx> Validator<'_, 'tcx> {

// FIXME(eddyb) maybe cache this?
fn qualif_local<Q: qualifs::Qualif>(&self, local: Local) -> bool {
let per_local = &|l| self.qualif_local::<Q>(l);
let per_local = &mut |l| self.qualif_local::<Q>(l);

if let TempState::Defined { location: loc, .. } = self.temps[local] {
let num_stmts = self.body[loc.block].statements.len();
Expand Down