From b8e9da7c0885f624d186499f7521bae71381a97c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 8 Apr 2019 10:32:35 +0200 Subject: [PATCH 1/7] `asm!` output can neither be promotable nor const --- src/librustc_mir/transform/promote_consts.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 43723aaf568da..9c792a35bb429 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -101,7 +101,6 @@ impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> { if *temp == TempState::Undefined { match context { PlaceContext::MutatingUse(MutatingUseContext::Store) | - PlaceContext::MutatingUse(MutatingUseContext::AsmOutput) | PlaceContext::MutatingUse(MutatingUseContext::Call) => { *temp = TempState::Defined { location, From 6b018444081a6a2940889bec495acea923e365cf Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 8 Apr 2019 15:28:00 +0200 Subject: [PATCH 2/7] Generalize initial "not const" assignments --- src/librustc_mir/transform/promote_consts.rs | 15 ++++++---- src/librustc_mir/transform/qualify_consts.rs | 31 ++++++++------------ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 9c792a35bb429..ddf963c7fa9b5 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -44,8 +44,8 @@ pub enum TempState { impl TempState { pub fn is_promotable(&self) -> bool { debug!("is_promotable: self={:?}", self); - if let TempState::Defined { uses, .. } = *self { - uses > 0 + if let TempState::Defined { .. } = *self { + true } else { false } @@ -80,9 +80,14 @@ impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> { context: PlaceContext<'tcx>, location: Location) { debug!("visit_local: index={:?} context={:?} location={:?}", index, context, location); - // We're only interested in temporaries - if self.mir.local_kind(index) != LocalKind::Temp { - return; + // We're only interested in temporaries and the return place + match self.mir.local_kind(index) { + | LocalKind::Temp + | LocalKind::ReturnPointer + => {}, + | LocalKind::Arg + | LocalKind::Var + => return, } // Ignore drops, if the temp gets promoted, diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 1faa9ad0d0da4..6272425012b98 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -631,26 +631,21 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { }; for (local, decl) in mir.local_decls.iter_enumerated() { - match mir.local_kind(local) { - LocalKind::Arg => { - let qualifs = cx.qualifs_in_any_value_of_ty(decl.ty); - for (per_local, qualif) in &mut cx.per_local.as_mut().zip(qualifs).0 { - if *qualif { - per_local.insert(local); - } + if let LocalKind::Arg = mir.local_kind(local) { + let qualifs = cx.qualifs_in_any_value_of_ty(decl.ty); + for (per_local, qualif) in &mut cx.per_local.as_mut().zip(qualifs).0 { + if *qualif { + per_local.insert(local); } - cx.per_local[IsNotConst].insert(local); - } - - LocalKind::Var if mode == Mode::Fn => { - cx.per_local[IsNotConst].insert(local); - } - - LocalKind::Temp if !temps[local].is_promotable() => { - cx.per_local[IsNotConst].insert(local); } - - _ => {} + } + if !temps[local].is_promotable() { + cx.per_local[IsNotConst].insert(local); + } + if let LocalKind::Var = mir.local_kind(local) { + // Sanity check to prevent implicit and explicit promotion of + // named locals + assert!(cx.per_local[IsNotConst].contains(local)); } } From ec5206588fe31e631d8f8b1d27400f18744b448e Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 8 Apr 2019 15:32:31 +0200 Subject: [PATCH 3/7] Explicit promotion is indistinguishable from explicit promotion Implicit promotion on the other hand has very strict rules on what may be done --- src/librustc_mir/transform/qualify_consts.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 6272425012b98..9095b78dd726a 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -508,13 +508,11 @@ impl Qualif for IsNotConst { } } -// Refers to temporaries which cannot be promoted as -// promote_consts decided they weren't simple enough. -// FIXME(oli-obk,eddyb): Remove this flag entirely and -// solely process this information via `IsNotConst`. -struct IsNotPromotable; +/// Refers to temporaries which cannot be promoted *implicitly*. +/// Explicit promotion e.g. for constant arguments declared via `rustc_args_required_const`. +struct IsNotImplicitlyPromotable; -impl Qualif for IsNotPromotable { +impl Qualif for IsNotImplicitlyPromotable { const IDX: usize = 3; fn in_call( @@ -550,7 +548,7 @@ macro_rules! static_assert_seq_qualifs { static_assert!(SEQ_QUALIFS: QUALIF_COUNT == $i); }; } -static_assert_seq_qualifs!(0 => HasMutInterior, NeedsDrop, IsNotConst, IsNotPromotable); +static_assert_seq_qualifs!(0 => HasMutInterior, NeedsDrop, IsNotConst, IsNotImplicitlyPromotable); impl ConstCx<'_, 'tcx> { fn qualifs_in_any_value_of_ty(&self, ty: Ty<'tcx>) -> PerQualif { @@ -558,7 +556,7 @@ impl ConstCx<'_, 'tcx> { qualifs[HasMutInterior] = HasMutInterior::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs[NeedsDrop] = NeedsDrop::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs[IsNotConst] = IsNotConst::in_any_value_of_ty(self, ty).unwrap_or(false); - qualifs[IsNotPromotable] = IsNotPromotable::in_any_value_of_ty(self, ty).unwrap_or(false); + qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs } @@ -567,7 +565,7 @@ impl ConstCx<'_, 'tcx> { qualifs[HasMutInterior] = HasMutInterior::in_local(self, local); qualifs[NeedsDrop] = NeedsDrop::in_local(self, local); qualifs[IsNotConst] = IsNotConst::in_local(self, local); - qualifs[IsNotPromotable] = IsNotPromotable::in_local(self, local); + qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_local(self, local); qualifs } @@ -576,7 +574,7 @@ impl ConstCx<'_, 'tcx> { qualifs[HasMutInterior] = HasMutInterior::in_value(self, source); qualifs[NeedsDrop] = NeedsDrop::in_value(self, source); qualifs[IsNotConst] = IsNotConst::in_value(self, source); - qualifs[IsNotPromotable] = IsNotPromotable::in_value(self, source); + qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_value(self, source); qualifs } } From d3d56732a7278881900ce6709892636bdc546862 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 8 Apr 2019 15:36:00 +0200 Subject: [PATCH 4/7] Get rid of "is not const" naming --- src/librustc_mir/transform/qualify_consts.rs | 32 ++++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 9095b78dd726a..8dd060d7a150d 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -365,11 +365,11 @@ impl Qualif for NeedsDrop { } } -// Not constant at all - non-`const fn` calls, asm!, +// Not promotable at all - non-`const fn` calls, asm!, // pointer comparisons, ptr-to-int casts, etc. -struct IsNotConst; +struct IsNotPromotable; -impl Qualif for IsNotConst { +impl Qualif for IsNotPromotable { const IDX: usize = 2; fn in_static(cx: &ConstCx<'_, 'tcx>, static_: &Static<'tcx>) -> bool { @@ -548,14 +548,14 @@ macro_rules! static_assert_seq_qualifs { static_assert!(SEQ_QUALIFS: QUALIF_COUNT == $i); }; } -static_assert_seq_qualifs!(0 => HasMutInterior, NeedsDrop, IsNotConst, IsNotImplicitlyPromotable); +static_assert_seq_qualifs!(0 => HasMutInterior, NeedsDrop, IsNotPromotable, IsNotImplicitlyPromotable); impl ConstCx<'_, 'tcx> { fn qualifs_in_any_value_of_ty(&self, ty: Ty<'tcx>) -> PerQualif { let mut qualifs = PerQualif::default(); qualifs[HasMutInterior] = HasMutInterior::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs[NeedsDrop] = NeedsDrop::in_any_value_of_ty(self, ty).unwrap_or(false); - qualifs[IsNotConst] = IsNotConst::in_any_value_of_ty(self, ty).unwrap_or(false); + qualifs[IsNotPromotable] = IsNotPromotable::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs } @@ -564,7 +564,7 @@ impl ConstCx<'_, 'tcx> { let mut qualifs = PerQualif::default(); qualifs[HasMutInterior] = HasMutInterior::in_local(self, local); qualifs[NeedsDrop] = NeedsDrop::in_local(self, local); - qualifs[IsNotConst] = IsNotConst::in_local(self, local); + qualifs[IsNotPromotable] = IsNotPromotable::in_local(self, local); qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_local(self, local); qualifs } @@ -573,7 +573,7 @@ impl ConstCx<'_, 'tcx> { let mut qualifs = PerQualif::default(); qualifs[HasMutInterior] = HasMutInterior::in_value(self, source); qualifs[NeedsDrop] = NeedsDrop::in_value(self, source); - qualifs[IsNotConst] = IsNotConst::in_value(self, source); + qualifs[IsNotPromotable] = IsNotPromotable::in_value(self, source); qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_value(self, source); qualifs } @@ -638,12 +638,12 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } } if !temps[local].is_promotable() { - cx.per_local[IsNotConst].insert(local); + cx.per_local[IsNotPromotable].insert(local); } if let LocalKind::Var = mir.local_kind(local) { // Sanity check to prevent implicit and explicit promotion of // named locals - assert!(cx.per_local[IsNotConst].contains(local)); + assert!(cx.per_local[IsNotPromotable].contains(local)); } } @@ -691,11 +691,11 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { // the borrowed place is disallowed from being borrowed, // due to either a mutable borrow (with some exceptions), // or an shared borrow of a value with interior mutability. - // Then `HasMutInterior` is replaced with `IsNotConst`, + // Then `HasMutInterior` is replaced with `IsNotPromotable`, // to avoid duplicate errors (e.g. from reborrowing). if qualifs[HasMutInterior] { qualifs[HasMutInterior] = false; - qualifs[IsNotConst] = true; + qualifs[IsNotPromotable] = true; if self.mode != Mode::Fn { if let BorrowKind::Mut { .. } = kind { @@ -810,7 +810,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } } - // Ensure the `IsNotConst` qualification is preserved. + // Ensure the `IsNotPromotable` qualification is preserved. // NOTE(eddyb) this is actually unnecessary right now, as // we never replace the local's qualif, but we might in // the future, and so it serves to catch changes that unset @@ -818,7 +818,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { // be replaced with calling `insert` to re-set the bit). if kind == LocalKind::Temp { if !self.temp_promotion_state[index].is_promotable() { - assert!(self.cx.per_local[IsNotConst].contains(index)); + assert!(self.cx.per_local[IsNotPromotable].contains(index)); } } } @@ -904,7 +904,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { // Account for errors in consts by using the // conservative type qualification instead. - if qualifs[IsNotConst] { + if qualifs[IsNotPromotable] { qualifs = self.qualifs_in_any_value_of_ty(mir.return_ty()); } @@ -1319,7 +1319,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { // which happens even without the user requesting it. // We can error out with a hard error if the argument is not // constant here. - if !IsNotConst::in_operand(self, arg) { + if !IsNotPromotable::in_operand(self, arg) { debug!("visit_terminator_kind: candidate={:?}", candidate); self.promotion_candidates.push(candidate); } else { @@ -1437,7 +1437,7 @@ fn mir_const_qualif<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, if mir.return_ty().references_error() { tcx.sess.delay_span_bug(mir.span, "mir_const_qualif: Mir had errors"); - return (1 << IsNotConst::IDX, Lrc::new(BitSet::new_empty(0))); + return (1 << IsNotPromotable::IDX, Lrc::new(BitSet::new_empty(0))); } Checker::new(tcx, def_id, mir, Mode::Const).check_const() From c63b9fff3f1e589d75db982e9839f9d52c5c57cc Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 8 Apr 2019 16:24:59 +0200 Subject: [PATCH 5/7] Pacify tidy --- src/librustc_mir/transform/qualify_consts.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 8dd060d7a150d..ae13e18ff03b0 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -548,7 +548,9 @@ macro_rules! static_assert_seq_qualifs { static_assert!(SEQ_QUALIFS: QUALIF_COUNT == $i); }; } -static_assert_seq_qualifs!(0 => HasMutInterior, NeedsDrop, IsNotPromotable, IsNotImplicitlyPromotable); +static_assert_seq_qualifs!( + 0 => HasMutInterior, NeedsDrop, IsNotPromotable, IsNotImplicitlyPromotable +); impl ConstCx<'_, 'tcx> { fn qualifs_in_any_value_of_ty(&self, ty: Ty<'tcx>) -> PerQualif { @@ -556,7 +558,8 @@ impl ConstCx<'_, 'tcx> { qualifs[HasMutInterior] = HasMutInterior::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs[NeedsDrop] = NeedsDrop::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs[IsNotPromotable] = IsNotPromotable::in_any_value_of_ty(self, ty).unwrap_or(false); - qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_any_value_of_ty(self, ty).unwrap_or(false); + qualifs[IsNotImplicitlyPromotable] = + IsNotImplicitlyPromotable::in_any_value_of_ty(self, ty).unwrap_or(false); qualifs } From ae4717d95010f2d75ba02c200ecd0ace7de0a93b Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 8 Apr 2019 18:34:45 +0200 Subject: [PATCH 6/7] Elaborate on implicit promotability --- src/librustc_mir/transform/qualify_consts.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index ae13e18ff03b0..5f8e7ad796ba3 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -509,7 +509,10 @@ impl Qualif for IsNotPromotable { } /// Refers to temporaries which cannot be promoted *implicitly*. -/// Explicit promotion e.g. for constant arguments declared via `rustc_args_required_const`. +/// Explicit promotion e.g. for constant arguments declared via `rustc_args_required_const` or by +/// happening inside a constant, static or const fn. Inside a const context all constness rules +/// apply, so implicit promotion simply has to follow the regular constant rules (modulo interior +/// mutability or `Drop` rules which are handled `HasMutInterior` and `NeedsDrop` respectively) struct IsNotImplicitlyPromotable; impl Qualif for IsNotImplicitlyPromotable { From f10394ae0c5f2f73d1eba688c2cf5832eb38bc96 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 10 Apr 2019 11:21:19 +0200 Subject: [PATCH 7/7] Documentation should have proper grammar --- src/librustc_mir/transform/qualify_consts.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 5f8e7ad796ba3..3154f8e89e394 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -509,10 +509,13 @@ impl Qualif for IsNotPromotable { } /// Refers to temporaries which cannot be promoted *implicitly*. -/// Explicit promotion e.g. for constant arguments declared via `rustc_args_required_const` or by -/// happening inside a constant, static or const fn. Inside a const context all constness rules +/// Explicit promotion happens e.g. for constant arguments declared via `rustc_args_required_const`. +/// Inside a const context all constness rules /// apply, so implicit promotion simply has to follow the regular constant rules (modulo interior -/// mutability or `Drop` rules which are handled `HasMutInterior` and `NeedsDrop` respectively) +/// mutability or `Drop` rules which are handled `HasMutInterior` and `NeedsDrop` respectively). +/// Implicit promotion inside regular functions does not happen if `const fn` calls are involved, +/// as the call may be perfectly alright at runtime, but fail at compile time e.g. due to addresses +/// being compared inside the function. struct IsNotImplicitlyPromotable; impl Qualif for IsNotImplicitlyPromotable {