From 7fb1c22da181426f6ea7662f5c7f47edbbe24f56 Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Sat, 16 Mar 2019 17:40:41 +0530 Subject: [PATCH 1/8] promoted is still left in 2 places --- src/librustc/mir/mod.rs | 37 +++--- src/librustc/mir/tcx.rs | 1 - src/librustc/mir/visit.rs | 15 ++- .../borrow_check/error_reporting.rs | 17 +-- src/librustc_mir/borrow_check/mod.rs | 56 +++++---- .../borrow_check/mutability_errors.rs | 23 ++-- .../borrow_check/nll/type_check/mod.rs | 86 +++++++------ src/librustc_mir/borrow_check/path_utils.rs | 1 - src/librustc_mir/borrow_check/place_ext.rs | 10 +- .../borrow_check/places_conflict.rs | 67 +++++----- src/librustc_mir/borrow_check/prefixes.rs | 2 - src/librustc_mir/build/expr/as_place.rs | 1 + .../dataflow/impls/borrowed_locals.rs | 1 - .../dataflow/move_paths/builder.rs | 1 - src/librustc_mir/dataflow/move_paths/mod.rs | 1 - src/librustc_mir/interpret/place.rs | 59 ++++----- src/librustc_mir/transform/add_retag.rs | 1 - src/librustc_mir/transform/check_unsafety.rs | 50 ++++---- src/librustc_mir/transform/const_prop.rs | 42 ++++--- src/librustc_mir/transform/inline.rs | 12 +- src/librustc_mir/transform/qualify_consts.rs | 119 ++++++++++-------- .../transform/qualify_min_const_fn.rs | 11 +- 22 files changed, 333 insertions(+), 280 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 9f2027e7d05..47b9535abc5 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1913,9 +1913,6 @@ pub enum PlaceBase<'tcx> { /// static or static mut variable Static(Box>), - - /// Constant code promoted to an injected static - Promoted(Box<(Promoted, Ty<'tcx>)>), } /// The `DefId` of a static, along with its normalized type (which is @@ -1924,11 +1921,13 @@ pub enum PlaceBase<'tcx> { pub struct Static<'tcx> { pub def_id: DefId, pub ty: Ty<'tcx>, + pub promoted: Option, } impl_stable_hash_for!(struct Static<'tcx> { def_id, - ty + ty, + promoted }); /// The `Projection` data structure defines things of the form `B.x` @@ -2048,7 +2047,7 @@ impl<'tcx> Place<'tcx> { match self { Place::Base(PlaceBase::Local(local)) => Some(*local), Place::Projection(box Projection { base, elem: _ }) => base.base_local(), - Place::Base(PlaceBase::Promoted(..)) | Place::Base(PlaceBase::Static(..)) => None, + Place::Base(PlaceBase::Static(..)) => None, } } } @@ -2059,18 +2058,22 @@ impl<'tcx> Debug for Place<'tcx> { match *self { Base(PlaceBase::Local(id)) => write!(fmt, "{:?}", id), - Base(PlaceBase::Static(box self::Static { def_id, ty })) => write!( - fmt, - "({}: {:?})", - ty::tls::with(|tcx| tcx.def_path_str(def_id)), - ty - ), - Base(PlaceBase::Promoted(ref promoted)) => write!( - fmt, - "({:?}: {:?})", - promoted.0, - promoted.1 - ), + Base(PlaceBase::Static(box self::Static { def_id, ty, promoted })) => { + match promoted { + None => write!( + fmt, + "({}: {:?})", + ty::tls::with(|tcx| tcx.def_path_str(def_id)), + ty + ), + Some(pr) => write!( + fmt, + "({:?}: {:?})", + pr, + ty + ), + } + }, Projection(ref data) => match data.elem { ProjectionElem::Downcast(ref adt_def, index) => { write!(fmt, "({:?} as {})", data.base, adt_def.variants[index].ident) diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs index a6f153eaf64..ac42eacacd7 100644 --- a/src/librustc/mir/tcx.rs +++ b/src/librustc/mir/tcx.rs @@ -160,7 +160,6 @@ impl<'tcx> Place<'tcx> { match *self { Place::Base(PlaceBase::Local(index)) => PlaceTy::Ty { ty: local_decls.local_decls()[index].ty }, - Place::Base(PlaceBase::Promoted(ref data)) => PlaceTy::Ty { ty: data.1 }, Place::Base(PlaceBase::Static(ref data)) => PlaceTy::Ty { ty: data.ty }, Place::Projection(ref proj) => diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 8bc0075c477..cddf5782121 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -737,11 +737,18 @@ macro_rules! make_mir_visitor { self.visit_local(local, context, location); } Place::Base(PlaceBase::Static(static_)) => { + match static_.promoted { + None => { + self.visit_static(static_, context, location); + } + Some(_) => { + self.visit_ty( + & $($mutability)? static_.ty, TyContext::Location(location) + ); + } + } self.visit_static(static_, context, location); } - Place::Base(PlaceBase::Promoted(promoted)) => { - self.visit_ty(& $($mutability)? promoted.1, TyContext::Location(location)); - }, Place::Projection(proj) => { self.visit_projection(proj, context, location); } @@ -752,7 +759,7 @@ macro_rules! make_mir_visitor { static_: & $($mutability)? Static<'tcx>, _context: PlaceContext<'tcx>, location: Location) { - let Static { def_id, ty } = static_; + let Static { def_id, ty, promoted: _ } = static_; self.visit_def_id(def_id, location); self.visit_ty(ty, TyContext::Location(location)); } diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 14289381aef..8f83c025ceb 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -1598,14 +1598,18 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { including_downcast: &IncludingDowncast, ) -> Result<(), ()> { match *place { - Place::Base(PlaceBase::Promoted(_)) => { - buf.push_str("promoted"); - } Place::Base(PlaceBase::Local(local)) => { self.append_local_to_string(local, buf)?; } Place::Base(PlaceBase::Static(ref static_)) => { - buf.push_str(&self.infcx.tcx.item_name(static_.def_id).to_string()); + match static_.promoted { + Some(_) => { + buf.push_str("promoted"); + } + None => { + buf.push_str(&self.infcx.tcx.item_name(static_.def_id).to_string()); + } + } } Place::Projection(ref proj) => { match proj.elem { @@ -1744,8 +1748,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let local = &self.mir.local_decls[local]; self.describe_field_from_ty(&local.ty, field) } - Place::Base(PlaceBase::Promoted(ref prom)) => - self.describe_field_from_ty(&prom.1, field), Place::Base(PlaceBase::Static(ref static_)) => self.describe_field_from_ty(&static_.ty, field), Place::Projection(ref proj) => match proj.elem { @@ -1828,8 +1830,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let tcx = self.infcx.tcx; match place { Place::Base(PlaceBase::Local(_)) | - Place::Base(PlaceBase::Static(_)) | - Place::Base(PlaceBase::Promoted(_)) => { + Place::Base(PlaceBase::Static(_)) => { StorageDeadOrDrop::LocalStorageDead } Place::Projection(box PlaceProjection { base, elem }) => { diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index c4e371d5afe..5726eba9bda 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1226,8 +1226,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } Operand::Move(Place::Base(PlaceBase::Static(..))) | Operand::Copy(Place::Base(PlaceBase::Static(..))) - | Operand::Move(Place::Base(PlaceBase::Promoted(..))) - | Operand::Copy(Place::Base(PlaceBase::Promoted(..))) | Operand::Constant(..) => {} } } @@ -1310,12 +1308,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // // FIXME: allow thread-locals to borrow other thread locals? let (might_be_alive, will_be_dropped) = match root_place { - Place::Base(PlaceBase::Promoted(_)) => (true, false), - Place::Base(PlaceBase::Static(_)) => { - // Thread-locals might be dropped after the function exits, but - // "true" statics will never be. - let is_thread_local = self.is_place_thread_local(&root_place); - (true, is_thread_local) + Place::Base(PlaceBase::Static(st)) => { + match st.promoted { + None => { + // Thread-locals might be dropped after the function exits, but + // "true" statics will never be. + let is_thread_local = self.is_place_thread_local(&root_place); + (true, is_thread_local) + } + Some(_) => (true, false), + } } Place::Base(PlaceBase::Local(_)) => { // Locals are always dropped at function exit, and if they @@ -1578,7 +1580,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { match *last_prefix { Place::Base(PlaceBase::Local(_)) => panic!("should have move path for every Local"), Place::Projection(_) => panic!("PrefixSet::All meant don't stop for Projection"), - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Static(_)) => Err(NoMovePathFound::ReachedStatic), } } @@ -1605,7 +1606,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let mut place = place; loop { match *place { - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Local(_)) | Place::Base(PlaceBase::Static(_)) => { // assigning to `x` does not require `x` be initialized. break; @@ -1953,10 +1953,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.used_mut_upvars.push(field); } } - RootPlace { - place: Place::Base(PlaceBase::Promoted(..)), - is_local_mutation_allowed: _, - } => {} RootPlace { place: Place::Base(PlaceBase::Static(..)), is_local_mutation_allowed: _, @@ -1994,18 +1990,28 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } // The rules for promotion are made by `qualify_consts`, there wouldn't even be a // `Place::Promoted` if the promotion weren't 100% legal. So we just forward this - Place::Base(PlaceBase::Promoted(_)) => Ok(RootPlace { - place, - is_local_mutation_allowed, - }), +// Place::Base(PlaceBase::Promoted(_)) => Ok(RootPlace { +// place, +// is_local_mutation_allowed, +// }), Place::Base(PlaceBase::Static(ref static_)) => { - if self.infcx.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) { - Err(place) - } else { - Ok(RootPlace { - place, - is_local_mutation_allowed, - }) + match static_.promoted { + Some(_) => { + Ok(RootPlace { + place, + is_local_mutation_allowed, + }) + } + None => { + if self.infcx.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) { + Err(place) + } else { + Ok(RootPlace { + place, + is_local_mutation_allowed, + }) + } + } } } Place::Projection(ref proj) => { diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index b8dae98ec64..d8c39e0715a 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -129,16 +129,19 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { } } - Place::Base(PlaceBase::Promoted(_)) => unreachable!(), - - Place::Base(PlaceBase::Static(box Static { def_id, ty: _ })) => { - if let Place::Base(PlaceBase::Static(_)) = access_place { - item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); - reason = String::new(); - } else { - item_msg = format!("`{}`", access_place_desc.unwrap()); - let static_name = &self.infcx.tcx.item_name(*def_id); - reason = format!(", as `{}` is an immutable static item", static_name); + Place::Base(PlaceBase::Static(box Static { def_id, ty: _, promoted })) => { + match promoted { + Some(_) => unreachable!(), + None => { + if let Place::Base(PlaceBase::Static(_)) = access_place { + item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); + reason = String::new(); + } else { + item_msg = format!("`{}`", access_place_desc.unwrap()); + let static_name = &self.infcx.tcx.item_name(*def_id); + reason = format!(", as `{}` is an immutable static item", static_name); + } + } } } diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 25a3160a498..334c20761e7 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -453,51 +453,55 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { Place::Base(PlaceBase::Local(index)) => PlaceTy::Ty { ty: self.mir.local_decls[index].ty, }, - Place::Base(PlaceBase::Promoted(box (index, sty))) => { - let sty = self.sanitize_type(place, sty); + Place::Base(PlaceBase::Static(box Static { def_id, ty: sty, promoted })) => { + match promoted { + Some(pr) => { + let sty = self.sanitize_type(place, sty); - if !self.errors_reported { - let promoted_mir = &self.mir.promoted[index]; - self.sanitize_promoted(promoted_mir, location); + if !self.errors_reported { + let promoted_mir = &self.mir.promoted[pr]; + self.sanitize_promoted(promoted_mir, location); - let promoted_ty = promoted_mir.return_ty(); + let promoted_ty = promoted_mir.return_ty(); - if let Err(terr) = self.cx.eq_types( - sty, - promoted_ty, - location.to_locations(), - ConstraintCategory::Boring, - ) { - span_mirbug!( - self, - place, - "bad promoted type ({:?}: {:?}): {:?}", - promoted_ty, - sty, - terr - ); - }; + if let Err(terr) = self.cx.eq_types( + sty, + promoted_ty, + location.to_locations(), + ConstraintCategory::Boring, + ) { + span_mirbug!( + self, + place, + "bad promoted type ({:?}: {:?}): {:?}", + promoted_ty, + sty, + terr + ); + }; + } + PlaceTy::Ty { ty: sty } + } + None => { + let sty = self.sanitize_type(place, sty); + let ty = self.tcx().type_of(def_id); + let ty = self.cx.normalize(ty, location); + if let Err(terr) = + self.cx + .eq_types(ty, sty, location.to_locations(), ConstraintCategory::Boring) + { + span_mirbug!( + self, + place, + "bad static type ({:?}: {:?}): {:?}", + ty, + sty, + terr + ); + } + PlaceTy::Ty { ty: sty } + } } - PlaceTy::Ty { ty: sty } - } - Place::Base(PlaceBase::Static(box Static { def_id, ty: sty })) => { - let sty = self.sanitize_type(place, sty); - let ty = self.tcx().type_of(def_id); - let ty = self.cx.normalize(ty, location); - if let Err(terr) = - self.cx - .eq_types(ty, sty, location.to_locations(), ConstraintCategory::Boring) - { - span_mirbug!( - self, - place, - "bad static type ({:?}: {:?}): {:?}", - ty, - sty, - terr - ); - } - PlaceTy::Ty { ty: sty } } Place::Projection(ref proj) => { let base_context = if context.is_mutating_use() { diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs index 9e0bb93c33a..42eb502b907 100644 --- a/src/librustc_mir/borrow_check/path_utils.rs +++ b/src/librustc_mir/borrow_check/path_utils.rs @@ -138,7 +138,6 @@ pub(super) fn is_active<'tcx>( /// This is called for all Yield statements on movable generators pub(super) fn borrow_of_local_data<'tcx>(place: &Place<'tcx>) -> bool { match place { - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Static(..)) => false, Place::Base(PlaceBase::Local(..)) => true, Place::Projection(box proj) => { diff --git a/src/librustc_mir/borrow_check/place_ext.rs b/src/librustc_mir/borrow_check/place_ext.rs index c05ee3cf65b..0bbd147d824 100644 --- a/src/librustc_mir/borrow_check/place_ext.rs +++ b/src/librustc_mir/borrow_check/place_ext.rs @@ -30,8 +30,6 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { locals_state_at_exit: &LocalsStateAtExit, ) -> bool { match self { - Place::Base(PlaceBase::Promoted(_)) => false, - // If a local variable is immutable, then we only need to track borrows to guard // against two kinds of errors: // * The variable being dropped while still borrowed (e.g., because the fn returns @@ -52,7 +50,12 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { } } Place::Base(PlaceBase::Static(static_)) => { - tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable) + match static_.promoted { + Some(_) => false, + None => { + tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable) + } + } } Place::Projection(proj) => match proj.elem { ProjectionElem::Field(..) @@ -88,7 +91,6 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { loop { match p { Place::Projection(pi) => p = &pi.base, - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Static(_)) => return None, Place::Base(PlaceBase::Local(l)) => return Some(*l), } diff --git a/src/librustc_mir/borrow_check/places_conflict.rs b/src/librustc_mir/borrow_check/places_conflict.rs index 1d18ada1fb6..7babdfcdd68 100644 --- a/src/librustc_mir/borrow_check/places_conflict.rs +++ b/src/librustc_mir/borrow_check/places_conflict.rs @@ -338,7 +338,6 @@ fn unroll_place<'tcx, R>( op, ), - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Local(_)) | Place::Base(PlaceBase::Static(_)) => { let list = PlaceComponents { component: place, @@ -371,41 +370,45 @@ fn place_element_conflict<'a, 'gcx: 'tcx, 'tcx>( Overlap::Disjoint } } - (Place::Base(PlaceBase::Static(static1)), Place::Base(PlaceBase::Static(static2))) => { - if static1.def_id != static2.def_id { - debug!("place_element_conflict: DISJOINT-STATIC"); - Overlap::Disjoint - } else if tcx.is_static(static1.def_id) == Some(hir::Mutability::MutMutable) { - // We ignore mutable statics - they can only be unsafe code. - debug!("place_element_conflict: IGNORE-STATIC-MUT"); - Overlap::Disjoint - } else { - debug!("place_element_conflict: DISJOINT-OR-EQ-STATIC"); - Overlap::EqualOrDisjoint - } - } - (Place::Base(PlaceBase::Promoted(p1)), Place::Base(PlaceBase::Promoted(p2))) => { - if p1.0 == p2.0 { - if let ty::Array(_, size) = p1.1.sty { - if size.unwrap_usize(tcx) == 0 { - // Ignore conflicts with promoted [T; 0]. - debug!("place_element_conflict: IGNORE-LEN-0-PROMOTED"); - return Overlap::Disjoint; + (Place::Base(PlaceBase::Static(s1)), Place::Base(PlaceBase::Static(s2))) => { + match (s1.promoted, s2.promoted) { + (None, None) => { + if s1.def_id != s2.def_id { + debug!("place_element_conflict: DISJOINT-STATIC"); + Overlap::Disjoint + } else if tcx.is_static(s1.def_id) == Some(hir::Mutability::MutMutable) { + // We ignore mutable statics - they can only be unsafe code. + debug!("place_element_conflict: IGNORE-STATIC-MUT"); + Overlap::Disjoint + } else { + debug!("place_element_conflict: DISJOINT-OR-EQ-STATIC"); + Overlap::EqualOrDisjoint } + }, + (Some(p1), Some(p2)) => { + if p1 == p2 { + if let ty::Array(_, size) =s1.ty.sty { + if size.unwrap_usize(tcx) == 0 { + // Ignore conflicts with promoted [T; 0]. + debug!("place_element_conflict: IGNORE-LEN-0-PROMOTED"); + return Overlap::Disjoint; + } + } + // the same promoted - base case, equal + debug!("place_element_conflict: DISJOINT-OR-EQ-PROMOTED"); + Overlap::EqualOrDisjoint + } else { + // different promoteds - base case, disjoint + debug!("place_element_conflict: DISJOINT-PROMOTED"); + Overlap::Disjoint + } + }, + (p1_, p2_) => { + debug!("place_element_conflict: DISJOINT-STATIC-LOCAL-PROMOTED"); + Overlap::Disjoint } - // the same promoted - base case, equal - debug!("place_element_conflict: DISJOINT-OR-EQ-PROMOTED"); - Overlap::EqualOrDisjoint - } else { - // different promoteds - base case, disjoint - debug!("place_element_conflict: DISJOINT-PROMOTED"); - Overlap::Disjoint } } - (Place::Base(PlaceBase::Local(_)), Place::Base(PlaceBase::Promoted(_))) | - (Place::Base(PlaceBase::Promoted(_)), Place::Base(PlaceBase::Local(_))) | - (Place::Base(PlaceBase::Promoted(_)), Place::Base(PlaceBase::Static(_))) | - (Place::Base(PlaceBase::Static(_)), Place::Base(PlaceBase::Promoted(_))) | (Place::Base(PlaceBase::Local(_)), Place::Base(PlaceBase::Static(_))) | (Place::Base(PlaceBase::Static(_)), Place::Base(PlaceBase::Local(_))) => { debug!("place_element_conflict: DISJOINT-STATIC-LOCAL-PROMOTED"); diff --git a/src/librustc_mir/borrow_check/prefixes.rs b/src/librustc_mir/borrow_check/prefixes.rs index 384fd5c9987..e70c9e81ebd 100644 --- a/src/librustc_mir/borrow_check/prefixes.rs +++ b/src/librustc_mir/borrow_check/prefixes.rs @@ -26,7 +26,6 @@ impl<'tcx> IsPrefixOf<'tcx> for Place<'tcx> { } match *cursor { - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Local(_)) | Place::Base(PlaceBase::Static(_)) => return false, Place::Projection(ref proj) => { @@ -87,7 +86,6 @@ impl<'cx, 'gcx, 'tcx> Iterator for Prefixes<'cx, 'gcx, 'tcx> { 'cursor: loop { let proj = match *cursor { - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Local(_)) | // search yielded this leaf Place::Base(PlaceBase::Static(_)) => { self.next = None; diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index 20b95c363f5..83e1d3ebb3e 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -128,6 +128,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ExprKind::StaticRef { id } => block.and(Place::Base(PlaceBase::Static(Box::new(Static { def_id: id, ty: expr.ty, + promoted: None, })))), ExprKind::PlaceTypeAscription { source, user_ty } => { diff --git a/src/librustc_mir/dataflow/impls/borrowed_locals.rs b/src/librustc_mir/dataflow/impls/borrowed_locals.rs index b9c8879b3c3..9d4600d13ac 100644 --- a/src/librustc_mir/dataflow/impls/borrowed_locals.rs +++ b/src/librustc_mir/dataflow/impls/borrowed_locals.rs @@ -93,7 +93,6 @@ struct BorrowedLocalsVisitor<'b, 'c: 'b> { fn find_local<'tcx>(place: &Place<'tcx>) -> Option { match *place { Place::Base(PlaceBase::Local(l)) => Some(l), - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Static(..)) => None, Place::Projection(ref proj) => { match proj.elem { diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 7a9140bce62..71805fd02b8 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -97,7 +97,6 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { debug!("lookup({:?})", place); match *place { Place::Base(PlaceBase::Local(local)) => Ok(self.builder.data.rev_lookup.locals[local]), - Place::Base(PlaceBase::Promoted(..)) | Place::Base(PlaceBase::Static(..)) => { Err(MoveError::cannot_move_out_of(self.loc, Static)) } diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 97f84675f94..7eef68e5f80 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -286,7 +286,6 @@ impl<'tcx> MovePathLookup<'tcx> { pub fn find(&self, place: &Place<'tcx>) -> LookupResult { match *place { Place::Base(PlaceBase::Local(local)) => LookupResult::Exact(self.locals[local]), - Place::Base(PlaceBase::Promoted(_)) | Place::Base(PlaceBase::Static(..)) => LookupResult::Parent(None), Place::Projection(ref proj) => { match self.find(&proj.base) { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 755bbd96b02..62e2cdffadf 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -583,35 +583,38 @@ where use rustc::mir::Place::*; use rustc::mir::PlaceBase; Ok(match *mir_place { - Base(PlaceBase::Promoted(ref promoted)) => { - let instance = self.frame().instance; - self.const_eval_raw(GlobalId { - instance, - promoted: Some(promoted.0), - })? - } - Base(PlaceBase::Static(ref static_)) => { - assert!(!static_.ty.needs_subst()); - let layout = self.layout_of(static_.ty)?; - let instance = ty::Instance::mono(*self.tcx, static_.def_id); - let cid = GlobalId { - instance, - promoted: None - }; - // Just create a lazy reference, so we can support recursive statics. - // tcx takes are of assigning every static one and only one unique AllocId. - // When the data here is ever actually used, memory will notice, - // and it knows how to deal with alloc_id that are present in the - // global table but not in its local memory: It calls back into tcx through - // a query, triggering the CTFE machinery to actually turn this lazy reference - // into a bunch of bytes. IOW, statics are evaluated with CTFE even when - // this EvalContext uses another Machine (e.g., in miri). This is what we - // want! This way, computing statics works concistently between codegen - // and miri: They use the same query to eventually obtain a `ty::Const` - // and use that for further computation. - let alloc = self.tcx.alloc_map.lock().intern_static(cid.instance.def_id()); - MPlaceTy::from_aligned_ptr(Pointer::from(alloc).with_default_tag(), layout) + match static_.promoted { + Some(promoted) => { + let instance = self.frame().instance; + self.const_eval_raw(GlobalId { + instance, + promoted: Some(promoted), + })? + } + None => { + assert!(!static_.ty.needs_subst()); + let layout = self.layout_of(static_.ty)?; + let instance = ty::Instance::mono(*self.tcx, static_.def_id); + let cid = GlobalId { + instance, + promoted: None + }; + // Just create a lazy reference, so we can support recursive statics. + // tcx takes are of assigning every static one and only one unique AllocId. + // When the data here is ever actually used, memory will notice, + // and it knows how to deal with alloc_id that are present in the + // global table but not in its local memory: It calls back into tcx through + // a query, triggering the CTFE machinery to actually turn this lazy reference + // into a bunch of bytes. IOW, statics are evaluated with CTFE even when + // this EvalContext uses another Machine (e.g., in miri). This is what we + // want! This way, computing statics works concistently between codegen + // and miri: They use the same query to eventually obtain a `ty::Const` + // and use that for further computation. + let alloc = self.tcx.alloc_map.lock().intern_static(cid.instance.def_id()); + MPlaceTy::from_aligned_ptr(Pointer::from(alloc).with_default_tag(), layout) + } + } } _ => bug!("eval_place_to_mplace called on {:?}", mir_place), diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs index 20b75c55867..b13a5fd2fd1 100644 --- a/src/librustc_mir/transform/add_retag.rs +++ b/src/librustc_mir/transform/add_retag.rs @@ -22,7 +22,6 @@ fn is_stable<'tcx>( match *place { // Locals and statics have stable addresses, for sure Base(PlaceBase::Local { .. }) | - Base(PlaceBase::Promoted { .. }) | Base(PlaceBase::Static { .. }) => true, // Recurse for projections diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index b494592c89f..01cb58a481a 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -300,29 +300,33 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { &Place::Base(PlaceBase::Local(..)) => { // locals are safe } - &Place::Base(PlaceBase::Promoted(_)) => { - bug!("unsafety checking should happen before promotion") - } - &Place::Base(PlaceBase::Static(box Static { def_id, ty: _ })) => { - if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) { - self.require_unsafe("use of mutable static", - "mutable statics can be mutated by multiple threads: aliasing violations \ - or data races will cause undefined behavior", - UnsafetyViolationKind::General); - } else if self.tcx.is_foreign_item(def_id) { - let source_info = self.source_info; - let lint_root = - self.source_scope_local_data[source_info.scope].lint_root; - self.register_violations(&[UnsafetyViolation { - source_info, - description: Symbol::intern("use of extern static").as_interned_str(), - details: - Symbol::intern("extern statics are not controlled by the Rust type \ - system: invalid data, aliasing violations or data \ - races will cause undefined behavior") - .as_interned_str(), - kind: UnsafetyViolationKind::ExternStatic(lint_root) - }], &[]); + &Place::Base(PlaceBase::Static(box Static { def_id, ty: _, promoted })) => { + match promoted { + Some(..) => { + bug!("unsafety checking should happen before promotion") + } + None => { + if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) { + self.require_unsafe("use of mutable static", + "mutable statics can be mutated by multiple threads: aliasing violations \ + or data races will cause undefined behavior", + UnsafetyViolationKind::General); + } else if self.tcx.is_foreign_item(def_id) { + let source_info = self.source_info; + let lint_root = + self.source_scope_local_data[source_info.scope].lint_root; + self.register_violations(&[UnsafetyViolation { + source_info, + description: Symbol::intern("use of extern static").as_interned_str(), + details: + Symbol::intern("extern statics are not controlled by the Rust type \ + system: invalid data, aliasing violations or data \ + races will cause undefined behavior") + .as_interned_str(), + kind: UnsafetyViolationKind::ExternStatic(lint_root) + }], &[]); + } + } } } }; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d8169684420..bbe62fa2d7b 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -282,25 +282,31 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { // an `Index` projection would throw us off-track. _ => None, }, - Place::Base(PlaceBase::Promoted(ref promoted)) => { - let generics = self.tcx.generics_of(self.source.def_id()); - if generics.requires_monomorphization(self.tcx) { - // FIXME: can't handle code with generics - return None; + Place::Base(PlaceBase::Static(ref static_)) => { + match static_.promoted { + Some(promoted) => { + let generics = self.tcx.generics_of(self.source.def_id()); + if generics.requires_monomorphization(self.tcx) { + // FIXME: can't handle code with generics + return None; + } + let substs = InternalSubsts::identity_for_item(self.tcx, self.source.def_id()); + let instance = Instance::new(self.source.def_id(), substs); + let cid = GlobalId { + instance, + promoted: Some(promoted), + }; + // cannot use `const_eval` here, because that would require having the MIR + // for the current function available, but we're producing said MIR right now + let res = self.use_ecx(source_info, |this| { + eval_promoted(this.tcx, cid, this.mir, this.param_env) + })?; + trace!("evaluated promoted {:?} to {:?}", promoted, res); + Some((res.into(), source_info.span)) + } + None => None } - let substs = InternalSubsts::identity_for_item(self.tcx, self.source.def_id()); - let instance = Instance::new(self.source.def_id(), substs); - let cid = GlobalId { - instance, - promoted: Some(promoted.0), - }; - // cannot use `const_eval` here, because that would require having the MIR - // for the current function available, but we're producing said MIR right now - let res = self.use_ecx(source_info, |this| { - eval_promoted(this.tcx, cid, this.mir, this.param_env) - })?; - trace!("evaluated promoted {:?} to {:?}", promoted, res); - Some((res.into(), source_info.span)) + }, _ => None, } diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 234483df13a..0b2a34d5fb9 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -692,12 +692,16 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { // Return pointer; update the place itself *place = self.destination.clone(); }, - Place::Base(PlaceBase::Promoted(ref mut promoted)) => { - if let Some(p) = self.promoted_map.get(promoted.0).cloned() { - promoted.0 = p; + Place::Base(PlaceBase::Static(ref mut static_)) => { + match static_.promoted { + Some(promoted) => { + if let Some(p) = self.promoted_map.get(promoted).cloned() { + static_.promoted = Some(p); + } + } + None => self.super_place(place, _ctxt, _location) } }, - _ => self.super_place(place, _ctxt, _location), } } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index e96689809ad..f0ba1306c99 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -188,8 +188,12 @@ trait Qualif { fn in_place(cx: &ConstCx<'_, 'tcx>, place: &Place<'tcx>) -> bool { match *place { Place::Base(PlaceBase::Local(local)) => Self::in_local(cx, local), - Place::Base(PlaceBase::Promoted(_)) => bug!("qualifying already promoted MIR"), - Place::Base(PlaceBase::Static(ref static_)) => Self::in_static(cx, static_), + Place::Base(PlaceBase::Static(ref static_)) => { + match static_.promoted { + Some(..) => bug!("qualifying already promoted MIR"), + None => Self::in_static(cx, static_), + } + }, Place::Projection(ref proj) => Self::in_projection(cx, proj), } } @@ -768,17 +772,20 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { ); dest = &proj.base; }, - Place::Base(PlaceBase::Promoted(..)) => - bug!("promoteds don't exist yet during promotion"), - Place::Base(PlaceBase::Static(..)) => { - // Catch more errors in the destination. `visit_place` also checks that we - // do not try to access statics from constants or try to mutate statics - self.visit_place( - dest, - PlaceContext::MutatingUse(MutatingUseContext::Store), - location - ); - return; + Place::Base(PlaceBase::Static(st)) => { + match st.promoted { + Some(..) => bug!("promoteds don't exist yet during promotion"), + None => { + // Catch more errors in the destination. `visit_place` also checks that we + // do not try to access statics from constants or try to mutate statics + self.visit_place( + dest, + PlaceContext::MutatingUse(MutatingUseContext::Store), + location + ); + return; + } + } } } }; @@ -919,51 +926,55 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { debug!("visit_place: place={:?} context={:?} location={:?}", place, context, location); self.super_place(place, context, location); match *place { - Place::Base(PlaceBase::Local(_)) | - Place::Base(PlaceBase::Promoted(_)) => {} + Place::Base(PlaceBase::Local(_)) => {} Place::Base(PlaceBase::Static(ref global)) => { - if self.tcx - .get_attrs(global.def_id) - .iter() - .any(|attr| attr.check_name("thread_local")) { - if self.mode != Mode::Fn { - span_err!(self.tcx.sess, self.span, E0625, - "thread-local statics cannot be \ - accessed at compile-time"); - } - return; - } + match global.promoted { + Some(..) => {} + None => { + if self.tcx + .get_attrs(global.def_id) + .iter() + .any(|attr| attr.check_name("thread_local")) { + if self.mode != Mode::Fn { + span_err!(self.tcx.sess, self.span, E0625, + "thread-local statics cannot be \ + accessed at compile-time"); + } + return; + } - // Only allow statics (not consts) to refer to other statics. - if self.mode == Mode::Static || self.mode == Mode::StaticMut { - if self.mode == Mode::Static && context.is_mutating_use() { - // this is not strictly necessary as miri will also bail out - // For interior mutability we can't really catch this statically as that - // goes through raw pointers and intermediate temporaries, so miri has - // to catch this anyway - self.tcx.sess.span_err( - self.span, - "cannot mutate statics in the initializer of another static", - ); - } - return; - } - unleash_miri!(self); + // Only allow statics (not consts) to refer to other statics. + if self.mode == Mode::Static || self.mode == Mode::StaticMut { + if self.mode == Mode::Static && context.is_mutating_use() { + // this is not strictly necessary as miri will also bail out + // For interior mutability we can't really catch this statically as that + // goes through raw pointers and intermediate temporaries, so miri has + // to catch this anyway + self.tcx.sess.span_err( + self.span, + "cannot mutate statics in the initializer of another static", + ); + } + return; + } + unleash_miri!(self); - if self.mode != Mode::Fn { - let mut err = struct_span_err!(self.tcx.sess, self.span, E0013, - "{}s cannot refer to statics, use \ - a constant instead", self.mode); - if self.tcx.sess.teach(&err.get_code().unwrap()) { - err.note( - "Static and const variables can refer to other const variables. But a \ - const variable cannot refer to a static variable." - ); - err.help( - "To fix this, the value can be extracted as a const and then used." - ); + if self.mode != Mode::Fn { + let mut err = struct_span_err!(self.tcx.sess, self.span, E0013, + "{}s cannot refer to statics, use \ + a constant instead", self.mode); + if self.tcx.sess.teach(&err.get_code().unwrap()) { + err.note( + "Static and const variables can refer to other const variables. But a \ + const variable cannot refer to a static variable." + ); + err.help( + "To fix this, the value can be extracted as a const and then used." + ); + } + err.emit() + } } - err.emit() } } Place::Projection(ref proj) => { diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index f82e536ab25..ea5bbaeb400 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -256,10 +256,13 @@ fn check_place( ) -> McfResult { match place { Place::Base(PlaceBase::Local(_)) => Ok(()), - // promoteds are always fine, they are essentially constants - Place::Base(PlaceBase::Promoted(_)) => Ok(()), - Place::Base(PlaceBase::Static(_)) => - Err((span, "cannot access `static` items in const fn".into())), + Place::Base(PlaceBase::Static(st)) => { + match st.promoted { + // promoteds are always fine, they are essentially constants + Some(..) => Ok(()), + None => Err((span, "cannot access `static` items in const fn".into())), + } + } Place::Projection(proj) => { match proj.elem { | ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } From a837b8a3688d746bc392a65524fe7d06fdb61263 Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Sun, 17 Mar 2019 11:12:39 +0530 Subject: [PATCH 2/8] cleaner code as per review --- src/librustc/mir/visit.rs | 10 -- .../borrow_check/error_reporting.rs | 11 +- src/librustc_mir/borrow_check/mod.rs | 35 ++---- .../borrow_check/mutability_errors.rs | 20 ++- .../borrow_check/nll/type_check/mod.rs | 16 +-- src/librustc_mir/borrow_check/place_ext.rs | 9 +- .../borrow_check/places_conflict.rs | 4 +- src/librustc_mir/interpret/place.rs | 62 +++++----- src/librustc_mir/transform/check_unsafety.rs | 47 ++++--- src/librustc_mir/transform/const_prop.rs | 43 +++---- src/librustc_mir/transform/qualify_consts.rs | 116 ++++++++---------- 11 files changed, 160 insertions(+), 213 deletions(-) diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index cddf5782121..6761c70550e 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -737,16 +737,6 @@ macro_rules! make_mir_visitor { self.visit_local(local, context, location); } Place::Base(PlaceBase::Static(static_)) => { - match static_.promoted { - None => { - self.visit_static(static_, context, location); - } - Some(_) => { - self.visit_ty( - & $($mutability)? static_.ty, TyContext::Location(location) - ); - } - } self.visit_static(static_, context, location); } Place::Projection(proj) => { diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 8f83c025ceb..b9877945e7e 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -1602,13 +1602,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.append_local_to_string(local, buf)?; } Place::Base(PlaceBase::Static(ref static_)) => { - match static_.promoted { - Some(_) => { - buf.push_str("promoted"); - } - None => { - buf.push_str(&self.infcx.tcx.item_name(static_.def_id).to_string()); - } + if static_.promoted.is_some() { + buf.push_str("promoted"); + } else { + buf.push_str(&self.infcx.tcx.item_name(static_.def_id).to_string()); } } Place::Projection(ref proj) => { diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 5726eba9bda..e92ed1f2c7c 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1309,15 +1309,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // FIXME: allow thread-locals to borrow other thread locals? let (might_be_alive, will_be_dropped) = match root_place { Place::Base(PlaceBase::Static(st)) => { - match st.promoted { - None => { - // Thread-locals might be dropped after the function exits, but - // "true" statics will never be. - let is_thread_local = self.is_place_thread_local(&root_place); - (true, is_thread_local) - } - Some(_) => (true, false), - } + (true, st.promoted.is_none() && self.is_place_thread_local(&root_place)) } Place::Base(PlaceBase::Local(_)) => { // Locals are always dropped at function exit, and if they @@ -1990,28 +1982,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } // The rules for promotion are made by `qualify_consts`, there wouldn't even be a // `Place::Promoted` if the promotion weren't 100% legal. So we just forward this -// Place::Base(PlaceBase::Promoted(_)) => Ok(RootPlace { -// place, -// is_local_mutation_allowed, -// }), Place::Base(PlaceBase::Static(ref static_)) => { - match static_.promoted { - Some(_) => { + if static_.promoted.is_some() { + Ok(RootPlace { + place, + is_local_mutation_allowed, + }) + } else { + if self.infcx.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) { + Err(place) + } else { Ok(RootPlace { place, is_local_mutation_allowed, }) } - None => { - if self.infcx.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) { - Err(place) - } else { - Ok(RootPlace { - place, - is_local_mutation_allowed, - }) - } - } } } Place::Projection(ref proj) => { diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index d8c39e0715a..2661d765718 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -130,18 +130,14 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { } Place::Base(PlaceBase::Static(box Static { def_id, ty: _, promoted })) => { - match promoted { - Some(_) => unreachable!(), - None => { - if let Place::Base(PlaceBase::Static(_)) = access_place { - item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); - reason = String::new(); - } else { - item_msg = format!("`{}`", access_place_desc.unwrap()); - let static_name = &self.infcx.tcx.item_name(*def_id); - reason = format!(", as `{}` is an immutable static item", static_name); - } - } + assert!(promoted.is_none()); + if let Place::Base(PlaceBase::Static(_)) = access_place { + item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); + reason = String::new(); + } else { + item_msg = format!("`{}`", access_place_desc.unwrap()); + let static_name = &self.infcx.tcx.item_name(*def_id); + reason = format!(", as `{}` is an immutable static item", static_name); } } diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 334c20761e7..c2efd3625c3 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -454,10 +454,9 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { ty: self.mir.local_decls[index].ty, }, Place::Base(PlaceBase::Static(box Static { def_id, ty: sty, promoted })) => { + let sty = self.sanitize_type(place, sty); match promoted { Some(pr) => { - let sty = self.sanitize_type(place, sty); - if !self.errors_reported { let promoted_mir = &self.mir.promoted[pr]; self.sanitize_promoted(promoted_mir, location); @@ -480,15 +479,18 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { ); }; } - PlaceTy::Ty { ty: sty } } None => { - let sty = self.sanitize_type(place, sty); let ty = self.tcx().type_of(def_id); let ty = self.cx.normalize(ty, location); if let Err(terr) = self.cx - .eq_types(ty, sty, location.to_locations(), ConstraintCategory::Boring) + .eq_types( + ty, + sty, + location.to_locations(), + ConstraintCategory::Boring + ) { span_mirbug!( self, @@ -498,10 +500,10 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { sty, terr ); - } - PlaceTy::Ty { ty: sty } + }; } } + PlaceTy::Ty { ty: sty } } Place::Projection(ref proj) => { let base_context = if context.is_mutating_use() { diff --git a/src/librustc_mir/borrow_check/place_ext.rs b/src/librustc_mir/borrow_check/place_ext.rs index 0bbd147d824..5e691db0a2e 100644 --- a/src/librustc_mir/borrow_check/place_ext.rs +++ b/src/librustc_mir/borrow_check/place_ext.rs @@ -50,11 +50,10 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { } } Place::Base(PlaceBase::Static(static_)) => { - match static_.promoted { - Some(_) => false, - None => { - tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable) - } + if static_.promoted.is_none() { + tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable) + } else { + false } } Place::Projection(proj) => match proj.elem { diff --git a/src/librustc_mir/borrow_check/places_conflict.rs b/src/librustc_mir/borrow_check/places_conflict.rs index 7babdfcdd68..c4f9df0cfe5 100644 --- a/src/librustc_mir/borrow_check/places_conflict.rs +++ b/src/librustc_mir/borrow_check/places_conflict.rs @@ -387,7 +387,7 @@ fn place_element_conflict<'a, 'gcx: 'tcx, 'tcx>( }, (Some(p1), Some(p2)) => { if p1 == p2 { - if let ty::Array(_, size) =s1.ty.sty { + if let ty::Array(_, size) = s1.ty.sty { if size.unwrap_usize(tcx) == 0 { // Ignore conflicts with promoted [T; 0]. debug!("place_element_conflict: IGNORE-LEN-0-PROMOTED"); @@ -404,7 +404,7 @@ fn place_element_conflict<'a, 'gcx: 'tcx, 'tcx>( } }, (p1_, p2_) => { - debug!("place_element_conflict: DISJOINT-STATIC-LOCAL-PROMOTED"); + debug!("place_element_conflict: DISJOINT-STATIC-PROMOTED"); Overlap::Disjoint } } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 62e2cdffadf..7adcb118f84 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -582,39 +582,37 @@ where ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { use rustc::mir::Place::*; use rustc::mir::PlaceBase; + use rustc::mir::Static; Ok(match *mir_place { - Base(PlaceBase::Static(ref static_)) => { - match static_.promoted { - Some(promoted) => { - let instance = self.frame().instance; - self.const_eval_raw(GlobalId { - instance, - promoted: Some(promoted), - })? - } - None => { - assert!(!static_.ty.needs_subst()); - let layout = self.layout_of(static_.ty)?; - let instance = ty::Instance::mono(*self.tcx, static_.def_id); - let cid = GlobalId { - instance, - promoted: None - }; - // Just create a lazy reference, so we can support recursive statics. - // tcx takes are of assigning every static one and only one unique AllocId. - // When the data here is ever actually used, memory will notice, - // and it knows how to deal with alloc_id that are present in the - // global table but not in its local memory: It calls back into tcx through - // a query, triggering the CTFE machinery to actually turn this lazy reference - // into a bunch of bytes. IOW, statics are evaluated with CTFE even when - // this EvalContext uses another Machine (e.g., in miri). This is what we - // want! This way, computing statics works concistently between codegen - // and miri: They use the same query to eventually obtain a `ty::Const` - // and use that for further computation. - let alloc = self.tcx.alloc_map.lock().intern_static(cid.instance.def_id()); - MPlaceTy::from_aligned_ptr(Pointer::from(alloc).with_default_tag(), layout) - } - } + Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty, ..})) => { + let instance = self.frame().instance; + self.const_eval_raw(GlobalId { + instance, + promoted: Some(promoted), + })? + } + + Base(PlaceBase::Static(box Static {promoted: None, ty, def_id})) => { + assert!(!ty.needs_subst()); + let layout = self.layout_of(ty)?; + let instance = ty::Instance::mono(*self.tcx, def_id); + let cid = GlobalId { + instance, + promoted: None + }; + // Just create a lazy reference, so we can support recursive statics. + // tcx takes are of assigning every static one and only one unique AllocId. + // When the data here is ever actually used, memory will notice, + // and it knows how to deal with alloc_id that are present in the + // global table but not in its local memory: It calls back into tcx through + // a query, triggering the CTFE machinery to actually turn this lazy reference + // into a bunch of bytes. IOW, statics are evaluated with CTFE even when + // this EvalContext uses another Machine (e.g., in miri). This is what we + // want! This way, computing statics works concistently between codegen + // and miri: They use the same query to eventually obtain a `ty::Const` + // and use that for further computation. + let alloc = self.tcx.alloc_map.lock().intern_static(cid.instance.def_id()); + MPlaceTy::from_aligned_ptr(Pointer::from(alloc).with_default_tag(), layout) } _ => bug!("eval_place_to_mplace called on {:?}", mir_place), diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 01cb58a481a..2ebe28e6ac0 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -301,32 +301,27 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { // locals are safe } &Place::Base(PlaceBase::Static(box Static { def_id, ty: _, promoted })) => { - match promoted { - Some(..) => { - bug!("unsafety checking should happen before promotion") - } - None => { - if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) { - self.require_unsafe("use of mutable static", - "mutable statics can be mutated by multiple threads: aliasing violations \ - or data races will cause undefined behavior", - UnsafetyViolationKind::General); - } else if self.tcx.is_foreign_item(def_id) { - let source_info = self.source_info; - let lint_root = - self.source_scope_local_data[source_info.scope].lint_root; - self.register_violations(&[UnsafetyViolation { - source_info, - description: Symbol::intern("use of extern static").as_interned_str(), - details: - Symbol::intern("extern statics are not controlled by the Rust type \ - system: invalid data, aliasing violations or data \ - races will cause undefined behavior") - .as_interned_str(), - kind: UnsafetyViolationKind::ExternStatic(lint_root) - }], &[]); - } - } + assert!(promoted.is_none(), "unsafety checking should happen before promotion"); + + if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) { + self.require_unsafe("use of mutable static", + "mutable statics can be mutated by multiple threads: aliasing violations \ + or data races will cause undefined behavior", + UnsafetyViolationKind::General); + } else if self.tcx.is_foreign_item(def_id) { + let source_info = self.source_info; + let lint_root = + self.source_scope_local_data[source_info.scope].lint_root; + self.register_violations(&[UnsafetyViolation { + source_info, + description: Symbol::intern("use of extern static").as_interned_str(), + details: + Symbol::intern("extern statics are not controlled by the Rust type \ + system: invalid data, aliasing violations or data \ + races will cause undefined behavior") + .as_interned_str(), + kind: UnsafetyViolationKind::ExternStatic(lint_root) + }], &[]); } } }; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index bbe62fa2d7b..3ee26e7dd26 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -266,6 +266,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { } fn eval_place(&mut self, place: &Place<'tcx>, source_info: SourceInfo) -> Option> { + use rustc::mir::Static; match *place { Place::Base(PlaceBase::Local(loc)) => self.places[loc].clone(), Place::Projection(ref proj) => match proj.elem { @@ -282,31 +283,25 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { // an `Index` projection would throw us off-track. _ => None, }, - Place::Base(PlaceBase::Static(ref static_)) => { - match static_.promoted { - Some(promoted) => { - let generics = self.tcx.generics_of(self.source.def_id()); - if generics.requires_monomorphization(self.tcx) { - // FIXME: can't handle code with generics - return None; - } - let substs = InternalSubsts::identity_for_item(self.tcx, self.source.def_id()); - let instance = Instance::new(self.source.def_id(), substs); - let cid = GlobalId { - instance, - promoted: Some(promoted), - }; - // cannot use `const_eval` here, because that would require having the MIR - // for the current function available, but we're producing said MIR right now - let res = self.use_ecx(source_info, |this| { - eval_promoted(this.tcx, cid, this.mir, this.param_env) - })?; - trace!("evaluated promoted {:?} to {:?}", promoted, res); - Some((res.into(), source_info.span)) - } - None => None + Place::Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty, ..})) => { + let generics = self.tcx.generics_of(self.source.def_id()); + if generics.requires_monomorphization(self.tcx) { + // FIXME: can't handle code with generics + return None; } - + let substs = InternalSubsts::identity_for_item(self.tcx, self.source.def_id()); + let instance = Instance::new(self.source.def_id(), substs); + let cid = GlobalId { + instance, + promoted: Some(promoted), + }; + // cannot use `const_eval` here, because that would require having the MIR + // for the current function available, but we're producing said MIR right now + let res = self.use_ecx(source_info, |this| { + eval_promoted(this.tcx, cid, this.mir, this.param_env) + })?; + trace!("evaluated promoted {:?} to {:?}", promoted, res); + Some((res.into(), source_info.span)) }, _ => None, } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index f0ba1306c99..9cbb891316d 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -189,10 +189,8 @@ trait Qualif { match *place { Place::Base(PlaceBase::Local(local)) => Self::in_local(cx, local), Place::Base(PlaceBase::Static(ref static_)) => { - match static_.promoted { - Some(..) => bug!("qualifying already promoted MIR"), - None => Self::in_static(cx, static_), - } + assert!(static_.promoted.is_none(), "qualifying already promoted MIR"); + Self::in_static(cx, static_) }, Place::Projection(ref proj) => Self::in_projection(cx, proj), } @@ -773,19 +771,15 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { dest = &proj.base; }, Place::Base(PlaceBase::Static(st)) => { - match st.promoted { - Some(..) => bug!("promoteds don't exist yet during promotion"), - None => { - // Catch more errors in the destination. `visit_place` also checks that we - // do not try to access statics from constants or try to mutate statics - self.visit_place( - dest, - PlaceContext::MutatingUse(MutatingUseContext::Store), - location - ); - return; - } - } + assert!(st.promoted.is_none(), "promoteds don't exist yet during promotion"); + // Catch more errors in the destination. `visit_place` also checks that we + // do not try to access statics from constants or try to mutate statics + self.visit_place( + dest, + PlaceContext::MutatingUse(MutatingUseContext::Store), + location + ); + return; } } }; @@ -928,53 +922,49 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { match *place { Place::Base(PlaceBase::Local(_)) => {} Place::Base(PlaceBase::Static(ref global)) => { - match global.promoted { - Some(..) => {} - None => { - if self.tcx - .get_attrs(global.def_id) - .iter() - .any(|attr| attr.check_name("thread_local")) { - if self.mode != Mode::Fn { - span_err!(self.tcx.sess, self.span, E0625, - "thread-local statics cannot be \ - accessed at compile-time"); - } - return; - } - - // Only allow statics (not consts) to refer to other statics. - if self.mode == Mode::Static || self.mode == Mode::StaticMut { - if self.mode == Mode::Static && context.is_mutating_use() { - // this is not strictly necessary as miri will also bail out - // For interior mutability we can't really catch this statically as that - // goes through raw pointers and intermediate temporaries, so miri has - // to catch this anyway - self.tcx.sess.span_err( - self.span, - "cannot mutate statics in the initializer of another static", - ); - } - return; - } - unleash_miri!(self); - - if self.mode != Mode::Fn { - let mut err = struct_span_err!(self.tcx.sess, self.span, E0013, - "{}s cannot refer to statics, use \ - a constant instead", self.mode); - if self.tcx.sess.teach(&err.get_code().unwrap()) { - err.note( - "Static and const variables can refer to other const variables. But a \ - const variable cannot refer to a static variable." - ); - err.help( - "To fix this, the value can be extracted as a const and then used." - ); - } - err.emit() - } + assert!(global.promoted.is_none(), {}); + if self.tcx + .get_attrs(global.def_id) + .iter() + .any(|attr| attr.check_name("thread_local")) { + if self.mode != Mode::Fn { + span_err!(self.tcx.sess, self.span, E0625, + "thread-local statics cannot be \ + accessed at compile-time"); } + return; + } + + // Only allow statics (not consts) to refer to other statics. + if self.mode == Mode::Static || self.mode == Mode::StaticMut { + if self.mode == Mode::Static && context.is_mutating_use() { + // this is not strictly necessary as miri will also bail out + // For interior mutability we can't really catch this statically as that + // goes through raw pointers and intermediate temporaries, so miri has + // to catch this anyway + self.tcx.sess.span_err( + self.span, + "cannot mutate statics in the initializer of another static", + ); + } + return; + } + unleash_miri!(self); + + if self.mode != Mode::Fn { + let mut err = struct_span_err!(self.tcx.sess, self.span, E0013, + "{}s cannot refer to statics, use \ + a constant instead", self.mode); + if self.tcx.sess.teach(&err.get_code().unwrap()) { + err.note( + "Static and const variables can refer to other const variables. But a \ + const variable cannot refer to a static variable." + ); + err.help( + "To fix this, the value can be extracted as a const and then used." + ); + } + err.emit() } } Place::Projection(ref proj) => { From 23c87a1f53a502cfca3ced42a33cf1389f6c081e Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Mon, 18 Mar 2019 14:51:08 +0530 Subject: [PATCH 3/8] fixed all compilation errors --- src/librustc_codegen_ssa/mir/block.rs | 12 ++++++++---- src/librustc_codegen_ssa/mir/place.rs | 10 +++++++--- src/librustc_mir/borrow_check/mod.rs | 14 +++++--------- src/librustc_mir/borrow_check/place_ext.rs | 7 ++----- .../borrow_check/places_conflict.rs | 2 +- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 2 +- src/librustc_mir/transform/inline.rs | 1 + src/librustc_mir/transform/promote_consts.rs | 17 +++++++++++++---- src/librustc_mir/transform/qualify_consts.rs | 4 ++-- 10 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 35fd30f34ad..62f454f1b9f 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -1,7 +1,7 @@ use rustc::middle::lang_items; use rustc::ty::{self, Ty, TypeFoldable}; use rustc::ty::layout::{self, LayoutOf, HasTyCtxt}; -use rustc::mir; +use rustc::mir::{self, Place, PlaceBase}; use rustc::mir::interpret::EvalErrorKind; use rustc_target::abi::call::{ArgType, FnType, PassMode, IgnoreMode}; use rustc_target::spec::abi::Abi; @@ -621,15 +621,19 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // but specified directly in the code. This means it gets promoted // and we can then extract the value by evaluating the promoted. mir::Operand::Copy( - mir::Place::Base(mir::PlaceBase::Promoted(box(index, ty))) + Place::Base(PlaceBase::Static( + box mir::Static {promoted: Some(promoted), ty, ..} + )) ) | mir::Operand::Move( - mir::Place::Base(mir::PlaceBase::Promoted(box(index, ty))) + Place::Base(PlaceBase::Static( + box mir::Static {promoted: Some(promoted), ty, ..} + )) ) => { let param_env = ty::ParamEnv::reveal_all(); let cid = mir::interpret::GlobalId { instance: self.instance, - promoted: Some(index), + promoted: Some(promoted), }; let c = bx.tcx().const_eval(param_env.and(cid)); let (llval, ty) = self.simd_shuffle_indices( diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index 0408ccf039f..1608429b070 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -408,11 +408,13 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let result = match *place { mir::Place::Base(mir::PlaceBase::Local(_)) => bug!(), // handled above - mir::Place::Base(mir::PlaceBase::Promoted(box (index, ty))) => { + mir::Place::Base( + mir::PlaceBase::Static(box mir::Static { def_id: _, ty, promoted: Some(promoted) }) + ) => { let param_env = ty::ParamEnv::reveal_all(); let cid = mir::interpret::GlobalId { instance: self.instance, - promoted: Some(index), + promoted: Some(promoted), }; let layout = cx.layout_of(self.monomorphize(&ty)); match bx.tcx().const_eval(param_env.and(cid)) { @@ -435,7 +437,9 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } } } - mir::Place::Base(mir::PlaceBase::Static(box mir::Static { def_id, ty })) => { + mir::Place::Base( + mir::PlaceBase::Static(box mir::Static { def_id, ty, promoted: None }) + ) => { // NB: The layout of a static may be unsized as is the case when working // with a static that is an extern_type. let layout = cx.layout_of(self.monomorphize(&ty)); diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index e92ed1f2c7c..e538622103a 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1983,20 +1983,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // The rules for promotion are made by `qualify_consts`, there wouldn't even be a // `Place::Promoted` if the promotion weren't 100% legal. So we just forward this Place::Base(PlaceBase::Static(ref static_)) => { - if static_.promoted.is_some() { + if static_.promoted.is_some() || + (static_.promoted.is_none() && + self.infcx.tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable) + ){ Ok(RootPlace { place, is_local_mutation_allowed, }) } else { - if self.infcx.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) { - Err(place) - } else { - Ok(RootPlace { - place, - is_local_mutation_allowed, - }) - } + Err(place) } } Place::Projection(ref proj) => { diff --git a/src/librustc_mir/borrow_check/place_ext.rs b/src/librustc_mir/borrow_check/place_ext.rs index 5e691db0a2e..0452465f0b9 100644 --- a/src/librustc_mir/borrow_check/place_ext.rs +++ b/src/librustc_mir/borrow_check/place_ext.rs @@ -50,11 +50,8 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { } } Place::Base(PlaceBase::Static(static_)) => { - if static_.promoted.is_none() { - tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable) - } else { - false - } + static_.promoted.is_none() && + (tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable)) } Place::Projection(proj) => match proj.elem { ProjectionElem::Field(..) diff --git a/src/librustc_mir/borrow_check/places_conflict.rs b/src/librustc_mir/borrow_check/places_conflict.rs index c4f9df0cfe5..783751056d7 100644 --- a/src/librustc_mir/borrow_check/places_conflict.rs +++ b/src/librustc_mir/borrow_check/places_conflict.rs @@ -403,7 +403,7 @@ fn place_element_conflict<'a, 'gcx: 'tcx, 'tcx>( Overlap::Disjoint } }, - (p1_, p2_) => { + (_, _) => { debug!("place_element_conflict: DISJOINT-STATIC-PROMOTED"); Overlap::Disjoint } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 7adcb118f84..6089e491684 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -584,7 +584,7 @@ where use rustc::mir::PlaceBase; use rustc::mir::Static; Ok(match *mir_place { - Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty, ..})) => { + Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty: _, ..})) => { let instance = self.frame().instance; self.const_eval_raw(GlobalId { instance, diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 3ee26e7dd26..c5c3a1b8eca 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -283,7 +283,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { // an `Index` projection would throw us off-track. _ => None, }, - Place::Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty, ..})) => { + Place::Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty: _, ..})) => { let generics = self.tcx.generics_of(self.source.def_id()); if generics.requires_monomorphization(self.tcx) { // FIXME: can't handle code with generics diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 0b2a34d5fb9..86b7da17879 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -702,6 +702,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { None => self.super_place(place, _ctxt, _location) } }, + _ => self.super_place(place, _ctxt, _location) } } diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 831d8b46a65..d777a7b362b 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -12,6 +12,7 @@ //! initialization and can otherwise silence errors, if //! move analysis runs after promotion on broken MIR. +use rustc::hir::def_id::DefId; use rustc::mir::*; use rustc::mir::visit::{PlaceContext, MutatingUseContext, MutVisitor, Visitor}; use rustc::mir::traversal::ReversePostorder; @@ -151,7 +152,8 @@ struct Promoter<'a, 'tcx: 'a> { /// If true, all nested temps are also kept in the /// source MIR, not moved to the promoted MIR. - keep_original: bool + keep_original: bool, + def_id: DefId } impl<'a, 'tcx> Promoter<'a, 'tcx> { @@ -287,14 +289,19 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { } fn promote_candidate(mut self, candidate: Candidate) { + use rustc::mir::Static; let mut operand = { + let def_id = self.def_id.clone(); let promoted = &mut self.promoted; let promoted_id = Promoted::new(self.source.promoted.len()); let mut promoted_place = |ty, span| { promoted.span = span; promoted.local_decls[RETURN_PLACE] = LocalDecl::new_return_place(ty, span); - Place::Base(PlaceBase::Promoted(box (promoted_id, ty))) + Place::Base(PlaceBase::Static( + Box::new(Static { def_id: def_id, ty, promoted: Some(promoted_id) }) + ) + ) }; let (blocks, local_decls) = self.source.basic_blocks_and_local_decls_mut(); match candidate { @@ -367,7 +374,8 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Promoter<'a, 'tcx> { pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>, mut temps: IndexVec, - candidates: Vec) { + candidates: Vec, + def_id: DefId) { // Visit candidates in reverse, in case they're nested. debug!("promote_candidates({:?})", candidates); @@ -412,7 +420,8 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, tcx, source: mir, temps: &mut temps, - keep_original: false + keep_original: false, + def_id }; promoter.promote_candidate(candidate); } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 9cbb891316d..c01ed4b1c59 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -922,7 +922,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { match *place { Place::Base(PlaceBase::Local(_)) => {} Place::Base(PlaceBase::Static(ref global)) => { - assert!(global.promoted.is_none(), {}); + assert!(global.promoted.is_none()); if self.tcx .get_attrs(global.def_id) .iter() @@ -1516,7 +1516,7 @@ impl MirPass for QualifyAndPromoteConstants { }; // Do the actual promotion, now that we know what's viable. - promote_consts::promote_candidates(mir, tcx, temps, candidates); + promote_consts::promote_candidates(mir, tcx, temps, candidates, def_id); } else { if !mir.control_flow_destroyed.is_empty() { let mut locals = mir.vars_iter(); From 776407e4e65e537ee7471a90d8a48eb26e7f0fd7 Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Tue, 19 Mar 2019 13:35:50 +0530 Subject: [PATCH 4/8] tidy checks --- src/librustc_mir/borrow_check/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index e538622103a..0358fffcde5 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1985,7 +1985,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Base(PlaceBase::Static(ref static_)) => { if static_.promoted.is_some() || (static_.promoted.is_none() && - self.infcx.tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable) + self.infcx.tcx.is_static(static_.def_id) + == Some(hir::Mutability::MutMutable) ){ Ok(RootPlace { place, From 8829ddadc4f82b43a8653dd1f40839513b6fb2f0 Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Wed, 20 Mar 2019 19:05:03 +0530 Subject: [PATCH 5/8] remove visit_static from librustc::mir --- src/librustc/mir/visit.rs | 21 ++++------------- src/librustc_mir/monomorphize/collector.rs | 23 ++++++++++++------- .../transform/qualify_min_const_fn.rs | 11 ++++----- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 6761c70550e..3f6133a7331 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -156,13 +156,6 @@ macro_rules! make_mir_visitor { self.super_place(place, context, location); } - fn visit_static(&mut self, - static_: & $($mutability)? Static<'tcx>, - context: PlaceContext<'tcx>, - location: Location) { - self.super_static(static_, context, location); - } - fn visit_projection(&mut self, place: & $($mutability)? PlaceProjection<'tcx>, context: PlaceContext<'tcx>, @@ -737,7 +730,10 @@ macro_rules! make_mir_visitor { self.visit_local(local, context, location); } Place::Base(PlaceBase::Static(static_)) => { - self.visit_static(static_, context, location); + if static_.promoted.is_none() { + self.visit_def_id(& $($mutability)? static_.def_id, location); + } + self.visit_ty(& $($mutability)? static_.ty, TyContext::Location(location)); } Place::Projection(proj) => { self.visit_projection(proj, context, location); @@ -745,15 +741,6 @@ macro_rules! make_mir_visitor { } } - fn super_static(&mut self, - static_: & $($mutability)? Static<'tcx>, - _context: PlaceContext<'tcx>, - location: Location) { - let Static { def_id, ty, promoted: _ } = static_; - self.visit_def_id(def_id, location); - self.visit_ty(ty, TyContext::Location(location)); - } - fn super_projection(&mut self, proj: & $($mutability)? PlaceProjection<'tcx>, context: PlaceContext<'tcx>, diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 4fe47a9666a..075b81e9fd9 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -650,19 +650,26 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { self.super_terminator_kind(block, kind, location); } - fn visit_static(&mut self, - static_: &mir::Static<'tcx>, + fn visit_place(&mut self, + place: &mir::Place<'tcx>, context: mir::visit::PlaceContext<'tcx>, location: Location) { - debug!("visiting static {:?} @ {:?}", static_.def_id, location); + match place { + mir::Place::Base( + mir::PlaceBase::Static(box mir::Static{def_id, promoted:None, ..}) + ) => { + debug!("visiting static {:?} @ {:?}", def_id, location); - let tcx = self.tcx; - let instance = Instance::mono(tcx, static_.def_id); - if should_monomorphize_locally(tcx, &instance) { - self.output.push(MonoItem::Static(static_.def_id)); + let tcx = self.tcx; + let instance = Instance::mono(tcx, *def_id); + if should_monomorphize_locally(tcx, &instance) { + self.output.push(MonoItem::Static(*def_id)); + } + } + _ => {} } - self.super_static(static_, context, location); + self.super_place(place, context, location); } } diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index ea5bbaeb400..da849faf40a 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -256,13 +256,10 @@ fn check_place( ) -> McfResult { match place { Place::Base(PlaceBase::Local(_)) => Ok(()), - Place::Base(PlaceBase::Static(st)) => { - match st.promoted { - // promoteds are always fine, they are essentially constants - Some(..) => Ok(()), - None => Err((span, "cannot access `static` items in const fn".into())), - } - } + // promoteds are always fine, they are essentially constants + Place::Base(PlaceBase::Static(box Static {def_id: _, ty: _, promoted: Some(_)})) => Ok(()), + Place::Base(PlaceBase::Static(box Static {def_id: _, ty: _, promoted: None})) => + Err((span, "cannot access `static` items in const fn".into())), Place::Projection(proj) => { match proj.elem { | ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } From cf2f1bb072c76ed45882e1a69ed7b8ec96bad0e9 Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Thu, 21 Mar 2019 00:37:08 +0530 Subject: [PATCH 6/8] review fixes --- src/librustc_codegen_ssa/mir/block.rs | 8 +-- .../borrow_check/nll/type_check/mod.rs | 58 ++++++++----------- src/librustc_mir/transform/const_prop.rs | 2 +- src/librustc_mir/transform/inline.rs | 11 +--- src/librustc_mir/transform/promote_consts.rs | 15 ++--- 5 files changed, 38 insertions(+), 56 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 62f454f1b9f..066b38be3db 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -622,13 +622,13 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // and we can then extract the value by evaluating the promoted. mir::Operand::Copy( Place::Base(PlaceBase::Static( - box mir::Static {promoted: Some(promoted), ty, ..} - )) + box mir::Static {promoted: Some(promoted), ty, ..} + )) ) | mir::Operand::Move( Place::Base(PlaceBase::Static( - box mir::Static {promoted: Some(promoted), ty, ..} - )) + box mir::Static {promoted: Some(promoted), ty, ..} + )) ) => { let param_env = ty::ParamEnv::reveal_all(); let cid = mir::interpret::GlobalId { diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index c2efd3625c3..2a32b475c79 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -455,6 +455,27 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { }, Place::Base(PlaceBase::Static(box Static { def_id, ty: sty, promoted })) => { let sty = self.sanitize_type(place, sty); + let check_err = + |verifier: &mut TypeVerifier<'a, 'b, 'gcx, 'tcx> , + place: &Place<'tcx>, + ty, + sty| { + if let Err(terr) = verifier.cx.eq_types( + sty, + ty, + location.to_locations(), + ConstraintCategory::Boring, + ) { + span_mirbug!( + verifier, + place, + "bad promoted type ({:?}: {:?}): {:?}", + ty, + sty, + terr + ); + }; + }; match promoted { Some(pr) => { if !self.errors_reported { @@ -462,45 +483,14 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { self.sanitize_promoted(promoted_mir, location); let promoted_ty = promoted_mir.return_ty(); - - if let Err(terr) = self.cx.eq_types( - sty, - promoted_ty, - location.to_locations(), - ConstraintCategory::Boring, - ) { - span_mirbug!( - self, - place, - "bad promoted type ({:?}: {:?}): {:?}", - promoted_ty, - sty, - terr - ); - }; + check_err(self, place, promoted_ty, sty); } } None => { let ty = self.tcx().type_of(def_id); let ty = self.cx.normalize(ty, location); - if let Err(terr) = - self.cx - .eq_types( - ty, - sty, - location.to_locations(), - ConstraintCategory::Boring - ) - { - span_mirbug!( - self, - place, - "bad static type ({:?}: {:?}): {:?}", - ty, - sty, - terr - ); - }; + + check_err(self, place, ty, sty); } } PlaceTy::Ty { ty: sty } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index c5c3a1b8eca..81cdb001005 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -283,7 +283,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { // an `Index` projection would throw us off-track. _ => None, }, - Place::Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty: _, ..})) => { + Place::Base(PlaceBase::Static(box Static {promoted: Some(promoted), ..})) => { let generics = self.tcx.generics_of(self.source.def_id()); if generics.requires_monomorphization(self.tcx) { // FIXME: can't handle code with generics diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 86b7da17879..ec2cf8a4c03 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -692,14 +692,9 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { // Return pointer; update the place itself *place = self.destination.clone(); }, - Place::Base(PlaceBase::Static(ref mut static_)) => { - match static_.promoted { - Some(promoted) => { - if let Some(p) = self.promoted_map.get(promoted).cloned() { - static_.promoted = Some(p); - } - } - None => self.super_place(place, _ctxt, _location) + Place::Base(PlaceBase::Static(box Static { promoted: Some(promoted), .. })) => { + if let Some(p) = self.promoted_map.get(*promoted).cloned() { + *promoted = p; } }, _ => self.super_place(place, _ctxt, _location) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index d777a7b362b..2344f070ea6 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -153,7 +153,7 @@ struct Promoter<'a, 'tcx: 'a> { /// If true, all nested temps are also kept in the /// source MIR, not moved to the promoted MIR. keep_original: bool, - def_id: DefId + def_id: DefId, } impl<'a, 'tcx> Promoter<'a, 'tcx> { @@ -291,17 +291,14 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { fn promote_candidate(mut self, candidate: Candidate) { use rustc::mir::Static; let mut operand = { - let def_id = self.def_id.clone(); + let def_id = self.def_id; let promoted = &mut self.promoted; let promoted_id = Promoted::new(self.source.promoted.len()); let mut promoted_place = |ty, span| { promoted.span = span; - promoted.local_decls[RETURN_PLACE] = - LocalDecl::new_return_place(ty, span); - Place::Base(PlaceBase::Static( - Box::new(Static { def_id: def_id, ty, promoted: Some(promoted_id) }) - ) - ) + promoted.local_decls[RETURN_PLACE] = LocalDecl::new_return_place(ty, span); + Place::Base( + PlaceBase::Static(Box::new(Static { def_id, ty, promoted: Some(promoted_id) }))) }; let (blocks, local_decls) = self.source.basic_blocks_and_local_decls_mut(); match candidate { @@ -421,7 +418,7 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, source: mir, temps: &mut temps, keep_original: false, - def_id + def_id, }; promoter.promote_candidate(candidate); } From 752544b28411540d7beb6fe4d1e2f1d8c775e05b Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Sat, 23 Mar 2019 18:29:02 +0530 Subject: [PATCH 7/8] adding mir::StaticKind enum for static and promoted --- src/librustc/mir/mod.rs | 47 ++++---- src/librustc/mir/visit.rs | 13 ++- src/librustc_codegen_ssa/mir/block.rs | 18 +-- src/librustc_codegen_ssa/mir/place.rs | 8 +- .../borrow_check/error_reporting.rs | 19 ++-- src/librustc_mir/borrow_check/mod.rs | 30 +++-- .../borrow_check/mutability_errors.rs | 10 +- .../borrow_check/nll/type_check/mod.rs | 62 ++++++----- src/librustc_mir/borrow_check/place_ext.rs | 9 +- .../borrow_check/places_conflict.rs | 104 +++++++++++------- src/librustc_mir/build/expr/as_place.rs | 3 +- src/librustc_mir/interpret/place.rs | 6 +- src/librustc_mir/monomorphize/collector.rs | 6 +- src/librustc_mir/transform/check_unsafety.rs | 9 +- src/librustc_mir/transform/const_prop.rs | 7 +- src/librustc_mir/transform/inline.rs | 4 +- src/librustc_mir/transform/promote_consts.rs | 18 ++- src/librustc_mir/transform/qualify_consts.rs | 31 ++++-- .../transform/qualify_min_const_fn.rs | 4 +- 19 files changed, 240 insertions(+), 168 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 47b9535abc5..12f14cfbb9a 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1915,19 +1915,22 @@ pub enum PlaceBase<'tcx> { Static(Box>), } -/// The `DefId` of a static, along with its normalized type (which is -/// stored to avoid requiring normalization when reading MIR). +/// We store the normalized type to avoid requiring normalization when reading MIR #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)] pub struct Static<'tcx> { - pub def_id: DefId, pub ty: Ty<'tcx>, - pub promoted: Option, + pub kind: StaticKind, +} + +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, HashStable, RustcEncodable, RustcDecodable)] +pub enum StaticKind { + Promoted(Promoted), + Static(DefId), } impl_stable_hash_for!(struct Static<'tcx> { - def_id, ty, - promoted + kind }); /// The `Projection` data structure defines things of the form `B.x` @@ -2058,21 +2061,23 @@ impl<'tcx> Debug for Place<'tcx> { match *self { Base(PlaceBase::Local(id)) => write!(fmt, "{:?}", id), - Base(PlaceBase::Static(box self::Static { def_id, ty, promoted })) => { - match promoted { - None => write!( - fmt, - "({}: {:?})", - ty::tls::with(|tcx| tcx.def_path_str(def_id)), - ty - ), - Some(pr) => write!( - fmt, - "({:?}: {:?})", - pr, - ty - ), - } + Base(PlaceBase::Static(box self::Static { ty, kind: StaticKind::Static(def_id) })) => { + write!( + fmt, + "({}: {:?})", + ty::tls::with(|tcx| tcx.def_path_str(def_id)), + ty + ) + }, + Base(PlaceBase::Static( + box self::Static { ty, kind: StaticKind::Promoted(promoted) }) + ) => { + write!( + fmt, + "({:?}: {:?})", + promoted, + ty + ) }, Projection(ref data) => match data.elem { ProjectionElem::Downcast(ref adt_def, index) => { diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 3f6133a7331..b1e2df0ae98 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -725,15 +725,18 @@ macro_rules! make_mir_visitor { place: & $($mutability)? Place<'tcx>, context: PlaceContext<'tcx>, location: Location) { + use crate::mir::{Static, StaticKind}; match place { Place::Base(PlaceBase::Local(local)) => { self.visit_local(local, context, location); } - Place::Base(PlaceBase::Static(static_)) => { - if static_.promoted.is_none() { - self.visit_def_id(& $($mutability)? static_.def_id, location); - } - self.visit_ty(& $($mutability)? static_.ty, TyContext::Location(location)); + Place::Base( + PlaceBase::Static(box Static{kind: StaticKind::Static(def_id), ..}) + ) => { + self.visit_def_id(& $($mutability)? *def_id, location) + } + Place::Base(PlaceBase::Static(box Static{ty, ..})) => { + self.visit_ty(& $($mutability)? *ty, TyContext::Location(location)); } Place::Projection(proj) => { self.visit_projection(proj, context, location); diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 066b38be3db..4774f8fe5a3 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -1,7 +1,7 @@ use rustc::middle::lang_items; use rustc::ty::{self, Ty, TypeFoldable}; use rustc::ty::layout::{self, LayoutOf, HasTyCtxt}; -use rustc::mir::{self, Place, PlaceBase}; +use rustc::mir::{self, Place, PlaceBase, Static, StaticKind}; use rustc::mir::interpret::EvalErrorKind; use rustc_target::abi::call::{ArgType, FnType, PassMode, IgnoreMode}; use rustc_target::spec::abi::Abi; @@ -621,14 +621,18 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // but specified directly in the code. This means it gets promoted // and we can then extract the value by evaluating the promoted. mir::Operand::Copy( - Place::Base(PlaceBase::Static( - box mir::Static {promoted: Some(promoted), ty, ..} - )) + Place::Base( + PlaceBase::Static( + box Static { kind: StaticKind::Promoted(promoted), ty } + ) + ) ) | mir::Operand::Move( - Place::Base(PlaceBase::Static( - box mir::Static {promoted: Some(promoted), ty, ..} - )) + Place::Base( + PlaceBase::Static( + box Static { kind: StaticKind::Promoted(promoted), ty } + ) + ) ) => { let param_env = ty::ParamEnv::reveal_all(); let cid = mir::interpret::GlobalId { diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index 1608429b070..7cafa0088a0 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -409,7 +409,9 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let result = match *place { mir::Place::Base(mir::PlaceBase::Local(_)) => bug!(), // handled above mir::Place::Base( - mir::PlaceBase::Static(box mir::Static { def_id: _, ty, promoted: Some(promoted) }) + mir::PlaceBase::Static( + box mir::Static { ty, kind: mir::StaticKind::Promoted(promoted) } + ) ) => { let param_env = ty::ParamEnv::reveal_all(); let cid = mir::interpret::GlobalId { @@ -438,7 +440,9 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } } mir::Place::Base( - mir::PlaceBase::Static(box mir::Static { def_id, ty, promoted: None }) + mir::PlaceBase::Static( + box mir::Static { ty, kind: mir::StaticKind::Static(def_id) } + ) ) => { // NB: The layout of a static may be unsized as is the case when working // with a static that is an extern_type. diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index b9877945e7e..fac75989aa5 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -10,7 +10,7 @@ use rustc::mir::{ self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, Constant, ConstraintCategory, Field, Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, PlaceProjection, ProjectionElem, Rvalue, Statement, StatementKind, - TerminatorKind, VarBindingForm, + Static, StaticKind, TerminatorKind, VarBindingForm, }; use rustc::ty::{self, DefIdTree}; use rustc::ty::print::Print; @@ -1601,12 +1601,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Base(PlaceBase::Local(local)) => { self.append_local_to_string(local, buf)?; } - Place::Base(PlaceBase::Static(ref static_)) => { - if static_.promoted.is_some() { - buf.push_str("promoted"); - } else { - buf.push_str(&self.infcx.tcx.item_name(static_.def_id).to_string()); - } + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => { + buf.push_str("promoted"); + } + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Static(def_id), .. })) => { + buf.push_str(&self.infcx.tcx.item_name(def_id).to_string()); } Place::Projection(ref proj) => { match proj.elem { @@ -1808,8 +1807,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// Checks if a place is a thread-local static. pub fn is_place_thread_local(&self, place: &Place<'tcx>) -> bool { - if let Place::Base(PlaceBase::Static(statik)) = place { - let attrs = self.infcx.tcx.get_attrs(statik.def_id); + if let Place::Base( + PlaceBase::Static(box Static{ kind: StaticKind::Static(def_id), .. }) + ) = place { + let attrs = self.infcx.tcx.get_attrs(*def_id); let is_thread_local = attrs.iter().any(|attr| attr.check_name("thread_local")); debug!( diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 0358fffcde5..40b329c1084 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -8,7 +8,9 @@ use rustc::infer::InferCtxt; use rustc::lint::builtin::UNUSED_MUT; use rustc::middle::borrowck::SignalledError; use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind}; -use rustc::mir::{ClearCrossCrate, Local, Location, Mir, Mutability, Operand, Place, PlaceBase}; +use rustc::mir::{ + ClearCrossCrate, Local, Location, Mir, Mutability, Operand, Place, PlaceBase, Static, StaticKind +}; use rustc::mir::{Field, Projection, ProjectionElem, Rvalue, Statement, StatementKind}; use rustc::mir::{Terminator, TerminatorKind}; use rustc::ty::query::Providers; @@ -1308,8 +1310,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // // FIXME: allow thread-locals to borrow other thread locals? let (might_be_alive, will_be_dropped) = match root_place { - Place::Base(PlaceBase::Static(st)) => { - (true, st.promoted.is_none() && self.is_place_thread_local(&root_place)) + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => { + (true, false) + } + Place::Base(PlaceBase::Static(box Static{ kind: _, .. })) => { + // Thread-locals might be dropped after the function exits, but + // "true" statics will never be. + (true, self.is_place_thread_local(&root_place)) } Place::Base(PlaceBase::Local(_)) => { // Locals are always dropped at function exit, and if they @@ -1982,18 +1989,19 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } // The rules for promotion are made by `qualify_consts`, there wouldn't even be a // `Place::Promoted` if the promotion weren't 100% legal. So we just forward this - Place::Base(PlaceBase::Static(ref static_)) => { - if static_.promoted.is_some() || - (static_.promoted.is_none() && - self.infcx.tcx.is_static(static_.def_id) - == Some(hir::Mutability::MutMutable) - ){ + Place::Base(PlaceBase::Static(box Static{kind: StaticKind::Promoted(_), ..})) => + Ok(RootPlace { + place, + is_local_mutation_allowed, + }), + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Static(def_id), .. })) => { + if self.infcx.tcx.is_static(def_id) != Some(hir::Mutability::MutMutable) { + Err(place) + } else { Ok(RootPlace { place, is_local_mutation_allowed, }) - } else { - Err(place) } } Place::Projection(ref proj) => { diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 2661d765718..f351212e9d5 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -1,7 +1,9 @@ use rustc::hir; use rustc::hir::Node; use rustc::mir::{self, BindingForm, Constant, ClearCrossCrate, Local, Location, Mir}; -use rustc::mir::{Mutability, Operand, Place, PlaceBase, Projection, ProjectionElem, Static}; +use rustc::mir::{ + Mutability, Operand, Place, PlaceBase, Projection, ProjectionElem, Static, StaticKind, +}; use rustc::mir::{Terminator, TerminatorKind}; use rustc::ty::{self, Const, DefIdTree, TyS, TyKind, TyCtxt}; use rustc_data_structures::indexed_vec::Idx; @@ -129,8 +131,10 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { } } - Place::Base(PlaceBase::Static(box Static { def_id, ty: _, promoted })) => { - assert!(promoted.is_none()); + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) => + unreachable!(), + + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), .. })) => { if let Place::Base(PlaceBase::Static(_)) = access_place { item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); reason = String::new(); diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 2a32b475c79..e12077fd5f7 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -453,45 +453,53 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { Place::Base(PlaceBase::Local(index)) => PlaceTy::Ty { ty: self.mir.local_decls[index].ty, }, - Place::Base(PlaceBase::Static(box Static { def_id, ty: sty, promoted })) => { + Place::Base( + PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted), ty: sty }) + ) => { let sty = self.sanitize_type(place, sty); - let check_err = - |verifier: &mut TypeVerifier<'a, 'b, 'gcx, 'tcx> , - place: &Place<'tcx>, - ty, - sty| { - if let Err(terr) = verifier.cx.eq_types( + + if !self.errors_reported { + let promoted_mir = &self.mir.promoted[promoted]; + self.sanitize_promoted(promoted_mir, location); + + let promoted_ty = promoted_mir.return_ty(); + + if let Err(terr) = self.cx.eq_types( sty, - ty, + promoted_ty, location.to_locations(), ConstraintCategory::Boring, ) { span_mirbug!( - verifier, + self, place, "bad promoted type ({:?}: {:?}): {:?}", - ty, + promoted_ty, sty, terr ); }; - }; - match promoted { - Some(pr) => { - if !self.errors_reported { - let promoted_mir = &self.mir.promoted[pr]; - self.sanitize_promoted(promoted_mir, location); - - let promoted_ty = promoted_mir.return_ty(); - check_err(self, place, promoted_ty, sty); - } - } - None => { - let ty = self.tcx().type_of(def_id); - let ty = self.cx.normalize(ty, location); - - check_err(self, place, ty, sty); - } + } + PlaceTy::Ty { ty: sty } + } + Place::Base( + PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), ty: sty }) + ) => { + let sty = self.sanitize_type(place, sty); + let ty = self.tcx().type_of(def_id); + let ty = self.cx.normalize(ty, location); + if let Err(terr) = + self.cx + .eq_types(ty, sty, location.to_locations(), ConstraintCategory::Boring) + { + span_mirbug!( + self, + place, + "bad static type ({:?}: {:?}): {:?}", + ty, + sty, + terr + ); } PlaceTy::Ty { ty: sty } } diff --git a/src/librustc_mir/borrow_check/place_ext.rs b/src/librustc_mir/borrow_check/place_ext.rs index 0452465f0b9..6bc56ab721f 100644 --- a/src/librustc_mir/borrow_check/place_ext.rs +++ b/src/librustc_mir/borrow_check/place_ext.rs @@ -1,6 +1,6 @@ use rustc::hir; use rustc::mir::ProjectionElem; -use rustc::mir::{Local, Mir, Place, PlaceBase, Mutability}; +use rustc::mir::{Local, Mir, Place, PlaceBase, Mutability, Static, StaticKind}; use rustc::ty::{self, TyCtxt}; use crate::borrow_check::borrow_set::LocalsStateAtExit; @@ -49,9 +49,10 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { } } } - Place::Base(PlaceBase::Static(static_)) => { - static_.promoted.is_none() && - (tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable)) + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => + false, + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Static(def_id), .. })) => { + tcx.is_static(*def_id) == Some(hir::Mutability::MutMutable) } Place::Projection(proj) => match proj.elem { ProjectionElem::Field(..) diff --git a/src/librustc_mir/borrow_check/places_conflict.rs b/src/librustc_mir/borrow_check/places_conflict.rs index 783751056d7..dc9b059de46 100644 --- a/src/librustc_mir/borrow_check/places_conflict.rs +++ b/src/librustc_mir/borrow_check/places_conflict.rs @@ -2,7 +2,7 @@ use crate::borrow_check::ArtificialField; use crate::borrow_check::Overlap; use crate::borrow_check::{Deep, Shallow, AccessDepth}; use rustc::hir; -use rustc::mir::{BorrowKind, Mir, Place, PlaceBase, Projection, ProjectionElem}; +use rustc::mir::{BorrowKind, Mir, Place, PlaceBase, Projection, ProjectionElem, Static, StaticKind}; use rustc::ty::{self, TyCtxt}; use std::cmp::max; @@ -370,47 +370,71 @@ fn place_element_conflict<'a, 'gcx: 'tcx, 'tcx>( Overlap::Disjoint } } - (Place::Base(PlaceBase::Static(s1)), Place::Base(PlaceBase::Static(s2))) => { - match (s1.promoted, s2.promoted) { - (None, None) => { - if s1.def_id != s2.def_id { - debug!("place_element_conflict: DISJOINT-STATIC"); - Overlap::Disjoint - } else if tcx.is_static(s1.def_id) == Some(hir::Mutability::MutMutable) { - // We ignore mutable statics - they can only be unsafe code. - debug!("place_element_conflict: IGNORE-STATIC-MUT"); - Overlap::Disjoint - } else { - debug!("place_element_conflict: DISJOINT-OR-EQ-STATIC"); - Overlap::EqualOrDisjoint - } - }, - (Some(p1), Some(p2)) => { - if p1 == p2 { - if let ty::Array(_, size) = s1.ty.sty { - if size.unwrap_usize(tcx) == 0 { - // Ignore conflicts with promoted [T; 0]. - debug!("place_element_conflict: IGNORE-LEN-0-PROMOTED"); - return Overlap::Disjoint; - } - } - // the same promoted - base case, equal - debug!("place_element_conflict: DISJOINT-OR-EQ-PROMOTED"); - Overlap::EqualOrDisjoint - } else { - // different promoteds - base case, disjoint - debug!("place_element_conflict: DISJOINT-PROMOTED"); - Overlap::Disjoint - } - }, - (_, _) => { - debug!("place_element_conflict: DISJOINT-STATIC-PROMOTED"); - Overlap::Disjoint - } + ( + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id_1), .. })), + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id_2), .. })), + ) => { + if *def_id_1 != *def_id_2 { + debug!("place_element_conflict: DISJOINT-STATIC"); + Overlap::Disjoint + } else if tcx.is_static(*def_id_1) == Some(hir::Mutability::MutMutable) { + // We ignore mutable statics - they can only be unsafe code. + debug!("place_element_conflict: IGNORE-STATIC-MUT"); + Overlap::Disjoint + } else { + debug!("place_element_conflict: DISJOINT-OR-EQ-STATIC"); + Overlap::EqualOrDisjoint } } - (Place::Base(PlaceBase::Local(_)), Place::Base(PlaceBase::Static(_))) | - (Place::Base(PlaceBase::Static(_)), Place::Base(PlaceBase::Local(_))) => { + ( + Place::Base( + PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted_1), ty }) + ), + Place::Base( + PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted_2), .. }) + ), + ) => { + if *promoted_1 == *promoted_2 { + if let ty::Array(_, size) = ty.sty { + if size.unwrap_usize(tcx) == 0 { + // Ignore conflicts with promoted [T; 0]. + debug!("place_element_conflict: IGNORE-LEN-0-PROMOTED"); + return Overlap::Disjoint; + } + } + // the same promoted - base case, equal + debug!("place_element_conflict: DISJOINT-OR-EQ-PROMOTED"); + Overlap::EqualOrDisjoint + } else { + // different promoteds - base case, disjoint + debug!("place_element_conflict: DISJOINT-PROMOTED"); + Overlap::Disjoint + } + } + ( + Place::Base(PlaceBase::Local(_)), + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) + ) | + ( + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })), + Place::Base(PlaceBase::Local(_)) + ) | + ( + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })), + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })) + ) | + ( + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })), + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) + ) | + ( + Place::Base(PlaceBase::Local(_)), + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })) + ) | + ( + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })), + Place::Base(PlaceBase::Local(_)) + ) => { debug!("place_element_conflict: DISJOINT-STATIC-LOCAL-PROMOTED"); Overlap::Disjoint } diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index 83e1d3ebb3e..199d03ac445 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -126,9 +126,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { block.and(place) } ExprKind::StaticRef { id } => block.and(Place::Base(PlaceBase::Static(Box::new(Static { - def_id: id, ty: expr.ty, - promoted: None, + kind: StaticKind::Static(id), })))), ExprKind::PlaceTypeAscription { source, user_ty } => { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 6089e491684..d2c279e48fe 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -582,9 +582,9 @@ where ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { use rustc::mir::Place::*; use rustc::mir::PlaceBase; - use rustc::mir::Static; + use rustc::mir::{Static, StaticKind}; Ok(match *mir_place { - Base(PlaceBase::Static(box Static {promoted: Some(promoted), ty: _, ..})) => { + Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted), .. })) => { let instance = self.frame().instance; self.const_eval_raw(GlobalId { instance, @@ -592,7 +592,7 @@ where })? } - Base(PlaceBase::Static(box Static {promoted: None, ty, def_id})) => { + Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), ty })) => { assert!(!ty.needs_subst()); let layout = self.layout_of(ty)?; let instance = ty::Instance::mono(*self.tcx, def_id); diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 075b81e9fd9..0ad6962cc4a 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -184,7 +184,7 @@ use rustc::ty::subst::{InternalSubsts, SubstsRef}; use rustc::ty::{self, TypeFoldable, Ty, TyCtxt, GenericParamDefKind}; use rustc::ty::adjustment::CustomCoerceUnsized; use rustc::session::config::EntryFnType; -use rustc::mir::{self, Location, Promoted}; +use rustc::mir::{self, Location, Place, PlaceBase, Promoted, Static, StaticKind}; use rustc::mir::visit::Visitor as MirVisitor; use rustc::mir::mono::MonoItem; use rustc::mir::interpret::{Scalar, GlobalId, AllocKind, ErrorHandled}; @@ -655,8 +655,8 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { context: mir::visit::PlaceContext<'tcx>, location: Location) { match place { - mir::Place::Base( - mir::PlaceBase::Static(box mir::Static{def_id, promoted:None, ..}) + Place::Base( + PlaceBase::Static(box Static{ kind:StaticKind::Static(def_id), .. }) ) => { debug!("visiting static {:?} @ {:?}", def_id, location); diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 2ebe28e6ac0..0e31515e4af 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -300,9 +300,12 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { &Place::Base(PlaceBase::Local(..)) => { // locals are safe } - &Place::Base(PlaceBase::Static(box Static { def_id, ty: _, promoted })) => { - assert!(promoted.is_none(), "unsafety checking should happen before promotion"); - + &Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) => { + bug!("unsafety checking should happen before promotion") + } + &Place::Base( + PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), .. }) + ) => { if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) { self.require_unsafe("use of mutable static", "mutable statics can be mutated by multiple threads: aliasing violations \ diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 81cdb001005..c2d594ba7b7 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -4,7 +4,7 @@ use rustc::hir::def::Def; use rustc::mir::{Constant, Location, Place, PlaceBase, Mir, Operand, Rvalue, Local}; -use rustc::mir::{NullOp, UnOp, StatementKind, Statement, BasicBlock, LocalKind}; +use rustc::mir::{NullOp, UnOp, StatementKind, Statement, BasicBlock, LocalKind, Static, StaticKind}; use rustc::mir::{TerminatorKind, ClearCrossCrate, SourceInfo, BinOp, ProjectionElem}; use rustc::mir::visit::{Visitor, PlaceContext, MutatingUseContext, NonMutatingUseContext}; use rustc::mir::interpret::{EvalErrorKind, Scalar, GlobalId, EvalResult}; @@ -266,7 +266,6 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { } fn eval_place(&mut self, place: &Place<'tcx>, source_info: SourceInfo) -> Option> { - use rustc::mir::Static; match *place { Place::Base(PlaceBase::Local(loc)) => self.places[loc].clone(), Place::Projection(ref proj) => match proj.elem { @@ -283,7 +282,9 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { // an `Index` projection would throw us off-track. _ => None, }, - Place::Base(PlaceBase::Static(box Static {promoted: Some(promoted), ..})) => { + Place::Base( + PlaceBase::Static(box Static {kind: StaticKind::Promoted(promoted), ..}) + ) => { let generics = self.tcx.generics_of(self.source.def_id()); if generics.requires_monomorphization(self.tcx) { // FIXME: can't handle code with generics diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index ec2cf8a4c03..1063381d6aa 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -692,7 +692,9 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { // Return pointer; update the place itself *place = self.destination.clone(); }, - Place::Base(PlaceBase::Static(box Static { promoted: Some(promoted), .. })) => { + Place::Base( + PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted), .. }) + ) => { if let Some(p) = self.promoted_map.get(*promoted).cloned() { *promoted = p; } diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 2344f070ea6..92039618d85 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -12,7 +12,6 @@ //! initialization and can otherwise silence errors, if //! move analysis runs after promotion on broken MIR. -use rustc::hir::def_id::DefId; use rustc::mir::*; use rustc::mir::visit::{PlaceContext, MutatingUseContext, MutVisitor, Visitor}; use rustc::mir::traversal::ReversePostorder; @@ -152,8 +151,7 @@ struct Promoter<'a, 'tcx: 'a> { /// If true, all nested temps are also kept in the /// source MIR, not moved to the promoted MIR. - keep_original: bool, - def_id: DefId, + keep_original: bool } impl<'a, 'tcx> Promoter<'a, 'tcx> { @@ -289,16 +287,16 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { } fn promote_candidate(mut self, candidate: Candidate) { - use rustc::mir::Static; let mut operand = { - let def_id = self.def_id; let promoted = &mut self.promoted; let promoted_id = Promoted::new(self.source.promoted.len()); let mut promoted_place = |ty, span| { promoted.span = span; - promoted.local_decls[RETURN_PLACE] = LocalDecl::new_return_place(ty, span); + promoted.local_decls[RETURN_PLACE] = + LocalDecl::new_return_place(ty, span); Place::Base( - PlaceBase::Static(Box::new(Static { def_id, ty, promoted: Some(promoted_id) }))) + PlaceBase::Static(box Static{ kind: StaticKind::Promoted(promoted_id), ty }) + ) }; let (blocks, local_decls) = self.source.basic_blocks_and_local_decls_mut(); match candidate { @@ -371,8 +369,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Promoter<'a, 'tcx> { pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>, mut temps: IndexVec, - candidates: Vec, - def_id: DefId) { + candidates: Vec) { // Visit candidates in reverse, in case they're nested. debug!("promote_candidates({:?})", candidates); @@ -417,8 +414,7 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, tcx, source: mir, temps: &mut temps, - keep_original: false, - def_id, + keep_original: false }; promoter.promote_candidate(candidate); } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index c01ed4b1c59..c35bf80bb2e 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -188,8 +188,9 @@ trait Qualif { fn in_place(cx: &ConstCx<'_, 'tcx>, place: &Place<'tcx>) -> bool { match *place { Place::Base(PlaceBase::Local(local)) => Self::in_local(cx, local), + Place::Base(PlaceBase::Static(box Static {kind: StaticKind::Promoted(_), .. })) => + bug!("qualifying already promoted MIR"), Place::Base(PlaceBase::Static(ref static_)) => { - assert!(static_.promoted.is_none(), "qualifying already promoted MIR"); Self::in_static(cx, static_) }, Place::Projection(ref proj) => Self::in_projection(cx, proj), @@ -372,11 +373,18 @@ impl Qualif for IsNotConst { const IDX: usize = 2; fn in_static(cx: &ConstCx<'_, 'tcx>, static_: &Static<'tcx>) -> bool { - // Only allow statics (not consts) to refer to other statics. - let allowed = cx.mode == Mode::Static || cx.mode == Mode::StaticMut; + match static_.kind { + StaticKind::Promoted(_) => unreachable!(), + StaticKind::Static(def_id) => { + // Only allow statics (not consts) to refer to other statics. + let allowed = cx.mode == Mode::Static || cx.mode == Mode::StaticMut; - !allowed || - cx.tcx.get_attrs(static_.def_id).iter().any(|attr| attr.check_name("thread_local")) + !allowed || + cx.tcx.get_attrs(def_id).iter().any( + |attr| attr.check_name("thread_local" + )) + } + } } fn in_projection(cx: &ConstCx<'_, 'tcx>, proj: &PlaceProjection<'tcx>) -> bool { @@ -770,8 +778,9 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { ); dest = &proj.base; }, - Place::Base(PlaceBase::Static(st)) => { - assert!(st.promoted.is_none(), "promoteds don't exist yet during promotion"); + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => + bug!("promoteds don't exist yet during promotion"), + Place::Base(PlaceBase::Static(box Static{ kind: _, .. })) => { // Catch more errors in the destination. `visit_place` also checks that we // do not try to access statics from constants or try to mutate statics self.visit_place( @@ -921,10 +930,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { self.super_place(place, context, location); match *place { Place::Base(PlaceBase::Local(_)) => {} - Place::Base(PlaceBase::Static(ref global)) => { - assert!(global.promoted.is_none()); + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => {} + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Static(def_id), .. })) => { if self.tcx - .get_attrs(global.def_id) + .get_attrs(def_id) .iter() .any(|attr| attr.check_name("thread_local")) { if self.mode != Mode::Fn { @@ -1516,7 +1525,7 @@ impl MirPass for QualifyAndPromoteConstants { }; // Do the actual promotion, now that we know what's viable. - promote_consts::promote_candidates(mir, tcx, temps, candidates, def_id); + promote_consts::promote_candidates(mir, tcx, temps, candidates); } else { if !mir.control_flow_destroyed.is_empty() { let mut locals = mir.vars_iter(); diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index da849faf40a..8742c5d759c 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -257,8 +257,8 @@ fn check_place( match place { Place::Base(PlaceBase::Local(_)) => Ok(()), // promoteds are always fine, they are essentially constants - Place::Base(PlaceBase::Static(box Static {def_id: _, ty: _, promoted: Some(_)})) => Ok(()), - Place::Base(PlaceBase::Static(box Static {def_id: _, ty: _, promoted: None})) => + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) => Ok(()), + Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })) => Err((span, "cannot access `static` items in const fn".into())), Place::Projection(proj) => { match proj.elem { From fb93f10dac46376b706643a74f3ebbabaf46c48f Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Sun, 24 Mar 2019 08:59:11 +0530 Subject: [PATCH 8/8] code review fixes --- src/librustc/mir/visit.rs | 11 +- src/librustc_mir/borrow_check/mod.rs | 2 +- .../borrow_check/nll/type_check/mod.rs | 76 ++++++------- .../borrow_check/places_conflict.rs | 100 +++++++----------- src/librustc_mir/transform/promote_consts.rs | 3 +- src/librustc_mir/transform/qualify_consts.rs | 4 +- 6 files changed, 81 insertions(+), 115 deletions(-) diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index b1e2df0ae98..54e5bfc4397 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -725,17 +725,14 @@ macro_rules! make_mir_visitor { place: & $($mutability)? Place<'tcx>, context: PlaceContext<'tcx>, location: Location) { - use crate::mir::{Static, StaticKind}; match place { Place::Base(PlaceBase::Local(local)) => { self.visit_local(local, context, location); } - Place::Base( - PlaceBase::Static(box Static{kind: StaticKind::Static(def_id), ..}) - ) => { - self.visit_def_id(& $($mutability)? *def_id, location) - } - Place::Base(PlaceBase::Static(box Static{ty, ..})) => { + Place::Base(PlaceBase::Static(box Static { kind, ty })) => { + if let StaticKind::Static(def_id) = kind { + self.visit_def_id(& $($mutability)? *def_id, location) + } self.visit_ty(& $($mutability)? *ty, TyContext::Location(location)); } Place::Projection(proj) => { diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 40b329c1084..a3909486610 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1313,7 +1313,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => { (true, false) } - Place::Base(PlaceBase::Static(box Static{ kind: _, .. })) => { + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Static(_), .. })) => { // Thread-locals might be dropped after the function exits, but // "true" statics will never be. (true, self.is_place_thread_local(&root_place)) diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index e12077fd5f7..aab650383cf 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -449,57 +449,49 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { context: PlaceContext<'_>, ) -> PlaceTy<'tcx> { debug!("sanitize_place: {:?}", place); - let place_ty = match *place { + let place_ty = match place { Place::Base(PlaceBase::Local(index)) => PlaceTy::Ty { - ty: self.mir.local_decls[index].ty, + ty: self.mir.local_decls[*index].ty, }, - Place::Base( - PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted), ty: sty }) - ) => { + Place::Base(PlaceBase::Static(box Static { kind, ty: sty })) => { let sty = self.sanitize_type(place, sty); - - if !self.errors_reported { - let promoted_mir = &self.mir.promoted[promoted]; - self.sanitize_promoted(promoted_mir, location); - - let promoted_ty = promoted_mir.return_ty(); - - if let Err(terr) = self.cx.eq_types( - sty, - promoted_ty, - location.to_locations(), - ConstraintCategory::Boring, - ) { - span_mirbug!( - self, + let check_err = + |verifier: &mut TypeVerifier<'a, 'b, 'gcx, 'tcx>, + place: &Place<'tcx>, + ty, + sty| { + if let Err(terr) = verifier.cx.eq_types( + sty, + ty, + location.to_locations(), + ConstraintCategory::Boring, + ) { + span_mirbug!( + verifier, place, "bad promoted type ({:?}: {:?}): {:?}", - promoted_ty, + ty, sty, terr ); + }; }; - } - PlaceTy::Ty { ty: sty } - } - Place::Base( - PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), ty: sty }) - ) => { - let sty = self.sanitize_type(place, sty); - let ty = self.tcx().type_of(def_id); - let ty = self.cx.normalize(ty, location); - if let Err(terr) = - self.cx - .eq_types(ty, sty, location.to_locations(), ConstraintCategory::Boring) - { - span_mirbug!( - self, - place, - "bad static type ({:?}: {:?}): {:?}", - ty, - sty, - terr - ); + match kind { + StaticKind::Promoted(promoted) => { + if !self.errors_reported { + let promoted_mir = &self.mir.promoted[*promoted]; + self.sanitize_promoted(promoted_mir, location); + + let promoted_ty = promoted_mir.return_ty(); + check_err(self, place, promoted_ty, sty); + } + } + StaticKind::Static(def_id) => { + let ty = self.tcx().type_of(*def_id); + let ty = self.cx.normalize(ty, location); + + check_err(self, place, ty, sty); + } } PlaceTy::Ty { ty: sty } } diff --git a/src/librustc_mir/borrow_check/places_conflict.rs b/src/librustc_mir/borrow_check/places_conflict.rs index dc9b059de46..52119d6b19b 100644 --- a/src/librustc_mir/borrow_check/places_conflict.rs +++ b/src/librustc_mir/borrow_check/places_conflict.rs @@ -2,7 +2,7 @@ use crate::borrow_check::ArtificialField; use crate::borrow_check::Overlap; use crate::borrow_check::{Deep, Shallow, AccessDepth}; use rustc::hir; -use rustc::mir::{BorrowKind, Mir, Place, PlaceBase, Projection, ProjectionElem, Static, StaticKind}; +use rustc::mir::{BorrowKind, Mir, Place, PlaceBase, Projection, ProjectionElem, StaticKind}; use rustc::ty::{self, TyCtxt}; use std::cmp::max; @@ -370,71 +370,47 @@ fn place_element_conflict<'a, 'gcx: 'tcx, 'tcx>( Overlap::Disjoint } } - ( - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id_1), .. })), - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id_2), .. })), - ) => { - if *def_id_1 != *def_id_2 { - debug!("place_element_conflict: DISJOINT-STATIC"); - Overlap::Disjoint - } else if tcx.is_static(*def_id_1) == Some(hir::Mutability::MutMutable) { - // We ignore mutable statics - they can only be unsafe code. - debug!("place_element_conflict: IGNORE-STATIC-MUT"); - Overlap::Disjoint - } else { - debug!("place_element_conflict: DISJOINT-OR-EQ-STATIC"); - Overlap::EqualOrDisjoint - } - } - ( - Place::Base( - PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted_1), ty }) - ), - Place::Base( - PlaceBase::Static(box Static { kind: StaticKind::Promoted(promoted_2), .. }) - ), - ) => { - if *promoted_1 == *promoted_2 { - if let ty::Array(_, size) = ty.sty { - if size.unwrap_usize(tcx) == 0 { - // Ignore conflicts with promoted [T; 0]. - debug!("place_element_conflict: IGNORE-LEN-0-PROMOTED"); - return Overlap::Disjoint; + (Place::Base(PlaceBase::Static(s1)), Place::Base(PlaceBase::Static(s2))) => { + match (&s1.kind, &s2.kind) { + (StaticKind::Static(def_id_1), StaticKind::Static(def_id_2)) => { + if def_id_1 != def_id_2 { + debug!("place_element_conflict: DISJOINT-STATIC"); + Overlap::Disjoint + } else if tcx.is_static(*def_id_1) == Some(hir::Mutability::MutMutable) { + // We ignore mutable statics - they can only be unsafe code. + debug!("place_element_conflict: IGNORE-STATIC-MUT"); + Overlap::Disjoint + } else { + debug!("place_element_conflict: DISJOINT-OR-EQ-STATIC"); + Overlap::EqualOrDisjoint } + }, + (StaticKind::Promoted(promoted_1), StaticKind::Promoted(promoted_2)) => { + if promoted_1 == promoted_2 { + if let ty::Array(_, size) = s1.ty.sty { + if size.unwrap_usize(tcx) == 0 { + // Ignore conflicts with promoted [T; 0]. + debug!("place_element_conflict: IGNORE-LEN-0-PROMOTED"); + return Overlap::Disjoint; + } + } + // the same promoted - base case, equal + debug!("place_element_conflict: DISJOINT-OR-EQ-PROMOTED"); + Overlap::EqualOrDisjoint + } else { + // different promoteds - base case, disjoint + debug!("place_element_conflict: DISJOINT-PROMOTED"); + Overlap::Disjoint + } + }, + (_, _) => { + debug!("place_element_conflict: DISJOINT-STATIC-PROMOTED"); + Overlap::Disjoint } - // the same promoted - base case, equal - debug!("place_element_conflict: DISJOINT-OR-EQ-PROMOTED"); - Overlap::EqualOrDisjoint - } else { - // different promoteds - base case, disjoint - debug!("place_element_conflict: DISJOINT-PROMOTED"); - Overlap::Disjoint } } - ( - Place::Base(PlaceBase::Local(_)), - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) - ) | - ( - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })), - Place::Base(PlaceBase::Local(_)) - ) | - ( - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })), - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })) - ) | - ( - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })), - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) - ) | - ( - Place::Base(PlaceBase::Local(_)), - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })) - ) | - ( - Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(_), .. })), - Place::Base(PlaceBase::Local(_)) - ) => { + (Place::Base(PlaceBase::Local(_)), Place::Base(PlaceBase::Static(_))) | + (Place::Base(PlaceBase::Static(_)), Place::Base(PlaceBase::Local(_))) => { debug!("place_element_conflict: DISJOINT-STATIC-LOCAL-PROMOTED"); Overlap::Disjoint } diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 92039618d85..73b88e9904b 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -292,8 +292,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { let promoted_id = Promoted::new(self.source.promoted.len()); let mut promoted_place = |ty, span| { promoted.span = span; - promoted.local_decls[RETURN_PLACE] = - LocalDecl::new_return_place(ty, span); + promoted.local_decls[RETURN_PLACE] = LocalDecl::new_return_place(ty, span); Place::Base( PlaceBase::Static(box Static{ kind: StaticKind::Promoted(promoted_id), ty }) ) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index c35bf80bb2e..0b9ad85e6b1 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -930,7 +930,9 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { self.super_place(place, context, location); match *place { Place::Base(PlaceBase::Local(_)) => {} - Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => {} + Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_), .. })) => { + unreachable!() + } Place::Base(PlaceBase::Static(box Static{ kind: StaticKind::Static(def_id), .. })) => { if self.tcx .get_attrs(def_id)