Auto merge of #63994 - Centril:refactor-qualify-consts, r=spastorino,oli-obk

Refactor the `MirPass for QualifyAndPromoteConstants`

This is an accumulation of drive-by commits while working on `Vec::new` as a stable `const fn`.
The PR is probably easiest read commit-by-commit.

r? @oli-obk

cc @eddyb @ecstatic-morse -- your two PRs #63812 and #63860 respectively will conflict with this a tiny bit but it should be trivial to reintegrate your changes atop of this.

Author: bors

src/librustc_mir/transform/qualify_consts.rs

@@ -25,11 +25,13 @@ 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};
 use std::usize;
 
+use rustc::hir::HirId;
 use crate::transform::{MirPass, MirSource};
 use super::promote_consts::{self, Candidate, TempState};
 
@@ -1596,51 +1598,24 @@ 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) = {
                 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)
@@ -1664,107 +1639,119 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> {
                 promote_consts::promote_candidates(def_id, body, tcx, temps, candidates)
             );
         } else {
-            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();
-                }
-            }
-            let promoted_temps = if mode == Mode::Const {
-                // Already computed by `mir_const_qualif`.
-                const_promoted_temps.unwrap()
-            } else {
-                Checker::new(tcx, def_id, body, mode).check_const().1
+            check_short_circuiting_in_const_local(tcx, body, mode);
+
+            let promoted_temps = match mode {
+                Mode::Const => tcx.mir_const_qualif(def_id).1,
+                _ => Checker::new(tcx, def_id, body, mode).check_const().1,
             };
+            remove_drop_and_storage_dead_on_promoted_locals(body, promoted_temps);
+        }
 
-            // 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,
-                            };
-                        }
-                    }
-                    _ => {}
-                }
-            }
+        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_static_is_sync(tcx, body, hir_id);
         }
+    }
+}
 
-        // Statics must be Sync.
-        if mode == Mode::Static {
-            // `#[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;
-                }
+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 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;
+    }
+
+    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();
+    }
+}
+
+/// 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<Local>,
+) {
+    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 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 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);
-                }
-            });
+        });
+        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_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);
+        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<FxHashSet<usize>> {
     let attrs = tcx.get_attrs(def_id);
     let attr = attrs.iter().find(|a| a.check_name(sym::rustc_args_required_const))?;