From 3cc8087c4849fcbb8eeae208ace59bdf9b156ca8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 08:52:13 +0200 Subject: [PATCH 1/8] qualify_consts: extractt 'determine_mode'. --- src/librustc_mir/transform/qualify_consts.rs | 49 ++++++++++---------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index a77421ce15008..959d21ecfadeb 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -30,6 +30,7 @@ use std::fmt; use std::ops::{Deref, Index, IndexMut}; use std::usize; +use rustc::hir::HirId; use crate::transform::{MirPass, MirSource}; use super::promote_consts::{self, Candidate, TempState}; @@ -1596,27 +1597,12 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { } let def_id = src.def_id(); - let id = tcx.hir().as_local_hir_id(def_id).unwrap(); - let mut const_promoted_temps = None; - let mode = match tcx.hir().body_owner_kind(id) { - hir::BodyOwnerKind::Closure => Mode::NonConstFn, - hir::BodyOwnerKind::Fn => { - if tcx.is_const_fn(def_id) { - Mode::ConstFn - } else { - Mode::NonConstFn - } - } - hir::BodyOwnerKind::Const => { - const_promoted_temps = Some(tcx.mir_const_qualif(def_id).1); - Mode::Const - } - hir::BodyOwnerKind::Static(hir::MutImmutable) => Mode::Static, - hir::BodyOwnerKind::Static(hir::MutMutable) => Mode::StaticMut, - }; + let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); + + let mode = determine_mode(tcx, hir_id, def_id); debug!("run_pass: mode={:?}", mode); - if mode == Mode::NonConstFn || mode == Mode::ConstFn { + if let Mode::NonConstFn | Mode::ConstFn = mode { // This is ugly because Checker holds onto mir, // which can't be mutated until its scope ends. let (temps, candidates) = { @@ -1664,6 +1650,11 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { promote_consts::promote_candidates(def_id, body, tcx, temps, candidates) ); } else { + let const_promoted_temps = match mode { + Mode::Const => Some(tcx.mir_const_qualif(def_id).1), + _ => None, + }; + if !body.control_flow_destroyed.is_empty() { let mut locals = body.vars_iter(); if let Some(local) = locals.next() { @@ -1695,11 +1686,10 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { error.emit(); } } - let promoted_temps = if mode == Mode::Const { + let promoted_temps = match mode { // Already computed by `mir_const_qualif`. - const_promoted_temps.unwrap() - } else { - Checker::new(tcx, def_id, body, mode).check_const().1 + Mode::Const => const_promoted_temps.unwrap(), + _ => Checker::new(tcx, def_id, body, mode).check_const().1, }; // In `const` and `static` everything without `StorageDead` @@ -1747,7 +1737,7 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { let ty = body.return_ty(); tcx.infer_ctxt().enter(|infcx| { let param_env = ty::ParamEnv::empty(); - let cause = traits::ObligationCause::new(body.span, id, traits::SharedStatic); + let cause = traits::ObligationCause::new(body.span, hir_id, traits::SharedStatic); let mut fulfillment_cx = traits::FulfillmentContext::new(); fulfillment_cx.register_bound(&infcx, param_env, @@ -1765,6 +1755,17 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { } } +fn determine_mode(tcx: TyCtxt<'_>, hir_id: HirId, def_id: DefId) -> Mode { + match tcx.hir().body_owner_kind(hir_id) { + hir::BodyOwnerKind::Closure => Mode::NonConstFn, + hir::BodyOwnerKind::Fn if tcx.is_const_fn(def_id) => Mode::ConstFn, + hir::BodyOwnerKind::Fn => Mode::NonConstFn, + hir::BodyOwnerKind::Const => Mode::Const, + hir::BodyOwnerKind::Static(hir::MutImmutable) => Mode::Static, + hir::BodyOwnerKind::Static(hir::MutMutable) => Mode::StaticMut, + } +} + fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option> { let attrs = tcx.get_attrs(def_id); let attr = attrs.iter().find(|a| a.check_name(sym::rustc_args_required_const))?; From 8f184b369d6332195ef61f0395cf32bd787e4368 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 08:58:52 +0200 Subject: [PATCH 2/8] qualify_consts: misc cleanup. --- src/librustc_mir/transform/qualify_consts.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 959d21ecfadeb..fcbb3433a62d9 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1714,12 +1714,8 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { }, target, .. - } => { - if promoted_temps.contains(index) { - terminator.kind = TerminatorKind::Goto { - target, - }; - } + } if promoted_temps.contains(index) => { + terminator.kind = TerminatorKind::Goto { target }; } _ => {} } @@ -1727,7 +1723,7 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { } // Statics must be Sync. - if mode == Mode::Static { + if let Mode::Static = mode { // `#[thread_local]` statics don't have to be `Sync`. for attr in &tcx.get_attrs(def_id)[..] { if attr.check_name(sym::thread_local) { From b6360fbc4c6d3898d51dfcbe9b042fef515133d1 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 09:10:26 +0200 Subject: [PATCH 3/8] qualify_consts: extract check_short_circuiting_in_const_local. --- src/librustc_mir/transform/qualify_consts.rs | 66 +++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index fcbb3433a62d9..0a9e2a5974300 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1655,37 +1655,8 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { _ => None, }; - if !body.control_flow_destroyed.is_empty() { - let mut locals = body.vars_iter(); - if let Some(local) = locals.next() { - let span = body.local_decls[local].source_info.span; - let mut error = tcx.sess.struct_span_err( - span, - &format!( - "new features like let bindings are not permitted in {}s \ - which also use short circuiting operators", - mode, - ), - ); - for (span, kind) in body.control_flow_destroyed.iter() { - error.span_note( - *span, - &format!("use of {} here does not actually short circuit due to \ - the const evaluator presently not being able to do control flow. \ - See https://github.com/rust-lang/rust/issues/49146 for more \ - information.", kind), - ); - } - for local in locals { - let span = body.local_decls[local].source_info.span; - error.span_note( - span, - "more locals defined here", - ); - } - error.emit(); - } - } + check_short_circuiting_in_const_local(tcx, body, mode); + let promoted_temps = match mode { // Already computed by `mir_const_qualif`. Mode::Const => const_promoted_temps.unwrap(), @@ -1762,6 +1733,39 @@ fn determine_mode(tcx: TyCtxt<'_>, hir_id: HirId, def_id: DefId) -> Mode { } } +fn check_short_circuiting_in_const_local(tcx: TyCtxt<'_>, body: &mut Body<'tcx>, mode: Mode) { + if body.control_flow_destroyed.is_empty() { + return; + } + + let mut locals = body.vars_iter(); + if let Some(local) = locals.next() { + let span = body.local_decls[local].source_info.span; + let mut error = tcx.sess.struct_span_err( + span, + &format!( + "new features like let bindings are not permitted in {}s \ + which also use short circuiting operators", + mode, + ), + ); + for (span, kind) in body.control_flow_destroyed.iter() { + error.span_note( + *span, + &format!("use of {} here does not actually short circuit due to \ + the const evaluator presently not being able to do control flow. \ + See https://github.com/rust-lang/rust/issues/49146 for more \ + information.", kind), + ); + } + for local in locals { + let span = body.local_decls[local].source_info.span; + error.span_note(span, "more locals defined here"); + } + error.emit(); + } +} + fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option> { let attrs = tcx.get_attrs(def_id); let attr = attrs.iter().find(|a| a.check_name(sym::rustc_args_required_const))?; From c1d440070e088c6b239bcf5f9f22dfc6d5b2b639 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 09:15:59 +0200 Subject: [PATCH 4/8] qualify_consts: fuse prompted_temps. --- src/librustc_mir/transform/qualify_consts.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 0a9e2a5974300..b052d1e9bb468 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1650,16 +1650,10 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { promote_consts::promote_candidates(def_id, body, tcx, temps, candidates) ); } else { - let const_promoted_temps = match mode { - Mode::Const => Some(tcx.mir_const_qualif(def_id).1), - _ => None, - }; - check_short_circuiting_in_const_local(tcx, body, mode); let promoted_temps = match mode { - // Already computed by `mir_const_qualif`. - Mode::Const => const_promoted_temps.unwrap(), + Mode::Const => tcx.mir_const_qualif(def_id).1, _ => Checker::new(tcx, def_id, body, mode).check_const().1, }; From 8af33b325377ea64fb9bbdc10073f9063293c9a6 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 09:35:37 +0200 Subject: [PATCH 5/8] qualify_consts: extract check_non_thread_local_static_is_sync --- src/librustc_mir/transform/qualify_consts.rs | 49 ++++++++++---------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index b052d1e9bb468..dbb994183e9d5 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1687,31 +1687,9 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { } } - // Statics must be Sync. if let Mode::Static = mode { - // `#[thread_local]` statics don't have to be `Sync`. - for attr in &tcx.get_attrs(def_id)[..] { - if attr.check_name(sym::thread_local) { - return; - } - } - let ty = body.return_ty(); - tcx.infer_ctxt().enter(|infcx| { - let param_env = ty::ParamEnv::empty(); - let cause = traits::ObligationCause::new(body.span, hir_id, traits::SharedStatic); - let mut fulfillment_cx = traits::FulfillmentContext::new(); - fulfillment_cx.register_bound(&infcx, - param_env, - ty, - tcx.require_lang_item( - lang_items::SyncTraitLangItem, - Some(body.span) - ), - cause); - if let Err(err) = fulfillment_cx.select_all_or_error(&infcx) { - infcx.report_fulfillment_errors(&err, None, false); - } - }); + // `static`s (not `static mut`s) which are not `#[thread_local]` must be `Sync`. + check_non_thread_local_static_is_sync(tcx, body, def_id, hir_id); } } } @@ -1760,6 +1738,29 @@ fn check_short_circuiting_in_const_local(tcx: TyCtxt<'_>, body: &mut Body<'tcx>, } } +fn check_non_thread_local_static_is_sync( + tcx: TyCtxt<'tcx>, + body: &mut Body<'tcx>, + def_id: DefId, + hir_id: HirId, +) { + // `#[thread_local]` statics don't have to be `Sync`. + if tcx.has_attr(def_id, sym::thread_local) { + return; + } + + let ty = body.return_ty(); + tcx.infer_ctxt().enter(|infcx| { + let cause = traits::ObligationCause::new(body.span, hir_id, traits::SharedStatic); + let mut fulfillment_cx = traits::FulfillmentContext::new(); + let sync_def_id = tcx.require_lang_item(lang_items::SyncTraitLangItem, Some(body.span)); + fulfillment_cx.register_bound(&infcx, ty::ParamEnv::empty(), ty, sync_def_id, cause); + if let Err(err) = fulfillment_cx.select_all_or_error(&infcx) { + infcx.report_fulfillment_errors(&err, None, false); + } + }); +} + fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option> { let attrs = tcx.get_attrs(def_id); let attr = attrs.iter().find(|a| a.check_name(sym::rustc_args_required_const))?; From 2f733aad5a73b7cb8ceec37a8ecec497f8807034 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 09:43:16 +0200 Subject: [PATCH 6/8] qualify_consts: extract remove_drop_and_storage_dead_on_promoted_locals. --- src/librustc_mir/transform/qualify_consts.rs | 63 +++++++++++--------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index dbb994183e9d5..fe02e120f3b49 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1656,35 +1656,7 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { Mode::Const => tcx.mir_const_qualif(def_id).1, _ => Checker::new(tcx, def_id, body, mode).check_const().1, }; - - // In `const` and `static` everything without `StorageDead` - // is `'static`, we don't have to create promoted MIR fragments, - // just remove `Drop` and `StorageDead` on "promoted" locals. - debug!("run_pass: promoted_temps={:?}", promoted_temps); - for block in body.basic_blocks_mut() { - block.statements.retain(|statement| { - match statement.kind { - StatementKind::StorageDead(index) => { - !promoted_temps.contains(index) - } - _ => true - } - }); - let terminator = block.terminator_mut(); - match terminator.kind { - TerminatorKind::Drop { - location: Place { - base: PlaceBase::Local(index), - projection: None, - }, - target, - .. - } if promoted_temps.contains(index) => { - terminator.kind = TerminatorKind::Goto { target }; - } - _ => {} - } - } + remove_drop_and_storage_dead_on_promoted_locals(body, promoted_temps); } if let Mode::Static = mode { @@ -1738,6 +1710,39 @@ fn check_short_circuiting_in_const_local(tcx: TyCtxt<'_>, body: &mut Body<'tcx>, } } +/// In `const` and `static` everything without `StorageDead` +/// is `'static`, we don't have to create promoted MIR fragments, +/// just remove `Drop` and `StorageDead` on "promoted" locals. +fn remove_drop_and_storage_dead_on_promoted_locals( + body: &mut Body<'tcx>, + promoted_temps: &BitSet, +) { + debug!("run_pass: promoted_temps={:?}", promoted_temps); + + for block in body.basic_blocks_mut() { + block.statements.retain(|statement| { + match statement.kind { + StatementKind::StorageDead(index) => !promoted_temps.contains(index), + _ => true + } + }); + let terminator = block.terminator_mut(); + match terminator.kind { + TerminatorKind::Drop { + location: Place { + base: PlaceBase::Local(index), + projection: None, + }, + target, + .. + } if promoted_temps.contains(index) => { + terminator.kind = TerminatorKind::Goto { target }; + } + _ => {} + } + } +} + fn check_non_thread_local_static_is_sync( tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, From 9196af0b3647c29cb1c268ba9b01546df55fc004 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 10:16:41 +0200 Subject: [PATCH 7/8] qualify_consts: extract error_min_const_fn_violation. --- src/librustc_mir/transform/qualify_consts.rs | 26 +++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index fe02e120f3b49..73475be24d445 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -25,6 +25,7 @@ use syntax::feature_gate::{emit_feature_err, GateIssue}; use syntax::symbol::sym; use syntax_pos::{Span, DUMMY_SP}; +use std::borrow::Cow; use std::cell::Cell; use std::fmt; use std::ops::{Deref, Index, IndexMut}; @@ -1607,26 +1608,14 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { // which can't be mutated until its scope ends. let (temps, candidates) = { let mut checker = Checker::new(tcx, def_id, body, mode); - if mode == Mode::ConstFn { + if let Mode::ConstFn = mode { if tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { checker.check_const(); } else if tcx.is_min_const_fn(def_id) { - // enforce `min_const_fn` for stable const fns + // Enforce `min_const_fn` for stable `const fn`s. use super::qualify_min_const_fn::is_min_const_fn; if let Err((span, err)) = is_min_const_fn(tcx, def_id, body) { - let mut diag = struct_span_err!( - tcx.sess, - span, - E0723, - "{}", - err, - ); - diag.note("for more information, see issue \ - https://github.com/rust-lang/rust/issues/57563"); - diag.help( - "add `#![feature(const_fn)]` to the crate attributes to enable", - ); - diag.emit(); + error_min_const_fn_violation(tcx, span, err); } else { // this should not produce any errors, but better safe than sorry // FIXME(#53819) @@ -1677,6 +1666,13 @@ fn determine_mode(tcx: TyCtxt<'_>, hir_id: HirId, def_id: DefId) -> Mode { } } +fn error_min_const_fn_violation(tcx: TyCtxt<'_>, span: Span, msg: Cow<'_, str>) { + struct_span_err!(tcx.sess, span, E0723, "{}", msg) + .note("for more information, see issue https://github.com/rust-lang/rust/issues/57563") + .help("add `#![feature(const_fn)]` to the crate attributes to enable") + .emit(); +} + fn check_short_circuiting_in_const_local(tcx: TyCtxt<'_>, body: &mut Body<'tcx>, mode: Mode) { if body.control_flow_destroyed.is_empty() { return; From 0a8a3dd88a1216e7487b8e39179fbac16d07bafe Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 31 Aug 2019 06:17:10 +0200 Subject: [PATCH 8/8] qualify_consts: move thread_local condition out. --- src/librustc_mir/transform/qualify_consts.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 73475be24d445..32b49ee942300 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1648,9 +1648,9 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { remove_drop_and_storage_dead_on_promoted_locals(body, promoted_temps); } - if let Mode::Static = mode { + if mode == Mode::Static && !tcx.has_attr(def_id, sym::thread_local) { // `static`s (not `static mut`s) which are not `#[thread_local]` must be `Sync`. - check_non_thread_local_static_is_sync(tcx, body, def_id, hir_id); + check_static_is_sync(tcx, body, hir_id); } } } @@ -1739,17 +1739,7 @@ fn remove_drop_and_storage_dead_on_promoted_locals( } } -fn check_non_thread_local_static_is_sync( - tcx: TyCtxt<'tcx>, - body: &mut Body<'tcx>, - def_id: DefId, - hir_id: HirId, -) { - // `#[thread_local]` statics don't have to be `Sync`. - if tcx.has_attr(def_id, sym::thread_local) { - return; - } - +fn check_static_is_sync(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, hir_id: HirId) { let ty = body.return_ty(); tcx.infer_ctxt().enter(|infcx| { let cause = traits::ObligationCause::new(body.span, hir_id, traits::SharedStatic);