diff options
5 files changed, 250 insertions, 63 deletions
diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index a3f685fb747..3a5bc37b85a 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -12,7 +12,6 @@ use rustc_middle::ty::cast::CastTy; use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts}; use rustc_middle::ty::{self, adjustment::PointerCast, Instance, InstanceDef, Ty, TyCtxt}; use rustc_middle::ty::{Binder, TraitPredicate, TraitRef}; -use rustc_mir_dataflow::impls::MaybeMutBorrowedLocals; use rustc_mir_dataflow::{self, Analysis}; use rustc_span::{sym, Span, Symbol}; use rustc_trait_selection::traits::error_reporting::InferCtxtExt; @@ -27,12 +26,6 @@ use super::resolver::FlowSensitiveAnalysis; use super::{ConstCx, Qualif}; use crate::const_eval::is_unstable_const_fn; -// 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`. -type IndirectlyMutableResults<'mir, 'tcx> = - rustc_mir_dataflow::ResultsCursor<'mir, 'tcx, MaybeMutBorrowedLocals<'mir, 'tcx>>; - type QualifResults<'mir, 'tcx, Q> = rustc_mir_dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'mir, 'mir, 'tcx, Q>>; @@ -41,36 +34,9 @@ pub struct Qualifs<'mir, 'tcx> { has_mut_interior: Option<QualifResults<'mir, 'tcx, HasMutInterior>>, needs_drop: Option<QualifResults<'mir, 'tcx, NeedsDrop>>, needs_non_const_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>, - indirectly_mutable: Option<IndirectlyMutableResults<'mir, 'tcx>>, } impl Qualifs<'mir, 'tcx> { - pub 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, 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) - .pass_name("const_qualification") - .iterate_to_fixpoint() - .into_results_cursor(&body) - }); - - indirectly_mutable.seek_before_primary_effect(location); - indirectly_mutable.get().contains(local) - } - /// Returns `true` if `local` is `NeedsDrop` at the given `Location`. /// /// Only updates the cursor if absolutely necessary @@ -95,7 +61,7 @@ impl Qualifs<'mir, 'tcx> { }); needs_drop.seek_before_primary_effect(location); - needs_drop.get().contains(local) || self.indirectly_mutable(ccx, local, location) + needs_drop.get().contains(local) } /// Returns `true` if `local` is `NeedsNonConstDrop` at the given `Location`. @@ -122,7 +88,7 @@ impl Qualifs<'mir, 'tcx> { }); needs_non_const_drop.seek_before_primary_effect(location); - needs_non_const_drop.get().contains(local) || self.indirectly_mutable(ccx, local, location) + needs_non_const_drop.get().contains(local) } /// Returns `true` if `local` is `HasMutInterior` at the given `Location`. @@ -149,7 +115,7 @@ impl Qualifs<'mir, 'tcx> { }); has_mut_interior.seek_before_primary_effect(location); - has_mut_interior.get().contains(local) || self.indirectly_mutable(ccx, local, location) + has_mut_interior.get().contains(local) } fn in_return_place( @@ -195,7 +161,7 @@ impl Qualifs<'mir, 'tcx> { .into_results_cursor(&ccx.body); cursor.seek_after_primary_effect(return_loc); - cursor.contains(RETURN_PLACE) + cursor.get().contains(RETURN_PLACE) } }; diff --git a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs index e20b86dd452..ff2271fb396 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs @@ -5,7 +5,11 @@ use rustc_index::bit_set::BitSet; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{self, BasicBlock, Local, Location, Statement, StatementKind}; +use rustc_mir_dataflow::fmt::DebugWithContext; +use rustc_mir_dataflow::JoinSemiLattice; +use rustc_span::DUMMY_SP; +use std::fmt; use std::marker::PhantomData; use super::{qualifs, ConstCx, Qualif}; @@ -13,13 +17,13 @@ use super::{qualifs, ConstCx, Qualif}; /// A `Visitor` that propagates qualifs between locals. This defines the transfer function of /// `FlowSensitiveAnalysis`. /// -/// This transfer does nothing when encountering an indirect assignment. Consumers should rely on -/// the `MaybeMutBorrowedLocals` dataflow pass to see if a `Local` may have become qualified via -/// an indirect assignment or function call. +/// To account for indirect assignments, data flow conservatively assumes that local becomes +/// qualified immediately after it is borrowed or its address escapes. The borrow must allow for +/// mutation, which includes shared borrows of places with interior mutability. The type of +/// borrowed place must contain the qualif. struct TransferFunction<'a, 'mir, 'tcx, Q> { ccx: &'a ConstCx<'mir, 'tcx>, - qualifs_per_local: &'a mut BitSet<Local>, - + state: &'a mut State, _qualif: PhantomData<Q>, } @@ -27,17 +31,18 @@ impl<Q> TransferFunction<'a, 'mir, 'tcx, Q> where Q: Qualif, { - fn new(ccx: &'a ConstCx<'mir, 'tcx>, qualifs_per_local: &'a mut BitSet<Local>) -> Self { - TransferFunction { ccx, qualifs_per_local, _qualif: PhantomData } + fn new(ccx: &'a ConstCx<'mir, 'tcx>, state: &'a mut State) -> Self { + TransferFunction { ccx, state, _qualif: PhantomData } } fn initialize_state(&mut self) { - self.qualifs_per_local.clear(); + self.state.qualif.clear(); + self.state.borrow.clear(); for arg in self.ccx.body.args_iter() { let arg_ty = self.ccx.body.local_decls[arg].ty; if Q::in_any_value_of_ty(self.ccx, arg_ty) { - self.qualifs_per_local.insert(arg); + self.state.qualif.insert(arg); } } } @@ -47,7 +52,7 @@ where match (value, place.as_ref()) { (true, mir::PlaceRef { local, .. }) => { - self.qualifs_per_local.insert(local); + self.state.qualif.insert(local); } // For now, we do not clear the qualif if a local is overwritten in full by @@ -55,7 +60,7 @@ where // with aggregates where we overwrite all fields with assignments, which would not // get this feature. (false, mir::PlaceRef { local: _, projection: &[] }) => { - // self.qualifs_per_local.remove(*local); + // self.state.qualif.remove(*local); } _ => {} @@ -78,6 +83,29 @@ where self.assign_qualif_direct(&return_place, qualif); } } + + fn address_of_allows_mutation(&self, mt: mir::Mutability, place: mir::Place<'tcx>) -> bool { + match mt { + mir::Mutability::Mut => true, + mir::Mutability::Not => self.shared_borrow_allows_mutation(place), + } + } + + fn ref_allows_mutation(&self, kind: mir::BorrowKind, place: mir::Place<'tcx>) -> bool { + match kind { + mir::BorrowKind::Mut { .. } => true, + mir::BorrowKind::Shared | mir::BorrowKind::Shallow | mir::BorrowKind::Unique => { + self.shared_borrow_allows_mutation(place) + } + } + } + + fn shared_borrow_allows_mutation(&self, place: mir::Place<'tcx>) -> bool { + !place + .ty(self.ccx.body, self.ccx.tcx) + .ty + .is_freeze(self.ccx.tcx.at(DUMMY_SP), self.ccx.param_env) + } } impl<Q> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx, Q> @@ -95,7 +123,12 @@ where // it no longer needs to be dropped. if let mir::Operand::Move(place) = operand { if let Some(local) = place.as_local() { - self.qualifs_per_local.remove(local); + // For backward compatibility with the MaybeMutBorrowedLocals used in an earlier + // implementation we retain qualif if a local had been borrowed before. This might + // not be strictly necessary since the local is no longer initialized. + if !self.state.borrow.contains(local) { + self.state.qualif.remove(local); + } } } } @@ -106,11 +139,8 @@ where rvalue: &mir::Rvalue<'tcx>, location: Location, ) { - let qualif = qualifs::in_rvalue::<Q, _>( - self.ccx, - &mut |l| self.qualifs_per_local.contains(l), - rvalue, - ); + let qualif = + qualifs::in_rvalue::<Q, _>(self.ccx, &mut |l| self.state.qualif.contains(l), rvalue); if !place.is_indirect() { self.assign_qualif_direct(place, qualif); } @@ -120,10 +150,53 @@ where self.super_assign(place, rvalue, location); } + fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) { + self.super_rvalue(rvalue, location); + + match rvalue { + mir::Rvalue::AddressOf(mt, borrowed_place) => { + if !borrowed_place.is_indirect() + && self.address_of_allows_mutation(*mt, *borrowed_place) + { + let place_ty = borrowed_place.ty(self.ccx.body, self.ccx.tcx).ty; + if Q::in_any_value_of_ty(self.ccx, place_ty) { + self.state.qualif.insert(borrowed_place.local); + self.state.borrow.insert(borrowed_place.local); + } + } + } + + mir::Rvalue::Ref(_, kind, borrowed_place) => { + if !borrowed_place.is_indirect() && self.ref_allows_mutation(*kind, *borrowed_place) + { + let place_ty = borrowed_place.ty(self.ccx.body, self.ccx.tcx).ty; + if Q::in_any_value_of_ty(self.ccx, place_ty) { + self.state.qualif.insert(borrowed_place.local); + self.state.borrow.insert(borrowed_place.local); + } + } + } + + mir::Rvalue::Cast(..) + | mir::Rvalue::ShallowInitBox(..) + | mir::Rvalue::Use(..) + | mir::Rvalue::ThreadLocalRef(..) + | mir::Rvalue::Repeat(..) + | mir::Rvalue::Len(..) + | mir::Rvalue::BinaryOp(..) + | mir::Rvalue::CheckedBinaryOp(..) + | mir::Rvalue::NullaryOp(..) + | mir::Rvalue::UnaryOp(..) + | mir::Rvalue::Discriminant(..) + | mir::Rvalue::Aggregate(..) => {} + } + } + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { match statement.kind { StatementKind::StorageDead(local) => { - self.qualifs_per_local.remove(local); + self.state.qualif.remove(local); + self.state.borrow.remove(local); } _ => self.super_statement(statement, location), } @@ -136,7 +209,7 @@ where if let mir::TerminatorKind::DropAndReplace { value, place, .. } = &terminator.kind { let qualif = qualifs::in_operand::<Q, _>( self.ccx, - &mut |l| self.qualifs_per_local.contains(l), + &mut |l| self.state.qualif.contains(l), value, ); @@ -145,6 +218,9 @@ where } } + // We ignore borrow on drop because custom drop impls are not allowed in consts. + // FIXME: Reconsider if accounting for borrows in drops is necessary for const drop. + // We need to assign qualifs to the dropped location before visiting the operand that // replaces it since qualifs can be cleared on move. self.super_terminator(terminator, location); @@ -165,24 +241,76 @@ where FlowSensitiveAnalysis { ccx, _qualif: PhantomData } } - fn transfer_function( - &self, - state: &'a mut BitSet<Local>, - ) -> TransferFunction<'a, 'mir, 'tcx, Q> { + fn transfer_function(&self, state: &'a mut State) -> TransferFunction<'a, 'mir, 'tcx, Q> { TransferFunction::<Q>::new(self.ccx, state) } } +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) struct State { + /// Describes whether a local contains qualif. + pub qualif: BitSet<Local>, + /// Describes whether a local's address escaped and it might become qualified as a result an + /// indirect mutation. + pub borrow: BitSet<Local>, +} + +impl State { + #[inline] + pub(super) fn contains(&self, local: Local) -> bool { + self.qualif.contains(local) + } +} + +impl<C> DebugWithContext<C> for State { + fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("qualif: ")?; + self.qualif.fmt_with(ctxt, f)?; + f.write_str(" borrow: ")?; + self.borrow.fmt_with(ctxt, f)?; + Ok(()) + } + + fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self == old { + return Ok(()); + } + + if self.qualif != old.qualif { + f.write_str("qualif: ")?; + self.qualif.fmt_diff_with(&old.qualif, ctxt, f)?; + f.write_str("\n")?; + } + + if self.borrow != old.borrow { + f.write_str("borrow: ")?; + self.qualif.fmt_diff_with(&old.borrow, ctxt, f)?; + f.write_str("\n")?; + } + + Ok(()) + } +} + +impl JoinSemiLattice for State { + fn join(&mut self, other: &Self) -> bool { + self.qualif.join(&other.qualif) || self.borrow.join(&other.borrow) + } +} + impl<Q> rustc_mir_dataflow::AnalysisDomain<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q> where Q: Qualif, { - type Domain = BitSet<Local>; + type Domain = State; const NAME: &'static str = Q::ANALYSIS_NAME; fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { - BitSet::new_empty(body.local_decls.len()) + State { + qualif: BitSet::new_empty(body.local_decls.len()), + borrow: BitSet::new_empty(body.local_decls.len()), + } } fn initialize_start_block(&self, _body: &mir::Body<'tcx>, state: &mut Self::Domain) { diff --git a/src/test/ui/consts/qualif-indirect-mutation-fail.rs b/src/test/ui/consts/qualif-indirect-mutation-fail.rs new file mode 100644 index 00000000000..cedead00fec --- /dev/null +++ b/src/test/ui/consts/qualif-indirect-mutation-fail.rs @@ -0,0 +1,44 @@ +// compile-flags: --crate-type=lib +#![feature(const_mut_refs)] +#![feature(const_precise_live_drops)] +#![feature(const_swap)] + +// Mutable borrow of a field with drop impl. +pub const fn f() { + let mut a: (u32, Option<String>) = (0, None); //~ ERROR destructors cannot be evaluated + let _ = &mut a.1; +} + +// Mutable borrow of a type with drop impl. +pub const A1: () = { + let mut x = None; //~ ERROR destructors cannot be evaluated + let mut y = Some(String::new()); + let a = &mut x; + let b = &mut y; + std::mem::swap(a, b); + std::mem::forget(y); +}; + +// Mutable borrow of a type with drop impl. +pub const A2: () = { + let mut x = None; + let mut y = Some(String::new()); + let a = &mut x; + let b = &mut y; + std::mem::swap(a, b); + std::mem::forget(y); + let _z = x; //~ ERROR destructors cannot be evaluated +}; + +// Shared borrow of a type that might be !Freeze and Drop. +pub const fn g1<T>() { + let x: Option<T> = None; //~ ERROR destructors cannot be evaluated + let _ = x.is_some(); +} + +// Shared borrow of a type that might be !Freeze and Drop. +pub const fn g2<T>() { + let x: Option<T> = None; + let _ = x.is_some(); + let _y = x; //~ ERROR destructors cannot be evaluated +} diff --git a/src/test/ui/consts/qualif-indirect-mutation-fail.stderr b/src/test/ui/consts/qualif-indirect-mutation-fail.stderr new file mode 100644 index 00000000000..aa6ed465594 --- /dev/null +++ b/src/test/ui/consts/qualif-indirect-mutation-fail.stderr @@ -0,0 +1,33 @@ +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:8:9 + | +LL | let mut a: (u32, Option<String>) = (0, None); + | ^^^^^ constant functions cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:14:9 + | +LL | let mut x = None; + | ^^^^^ constants cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:30:9 + | +LL | let _z = x; + | ^^ constants cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:35:9 + | +LL | let x: Option<T> = None; + | ^ constant functions cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:43:9 + | +LL | let _y = x; + | ^^ constant functions cannot evaluate destructors + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0493`. diff --git a/src/test/ui/consts/qualif-indirect-mutation-pass.rs b/src/test/ui/consts/qualif-indirect-mutation-pass.rs new file mode 100644 index 00000000000..35a9b70a5f6 --- /dev/null +++ b/src/test/ui/consts/qualif-indirect-mutation-pass.rs @@ -0,0 +1,16 @@ +// compile-flags: --crate-type=lib +// check-pass +#![feature(const_mut_refs)] +#![feature(const_precise_live_drops)] + +pub const fn f() { + let mut x: (Option<String>, u32) = (None, 0); + let mut a = 10; + *(&mut a) = 11; + x.1 = a; +} + +pub const fn g() { + let mut a: (u32, Option<String>) = (0, None); + let _ = &mut a.0; +} |