Skip to content

Commit bae3d0d

Browse files
committed
Auto merge of #68804 - ecstatic-morse:qualif-cursor-lazy, r=estebank
Always use lazy qualif getters during const-checking `has_mut_interior_eager_seek` was needed to work around an overly restrictive bound on the `per_local` argument to the `Qualif` trait. This PR makes that bound `FnMut` instead of `Fn` so we can seek cursors inside of it, resolving a FIXME in the const-checking code.
2 parents fc07615 + 21a040e commit bae3d0d

File tree

4 files changed

+36
-53
lines changed

4 files changed

+36
-53
lines changed

src/librustc_mir/transform/check_consts/qualifs.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ pub trait Qualif {
3434

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

6767
fn in_projection(
6868
cx: &ConstCx<'_, 'tcx>,
69-
per_local: &impl Fn(Local) -> bool,
69+
per_local: &mut impl FnMut(Local) -> bool,
7070
place: PlaceRef<'_, 'tcx>,
7171
) -> bool {
7272
Self::in_projection_structurally(cx, per_local, place)
7373
}
7474

7575
fn in_place(
7676
cx: &ConstCx<'_, 'tcx>,
77-
per_local: &impl Fn(Local) -> bool,
77+
per_local: &mut impl FnMut(Local) -> bool,
7878
place: PlaceRef<'_, 'tcx>,
7979
) -> bool {
8080
match place {
@@ -85,7 +85,7 @@ pub trait Qualif {
8585

8686
fn in_operand(
8787
cx: &ConstCx<'_, 'tcx>,
88-
per_local: &impl Fn(Local) -> bool,
88+
per_local: &mut impl FnMut(Local) -> bool,
8989
operand: &Operand<'tcx>,
9090
) -> bool {
9191
match *operand {
@@ -126,7 +126,7 @@ pub trait Qualif {
126126

127127
fn in_rvalue_structurally(
128128
cx: &ConstCx<'_, 'tcx>,
129-
per_local: &impl Fn(Local) -> bool,
129+
per_local: &mut impl FnMut(Local) -> bool,
130130
rvalue: &Rvalue<'tcx>,
131131
) -> bool {
132132
match *rvalue {
@@ -170,15 +170,15 @@ pub trait Qualif {
170170

171171
fn in_rvalue(
172172
cx: &ConstCx<'_, 'tcx>,
173-
per_local: &impl Fn(Local) -> bool,
173+
per_local: &mut impl FnMut(Local) -> bool,
174174
rvalue: &Rvalue<'tcx>,
175175
) -> bool {
176176
Self::in_rvalue_structurally(cx, per_local, rvalue)
177177
}
178178

179179
fn in_call(
180180
cx: &ConstCx<'_, 'tcx>,
181-
_per_local: &impl Fn(Local) -> bool,
181+
_per_local: &mut impl FnMut(Local) -> bool,
182182
_callee: &Operand<'tcx>,
183183
_args: &[Operand<'tcx>],
184184
return_ty: Ty<'tcx>,
@@ -208,7 +208,7 @@ impl Qualif for HasMutInterior {
208208

209209
fn in_rvalue(
210210
cx: &ConstCx<'_, 'tcx>,
211-
per_local: &impl Fn(Local) -> bool,
211+
per_local: &mut impl FnMut(Local) -> bool,
212212
rvalue: &Rvalue<'tcx>,
213213
) -> bool {
214214
match *rvalue {
@@ -249,7 +249,7 @@ impl Qualif for NeedsDrop {
249249

250250
fn in_rvalue(
251251
cx: &ConstCx<'_, 'tcx>,
252-
per_local: &impl Fn(Local) -> bool,
252+
per_local: &mut impl FnMut(Local) -> bool,
253253
rvalue: &Rvalue<'tcx>,
254254
) -> bool {
255255
if let Rvalue::Aggregate(ref kind, _) = *rvalue {

src/librustc_mir/transform/check_consts/resolver.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,13 @@ where
7171
return_place: &mir::Place<'tcx>,
7272
) {
7373
let return_ty = return_place.ty(*self.item.body, self.item.tcx).ty;
74-
let qualif =
75-
Q::in_call(self.item, &|l| self.qualifs_per_local.contains(l), func, args, return_ty);
74+
let qualif = Q::in_call(
75+
self.item,
76+
&mut |l| self.qualifs_per_local.contains(l),
77+
func,
78+
args,
79+
return_ty,
80+
);
7681
if !return_place.is_indirect() {
7782
self.assign_qualif_direct(return_place, qualif);
7883
}
@@ -105,7 +110,7 @@ where
105110
rvalue: &mir::Rvalue<'tcx>,
106111
location: Location,
107112
) {
108-
let qualif = Q::in_rvalue(self.item, &|l| self.qualifs_per_local.contains(l), rvalue);
113+
let qualif = Q::in_rvalue(self.item, &mut |l| self.qualifs_per_local.contains(l), rvalue);
109114
if !place.is_indirect() {
110115
self.assign_qualif_direct(place, qualif);
111116
}
@@ -120,7 +125,8 @@ where
120125
// here; that occurs in `apply_call_return_effect`.
121126

122127
if let mir::TerminatorKind::DropAndReplace { value, location: dest, .. } = kind {
123-
let qualif = Q::in_operand(self.item, &|l| self.qualifs_per_local.contains(l), value);
128+
let qualif =
129+
Q::in_operand(self.item, &mut |l| self.qualifs_per_local.contains(l), value);
124130
if !dest.is_indirect() {
125131
self.assign_qualif_direct(dest, qualif);
126132
}

src/librustc_mir/transform/check_consts/validation.rs

+16-39
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ impl Qualifs<'a, 'mir, 'tcx> {
6464
/// Returns `true` if `local` is `NeedsDrop` at the given `Location`.
6565
///
6666
/// Only updates the cursor if absolutely necessary
67-
fn needs_drop_lazy_seek(&mut self, local: Local, location: Location) -> bool {
67+
fn needs_drop(&mut self, local: Local, location: Location) -> bool {
6868
if !self.needs_drop.in_any_value_of_ty.contains(local) {
6969
return false;
7070
}
@@ -76,7 +76,7 @@ impl Qualifs<'a, 'mir, 'tcx> {
7676
/// Returns `true` if `local` is `HasMutInterior` at the given `Location`.
7777
///
7878
/// Only updates the cursor if absolutely necessary.
79-
fn has_mut_interior_lazy_seek(&mut self, local: Local, location: Location) -> bool {
79+
fn has_mut_interior(&mut self, local: Local, location: Location) -> bool {
8080
if !self.has_mut_interior.in_any_value_of_ty.contains(local) {
8181
return false;
8282
}
@@ -86,17 +86,6 @@ impl Qualifs<'a, 'mir, 'tcx> {
8686
|| self.indirectly_mutable(local, location)
8787
}
8888

89-
/// Returns `true` if `local` is `HasMutInterior`, but requires the `has_mut_interior` and
90-
/// `indirectly_mutable` cursors to be updated beforehand.
91-
fn has_mut_interior_eager_seek(&self, local: Local) -> bool {
92-
if !self.has_mut_interior.in_any_value_of_ty.contains(local) {
93-
return false;
94-
}
95-
96-
self.has_mut_interior.cursor.get().contains(local)
97-
|| self.indirectly_mutable.get().contains(local)
98-
}
99-
10089
fn in_return_place(&mut self, item: &Item<'_, 'tcx>) -> ConstQualifs {
10190
// Find the `Return` terminator if one exists.
10291
//
@@ -120,8 +109,8 @@ impl Qualifs<'a, 'mir, 'tcx> {
120109
let return_loc = item.body.terminator_loc(return_block);
121110

122111
ConstQualifs {
123-
needs_drop: self.needs_drop_lazy_seek(RETURN_PLACE, return_loc),
124-
has_mut_interior: self.has_mut_interior_lazy_seek(RETURN_PLACE, return_loc),
112+
needs_drop: self.needs_drop(RETURN_PLACE, return_loc),
113+
has_mut_interior: self.has_mut_interior(RETURN_PLACE, return_loc),
125114
}
126115
}
127116
}
@@ -244,23 +233,6 @@ impl Validator<'a, 'mir, 'tcx> {
244233
self.check_op_spanned(ops::StaticAccess, span)
245234
}
246235
}
247-
248-
fn check_immutable_borrow_like(&mut self, location: Location, place: &Place<'tcx>) {
249-
// FIXME: Change the `in_*` methods to take a `FnMut` so we don't have to manually
250-
// seek the cursors beforehand.
251-
self.qualifs.has_mut_interior.cursor.seek_before(location);
252-
self.qualifs.indirectly_mutable.seek(location);
253-
254-
let borrowed_place_has_mut_interior = HasMutInterior::in_place(
255-
&self.item,
256-
&|local| self.qualifs.has_mut_interior_eager_seek(local),
257-
place.as_ref(),
258-
);
259-
260-
if borrowed_place_has_mut_interior {
261-
self.check_op(ops::CellBorrow);
262-
}
263-
}
264236
}
265237

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

368340
Rvalue::Ref(_, BorrowKind::Shared, ref place)
369-
| Rvalue::Ref(_, BorrowKind::Shallow, ref place) => {
370-
self.check_immutable_borrow_like(location, place)
371-
}
372-
373-
Rvalue::AddressOf(Mutability::Not, ref place) => {
374-
self.check_immutable_borrow_like(location, place)
341+
| Rvalue::Ref(_, BorrowKind::Shallow, ref place)
342+
| Rvalue::AddressOf(Mutability::Not, ref place) => {
343+
let borrowed_place_has_mut_interior = HasMutInterior::in_place(
344+
&self.item,
345+
&mut |local| self.qualifs.has_mut_interior(local, location),
346+
place.as_ref(),
347+
);
348+
349+
if borrowed_place_has_mut_interior {
350+
self.check_op(ops::CellBorrow);
351+
}
375352
}
376353

377354
Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) => {
@@ -571,7 +548,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
571548
let needs_drop = if let Some(local) = dropped_place.as_local() {
572549
// Use the span where the local was declared as the span of the drop error.
573550
err_span = self.body.local_decls[local].source_info.span;
574-
self.qualifs.needs_drop_lazy_seek(local, location)
551+
self.qualifs.needs_drop(local, location)
575552
} else {
576553
true
577554
};

src/librustc_mir/transform/promote_consts.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ impl<'tcx> Validator<'_, 'tcx> {
407407

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

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

0 commit comments

Comments
 (0)