Skip to content

Commit

Permalink
Rollup merge of rust-lang#71330 - ecstatic-morse:const-qualif-lazy, r…
Browse files Browse the repository at this point in the history
…=oli-obk

Only run dataflow for const qualification if type-based check would fail

This is the optimization discussed in rust-lang#49146 (comment). We wait for `Qualif::in_any_value_of_ty` to return `true` before running dataflow. For bodies that deal mostly with primitive types, this will avoid running dataflow at all during const qualification.

This also removes the `BitSet` used to cache `in_any_value_of_ty` for each local, which was only necessary for an old version of rust-lang#64470 that also handled promotability.
  • Loading branch information
Dylan-DPC committed Apr 24, 2020
2 parents 2e2080d + 15f95b1 commit f3331cb
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 99 deletions.
155 changes: 83 additions & 72 deletions src/librustc_mir/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use rustc_errors::struct_span_err;
use rustc_hir::lang_items;
use rustc_hir::{def_id::DefId, HirId};
use rustc_index::bit_set::BitSet;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
Expand All @@ -28,70 +27,100 @@ use crate::dataflow::{self, Analysis};
// We are using `MaybeMutBorrowedLocals` as a proxy for whether an item may have been mutated
// through a pointer prior to the given point. This is okay even though `MaybeMutBorrowedLocals`
// kills locals upon `StorageDead` because a local will never be used after a `StorageDead`.
pub type IndirectlyMutableResults<'mir, 'tcx> =
type IndirectlyMutableResults<'mir, 'tcx> =
dataflow::ResultsCursor<'mir, 'tcx, MaybeMutBorrowedLocals<'mir, 'tcx>>;

struct QualifCursor<'a, 'mir, 'tcx, Q: Qualif> {
cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>,
in_any_value_of_ty: BitSet<Local>,
}

impl<Q: Qualif> QualifCursor<'a, 'mir, 'tcx, Q> {
pub fn new(q: Q, ccx: &'a ConstCx<'mir, 'tcx>) -> Self {
let cursor = FlowSensitiveAnalysis::new(q, ccx)
.into_engine(ccx.tcx, ccx.body, ccx.def_id)
.iterate_to_fixpoint()
.into_results_cursor(ccx.body);

let mut in_any_value_of_ty = BitSet::new_empty(ccx.body.local_decls.len());
for (local, decl) in ccx.body.local_decls.iter_enumerated() {
if Q::in_any_value_of_ty(ccx, decl.ty) {
in_any_value_of_ty.insert(local);
}
}
type QualifResults<'mir, 'tcx, Q> =
dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'mir, 'mir, 'tcx, Q>>;

QualifCursor { cursor, in_any_value_of_ty }
}
#[derive(Default)]
pub struct Qualifs<'mir, 'tcx> {
has_mut_interior: Option<QualifResults<'mir, 'tcx, HasMutInterior>>,
needs_drop: Option<QualifResults<'mir, 'tcx, NeedsDrop>>,
indirectly_mutable: Option<IndirectlyMutableResults<'mir, 'tcx>>,
}

pub struct Qualifs<'a, 'mir, 'tcx> {
has_mut_interior: QualifCursor<'a, 'mir, 'tcx, HasMutInterior>,
needs_drop: QualifCursor<'a, 'mir, 'tcx, NeedsDrop>,
indirectly_mutable: IndirectlyMutableResults<'mir, 'tcx>,
}

impl Qualifs<'a, 'mir, 'tcx> {
fn indirectly_mutable(&mut self, local: Local, location: Location) -> bool {
self.indirectly_mutable.seek_before(location);
self.indirectly_mutable.get().contains(local)
impl Qualifs<'mir, 'tcx> {
fn indirectly_mutable(
&mut self,
ccx: &'mir ConstCx<'mir, 'tcx>,
local: Local,
location: Location,
) -> bool {
let indirectly_mutable = self.indirectly_mutable.get_or_insert_with(|| {
let ConstCx { tcx, body, def_id, param_env, .. } = *ccx;

// We can use `unsound_ignore_borrow_on_drop` here because custom drop impls are not
// allowed in a const.
//
// FIXME(ecstaticmorse): Someday we want to allow custom drop impls. How do we do this
// without breaking stable code?
MaybeMutBorrowedLocals::mut_borrows_only(tcx, &body, param_env)
.unsound_ignore_borrow_on_drop()
.into_engine(tcx, &body, def_id)
.iterate_to_fixpoint()
.into_results_cursor(&body)
});

indirectly_mutable.seek_before(location);
indirectly_mutable.get().contains(local)
}

/// Returns `true` if `local` is `NeedsDrop` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary
fn needs_drop(&mut self, local: Local, location: Location) -> bool {
if !self.needs_drop.in_any_value_of_ty.contains(local) {
fn needs_drop(
&mut self,
ccx: &'mir ConstCx<'mir, 'tcx>,
local: Local,
location: Location,
) -> bool {
let ty = ccx.body.local_decls[local].ty;
if !NeedsDrop::in_any_value_of_ty(ccx, ty) {
return false;
}

self.needs_drop.cursor.seek_before(location);
self.needs_drop.cursor.get().contains(local) || self.indirectly_mutable(local, location)
let needs_drop = self.needs_drop.get_or_insert_with(|| {
let ConstCx { tcx, body, def_id, .. } = *ccx;

FlowSensitiveAnalysis::new(NeedsDrop, ccx)
.into_engine(tcx, &body, def_id)
.iterate_to_fixpoint()
.into_results_cursor(&body)
});

needs_drop.seek_before(location);
needs_drop.get().contains(local) || self.indirectly_mutable(ccx, local, location)
}

/// Returns `true` if `local` is `HasMutInterior` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary.
fn has_mut_interior(&mut self, local: Local, location: Location) -> bool {
if !self.has_mut_interior.in_any_value_of_ty.contains(local) {
fn has_mut_interior(
&mut self,
ccx: &'mir ConstCx<'mir, 'tcx>,
local: Local,
location: Location,
) -> bool {
let ty = ccx.body.local_decls[local].ty;
if !HasMutInterior::in_any_value_of_ty(ccx, ty) {
return false;
}

self.has_mut_interior.cursor.seek_before(location);
self.has_mut_interior.cursor.get().contains(local)
|| self.indirectly_mutable(local, location)
let has_mut_interior = self.has_mut_interior.get_or_insert_with(|| {
let ConstCx { tcx, body, def_id, .. } = *ccx;

FlowSensitiveAnalysis::new(HasMutInterior, ccx)
.into_engine(tcx, &body, def_id)
.iterate_to_fixpoint()
.into_results_cursor(&body)
});

has_mut_interior.seek_before(location);
has_mut_interior.get().contains(local) || self.indirectly_mutable(ccx, local, location)
}

fn in_return_place(&mut self, ccx: &ConstCx<'_, 'tcx>) -> ConstQualifs {
fn in_return_place(&mut self, ccx: &'mir ConstCx<'mir, 'tcx>) -> ConstQualifs {
// Find the `Return` terminator if one exists.
//
// If no `Return` terminator exists, this MIR is divergent. Just return the conservative
Expand All @@ -114,49 +143,31 @@ impl Qualifs<'a, 'mir, 'tcx> {
let return_loc = ccx.body.terminator_loc(return_block);

ConstQualifs {
needs_drop: self.needs_drop(RETURN_PLACE, return_loc),
has_mut_interior: self.has_mut_interior(RETURN_PLACE, return_loc),
needs_drop: self.needs_drop(ccx, RETURN_PLACE, return_loc),
has_mut_interior: self.has_mut_interior(ccx, RETURN_PLACE, return_loc),
}
}
}

pub struct Validator<'a, 'mir, 'tcx> {
ccx: &'a ConstCx<'mir, 'tcx>,
qualifs: Qualifs<'a, 'mir, 'tcx>,
pub struct Validator<'mir, 'tcx> {
ccx: &'mir ConstCx<'mir, 'tcx>,
qualifs: Qualifs<'mir, 'tcx>,

/// The span of the current statement.
span: Span,
}

impl Deref for Validator<'_, 'mir, 'tcx> {
impl Deref for Validator<'mir, 'tcx> {
type Target = ConstCx<'mir, 'tcx>;

fn deref(&self) -> &Self::Target {
&self.ccx
}
}

impl Validator<'a, 'mir, 'tcx> {
pub fn new(ccx: &'a ConstCx<'mir, 'tcx>) -> Self {
let ConstCx { tcx, body, def_id, param_env, .. } = *ccx;

let needs_drop = QualifCursor::new(NeedsDrop, ccx);
let has_mut_interior = QualifCursor::new(HasMutInterior, ccx);

// We can use `unsound_ignore_borrow_on_drop` here because custom drop impls are not
// allowed in a const.
//
// FIXME(ecstaticmorse): Someday we want to allow custom drop impls. How do we do this
// without breaking stable code?
let indirectly_mutable = MaybeMutBorrowedLocals::mut_borrows_only(tcx, body, param_env)
.unsound_ignore_borrow_on_drop()
.into_engine(tcx, body, def_id)
.iterate_to_fixpoint()
.into_results_cursor(body);

let qualifs = Qualifs { needs_drop, has_mut_interior, indirectly_mutable };

Validator { span: ccx.body.span, ccx, qualifs }
impl Validator<'mir, 'tcx> {
pub fn new(ccx: &'mir ConstCx<'mir, 'tcx>) -> Self {
Validator { span: ccx.body.span, ccx, qualifs: Default::default() }
}

pub fn check_body(&mut self) {
Expand Down Expand Up @@ -239,7 +250,7 @@ impl Validator<'a, 'mir, 'tcx> {
}
}

impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
impl Visitor<'tcx> for Validator<'mir, 'tcx> {
fn visit_basic_block_data(&mut self, bb: BasicBlock, block: &BasicBlockData<'tcx>) {
trace!("visit_basic_block_data: bb={:?} is_cleanup={:?}", bb, block.is_cleanup);

Expand Down Expand Up @@ -345,7 +356,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
| Rvalue::AddressOf(Mutability::Not, ref place) => {
let borrowed_place_has_mut_interior = qualifs::in_place::<HasMutInterior, _>(
&self.ccx,
&mut |local| self.qualifs.has_mut_interior(local, location),
&mut |local| self.qualifs.has_mut_interior(self.ccx, local, location),
place.as_ref(),
);

Expand Down Expand Up @@ -571,7 +582,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(local, location)
self.qualifs.needs_drop(self.ccx, local, location)
} else {
true
};
Expand Down
21 changes: 16 additions & 5 deletions src/test/ui/issues/issue-17252.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
error[E0391]: cycle detected when const checking `FOO`
--> $DIR/issue-17252.rs:1:20
error[E0391]: cycle detected when normalizing `FOO`
|
note: ...which requires const-evaluating + checking `FOO`...
--> $DIR/issue-17252.rs:1:1
|
LL | const FOO: usize = FOO;
| ^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires const-evaluating + checking `FOO`...
--> $DIR/issue-17252.rs:1:1
|
LL | const FOO: usize = FOO;
| ^^^
| ^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires const-evaluating `FOO`...
--> $DIR/issue-17252.rs:1:1
|
= note: ...which again requires const checking `FOO`, completing the cycle
note: cycle used when const checking `main::{{constant}}#0`
LL | const FOO: usize = FOO;
| ^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which again requires normalizing `FOO`, completing the cycle
note: cycle used when const-evaluating `main::{{constant}}#0`
--> $DIR/issue-17252.rs:4:18
|
LL | let _x: [u8; FOO]; // caused stack overflow prior to fix
Expand Down
17 changes: 14 additions & 3 deletions src/test/ui/issues/issue-23302-1.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
error[E0391]: cycle detected when const checking `X::A::{{constant}}#0`
error[E0391]: cycle detected when const-evaluating + checking `X::A::{{constant}}#0`
--> $DIR/issue-23302-1.rs:4:9
|
LL | A = X::A as isize,
| ^^^^^^^^^^^^^
|
= note: ...which again requires const checking `X::A::{{constant}}#0`, completing the cycle
note: cycle used when processing `X::A::{{constant}}#0`
note: ...which requires const-evaluating + checking `X::A::{{constant}}#0`...
--> $DIR/issue-23302-1.rs:4:9
|
LL | A = X::A as isize,
| ^^^^^^^^^^^^^
note: ...which requires const-evaluating `X::A::{{constant}}#0`...
--> $DIR/issue-23302-1.rs:4:9
|
LL | A = X::A as isize,
| ^^^^^^^^^^^^^
= note: ...which requires normalizing `X::A as isize`...
= note: ...which again requires const-evaluating + checking `X::A::{{constant}}#0`, completing the cycle
note: cycle used when collecting item types in top-level module
--> $DIR/issue-23302-1.rs:3:1
|
LL | enum X {
| ^^^^^^

error: aborting due to previous error

Expand Down
17 changes: 14 additions & 3 deletions src/test/ui/issues/issue-23302-2.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
error[E0391]: cycle detected when const checking `Y::A::{{constant}}#0`
error[E0391]: cycle detected when const-evaluating + checking `Y::A::{{constant}}#0`
--> $DIR/issue-23302-2.rs:4:9
|
LL | A = Y::B as isize,
| ^^^^^^^^^^^^^
|
= note: ...which again requires const checking `Y::A::{{constant}}#0`, completing the cycle
note: cycle used when processing `Y::A::{{constant}}#0`
note: ...which requires const-evaluating + checking `Y::A::{{constant}}#0`...
--> $DIR/issue-23302-2.rs:4:9
|
LL | A = Y::B as isize,
| ^^^^^^^^^^^^^
note: ...which requires const-evaluating `Y::A::{{constant}}#0`...
--> $DIR/issue-23302-2.rs:4:9
|
LL | A = Y::B as isize,
| ^^^^^^^^^^^^^
= note: ...which requires normalizing `Y::B as isize`...
= note: ...which again requires const-evaluating + checking `Y::A::{{constant}}#0`, completing the cycle
note: cycle used when collecting item types in top-level module
--> $DIR/issue-23302-2.rs:3:1
|
LL | enum Y {
| ^^^^^^

error: aborting due to previous error

Expand Down
36 changes: 27 additions & 9 deletions src/test/ui/issues/issue-23302-3.stderr
Original file line number Diff line number Diff line change
@@ -1,20 +1,38 @@
error[E0391]: cycle detected when const checking `A`
--> $DIR/issue-23302-3.rs:1:16
error[E0391]: cycle detected when const-evaluating + checking `A`
--> $DIR/issue-23302-3.rs:1:1
|
LL | const A: i32 = B;
| ^
| ^^^^^^^^^^^^^^^^^
|
note: ...which requires const checking `B`...
--> $DIR/issue-23302-3.rs:3:16
note: ...which requires const-evaluating + checking `A`...
--> $DIR/issue-23302-3.rs:1:1
|
LL | const B: i32 = A;
| ^
= note: ...which again requires const checking `A`, completing the cycle
note: cycle used when processing `A`
LL | const A: i32 = B;
| ^^^^^^^^^^^^^^^^^
note: ...which requires const-evaluating `A`...
--> $DIR/issue-23302-3.rs:1:1
|
LL | const A: i32 = B;
| ^^^^^^^^^^^^^^^^^
= note: ...which requires normalizing `B`...
note: ...which requires const-evaluating + checking `B`...
--> $DIR/issue-23302-3.rs:3:1
|
LL | const B: i32 = A;
| ^^^^^^^^^^^^^^^^^
note: ...which requires const-evaluating + checking `B`...
--> $DIR/issue-23302-3.rs:3:1
|
LL | const B: i32 = A;
| ^^^^^^^^^^^^^^^^^
note: ...which requires const-evaluating `B`...
--> $DIR/issue-23302-3.rs:3:1
|
LL | const B: i32 = A;
| ^^^^^^^^^^^^^^^^^
= note: ...which requires normalizing `A`...
= note: ...which again requires const-evaluating + checking `A`, completing the cycle
= note: cycle used when running analysis passes on this crate

error: aborting due to previous error

Expand Down
Loading

0 comments on commit f3331cb

Please sign in to comment.