Auto merge of rust-lang#66170 - ecstatic-morse:hir-const-check, r=Centril,oli-obk

Add a HIR pass to check consts for `if`, `loop`, etc.

Resolves rust-lang#66125.

This PR adds a HIR pass to check for high-level control flow constructs that are forbidden in a const-context. The MIR const-checker is unable to provide good spans for these since they are lowered to control flow primitives (e.g., `Goto` and `SwitchInt`), and these often don't map back to the underlying statement as a whole. This PR is intended only to improve diagnostics once `if` and `match` become commonplace in constants (behind a feature flag). The MIR const-checker will continue to operate unchanged, and will catch anything this check might miss.

In this implementation, the HIR const-checking pass is run much earlier than the MIR one, so it will supersede any errors from the latter. I will need some mentoring if we wish to change this, since I'm not familiar with the diagnostics system. Moving this pass into the same phase as the MIR const-checker could also help keep backwards compatibility for items like `const _: () = loop { break; };`, which are currently (erroneously?) accepted by the MIR const-checker (see rust-lang#62272).

r? @Centril
cc @eddyb (since they filed rust-lang#62272)

Author: bors

src/librustc/hir/map/mod.rs

@@ -79,6 +79,33 @@ impl<'hir> Entry<'hir> {
         }
     }
 
+    fn fn_sig(&self) -> Option<&'hir FnSig> {
+        match &self.node {
+            Node::Item(item) => {
+                match &item.kind {
+                    ItemKind::Fn(sig, _, _) => Some(sig),
+                    _ => None,
+                }
+            }
+
+            Node::TraitItem(item) => {
+                match &item.kind {
+                    TraitItemKind::Method(sig, _) => Some(sig),
+                    _ => None
+                }
+            }
+
+            Node::ImplItem(item) => {
+                match &item.kind {
+                    ImplItemKind::Method(sig, _) => Some(sig),
+                    _ => None,
+                }
+            }
+
+            _ => None,
+        }
+    }
+
     fn associated_body(self) -> Option<BodyId> {
         match self.node {
             Node::Item(item) => {
@@ -450,6 +477,14 @@ impl<'hir> Map<'hir> {
         }
     }
 
+    pub fn fn_sig_by_hir_id(&self, hir_id: HirId) -> Option<&'hir FnSig> {
+        if let Some(entry) = self.find_entry(hir_id) {
+            entry.fn_sig()
+        } else {
+            bug!("no entry for hir_id `{}`", hir_id)
+        }
+    }
+
     /// Returns the `HirId` that corresponds to the definition of
     /// which this is the body of, i.e., a `fn`, `const` or `static`
     /// item (possibly associated), a closure, or a `hir::AnonConst`.

src/librustc/query/mod.rs

@@ -329,6 +329,11 @@ rustc_queries! {
             desc { |tcx| "checking for unstable API usage in {}", key.describe_as_module(tcx) }
         }
 
+        /// Checks the const bodies in the module for illegal operations (e.g. `if` or `loop`).
+        query check_mod_const_bodies(key: DefId) -> () {
+            desc { |tcx| "checking consts in {}", key.describe_as_module(tcx) }
+        }
+
         /// Checks the loops in the module.
         query check_mod_loops(key: DefId) -> () {
             desc { |tcx| "checking loops in {}", key.describe_as_module(tcx) }

src/librustc_interface/passes.rs

@@ -875,6 +875,7 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> {
                 tcx.ensure().check_mod_loops(local_def_id);
                 tcx.ensure().check_mod_attrs(local_def_id);
                 tcx.ensure().check_mod_unstable_api_usage(local_def_id);
+                tcx.ensure().check_mod_const_bodies(local_def_id);
             });
         });
     });

src/librustc_mir/transform/check_consts/validation.rs

@@ -461,7 +461,14 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
                 self.super_statement(statement, location);
             }
             StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => {
-                self.check_op(ops::IfOrMatch);
+                // FIXME: make this the `emit_error` impl of `ops::IfOrMatch` once the const
+                // checker is no longer run in compatability mode.
+                if !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
+                    self.tcx.sess.delay_span_bug(
+                        self.span,
+                        "complex control flow is forbidden in a const context",
+                    );
+                }
             }
             // FIXME(eddyb) should these really do nothing?
             StatementKind::FakeRead(..) |

src/librustc_mir/transform/qualify_consts.rs

@@ -723,8 +723,12 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
                     bb = target;
                 }
                 _ => {
-                    self.not_const(ops::Loop);
-                    validator.check_op(ops::Loop);
+                    if !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
+                        self.tcx.sess.delay_span_bug(
+                            self.span,
+                            "complex control flow is forbidden in a const context",
+                        );
+                    }
                     break;
                 }
             }
@@ -1253,7 +1257,12 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
                 self.super_statement(statement, location);
             }
             StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => {
-                self.not_const(ops::IfOrMatch);
+                if !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
+                    self.tcx.sess.delay_span_bug(
+                        self.span,
+                        "complex control flow is forbidden in a const context",
+                    );
+                }
             }
             // FIXME(eddyb) should these really do nothing?
             StatementKind::FakeRead(..) |

src/librustc_passes/check_const.rs

@@ -0,0 +1,160 @@
+//! This pass checks HIR bodies that may be evaluated at compile-time (e.g., `const`, `static`,
+//! `const fn`) for structured control flow (e.g. `if`, `while`), which is forbidden in a const
+//! context.
+//!
+//! By the time the MIR const-checker runs, these high-level constructs have been lowered to
+//! control-flow primitives (e.g., `Goto`, `SwitchInt`), making it tough to properly attribute
+//! errors. We still look for those primitives in the MIR const-checker to ensure nothing slips
+//! through, but errors for structured control flow in a `const` should be emitted here.
+
+use rustc::hir::def_id::DefId;
+use rustc::hir::intravisit::{Visitor, NestedVisitorMap};
+use rustc::hir::map::Map;
+use rustc::hir;
+use rustc::session::Session;
+use rustc::ty::TyCtxt;
+use rustc::ty::query::Providers;
+use syntax::ast::Mutability;
+use syntax::span_err;
+use syntax_pos::Span;
+
+use std::fmt;
+
+#[derive(Copy, Clone)]
+enum ConstKind {
+    Static,
+    StaticMut,
+    ConstFn,
+    Const,
+    AnonConst,
+}
+
+impl ConstKind {
+    fn for_body(body: &hir::Body, hir_map: &Map<'_>) -> Option<Self> {
+        let is_const_fn = |id| hir_map.fn_sig_by_hir_id(id).unwrap().header.is_const();
+
+        let owner = hir_map.body_owner(body.id());
+        let const_kind = match hir_map.body_owner_kind(owner) {
+            hir::BodyOwnerKind::Const => Self::Const,
+            hir::BodyOwnerKind::Static(Mutability::Mutable) => Self::StaticMut,
+            hir::BodyOwnerKind::Static(Mutability::Immutable) => Self::Static,
+
+            hir::BodyOwnerKind::Fn if is_const_fn(owner) => Self::ConstFn,
+            hir::BodyOwnerKind::Fn | hir::BodyOwnerKind::Closure => return None,
+        };
+
+        Some(const_kind)
+    }
+}
+
+impl fmt::Display for ConstKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let s = match self {
+            Self::Static => "static",
+            Self::StaticMut => "static mut",
+            Self::Const | Self::AnonConst => "const",
+            Self::ConstFn => "const fn",
+        };
+
+        write!(f, "{}", s)
+    }
+}
+
+fn check_mod_const_bodies(tcx: TyCtxt<'_>, module_def_id: DefId) {
+    let mut vis = CheckConstVisitor::new(tcx);
+    tcx.hir().visit_item_likes_in_module(module_def_id, &mut vis.as_deep_visitor());
+}
+
+pub(crate) fn provide(providers: &mut Providers<'_>) {
+    *providers = Providers {
+        check_mod_const_bodies,
+        ..*providers
+    };
+}
+
+#[derive(Copy, Clone)]
+struct CheckConstVisitor<'tcx> {
+    sess: &'tcx Session,
+    hir_map: &'tcx Map<'tcx>,
+    const_kind: Option<ConstKind>,
+}
+
+impl<'tcx> CheckConstVisitor<'tcx> {
+    fn new(tcx: TyCtxt<'tcx>) -> Self {
+        CheckConstVisitor {
+            sess: &tcx.sess,
+            hir_map: tcx.hir(),
+            const_kind: None,
+        }
+    }
+
+    /// Emits an error when an unsupported expression is found in a const context.
+    fn const_check_violated(&self, bad_op: &str, span: Span) {
+        if self.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
+            self.sess.span_warn(span, "skipping const checks");
+            return;
+        }
+
+        let const_kind = self.const_kind
+            .expect("`const_check_violated` may only be called inside a const context");
+
+        span_err!(self.sess, span, E0744, "`{}` is not allowed in a `{}`", bad_op, const_kind);
+    }
+
+    /// Saves the parent `const_kind` before calling `f` and restores it afterwards.
+    fn recurse_into(&mut self, kind: Option<ConstKind>, f: impl FnOnce(&mut Self)) {
+        let parent_kind = self.const_kind;
+        self.const_kind = kind;
+        f(self);
+        self.const_kind = parent_kind;
+    }
+}
+
+impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> {
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.hir_map)
+    }
+
+    fn visit_anon_const(&mut self, anon: &'tcx hir::AnonConst) {
+        let kind = Some(ConstKind::AnonConst);
+        self.recurse_into(kind, |this| hir::intravisit::walk_anon_const(this, anon));
+    }
+
+    fn visit_body(&mut self, body: &'tcx hir::Body) {
+        let kind = ConstKind::for_body(body, self.hir_map);
+        self.recurse_into(kind, |this| hir::intravisit::walk_body(this, body));
+    }
+
+    fn visit_expr(&mut self, e: &'tcx hir::Expr) {
+        match &e.kind {
+            // Skip the following checks if we are not currently in a const context.
+            _ if self.const_kind.is_none() => {}
+
+            hir::ExprKind::Loop(_, _, source) => {
+                self.const_check_violated(source.name(), e.span);
+            }
+
+            hir::ExprKind::Match(_, _, source) => {
+                use hir::MatchSource::*;
+
+                let op = match source {
+                    Normal => Some("match"),
+                    IfDesugar { .. } | IfLetDesugar { .. } => Some("if"),
+                    TryDesugar => Some("?"),
+                    AwaitDesugar => Some(".await"),
+
+                    // These are handled by `ExprKind::Loop` above.
+                    WhileDesugar | WhileLetDesugar | ForLoopDesugar => None,
+                };
+
+                if let Some(op) = op {
+                    self.const_check_violated(op, e.span);
+                }
+            }
+
+            _ => {},
+        }
+
+        hir::intravisit::walk_expr(self, e);
+    }
+}

src/librustc_passes/error_codes.rs

@@ -626,6 +626,28 @@ async fn foo() {}
 Switch to the Rust 2018 edition to use `async fn`.
 "##,
 
+E0744: r##"
+Control-flow expressions are not allowed inside a const context.
+
+At the moment, `if` and `match`, as well as the looping constructs `for`,
+`while`, and `loop`, are forbidden inside a `const`, `static`, or `const fn`.
+
+```compile_fail,E0744
+const _: i32 = {
+    let mut x = 0;
+    loop {
+        x += 1;
+        if x == 4 {
+            break;
+        }
+    }
+
+    x
+};
+```
+
+"##,
+
 ;
     E0226, // only a single explicit lifetime bound is permitted
     E0472, // asm! is unsupported on this target

src/librustc_passes/lib.rs

@@ -23,6 +23,7 @@ use rustc::ty::query::Providers;
 pub mod error_codes;
 
 pub mod ast_validation;
+mod check_const;
 pub mod hir_stats;
 pub mod layout_test;
 pub mod loops;
@@ -32,6 +33,7 @@ mod liveness;
 mod intrinsicck;
 
 pub fn provide(providers: &mut Providers<'_>) {
+    check_const::provide(providers);
     entry::provide(providers);
     loops::provide(providers);
     liveness::provide(providers);

src/test/compile-fail/consts/const-fn-error.rs

@@ -7,9 +7,8 @@ const fn f(x: usize) -> usize {
     for i in 0..x {
         //~^ ERROR E0015
         //~| ERROR E0017
-        //~| ERROR E0019
-        //~| ERROR E0019
         //~| ERROR E0080
+        //~| ERROR E0744
         sum += i;
     }
     sum

src/test/compile-fail/issue-52443.rs

@@ -1,14 +1,14 @@
 fn main() {
     [(); & { loop { continue } } ]; //~ ERROR mismatched types
+    //~^ ERROR `loop` is not allowed in a `const`
     [(); loop { break }]; //~ ERROR mismatched types
+    //~^ ERROR `loop` is not allowed in a `const`
     [(); {while true {break}; 0}];
-    //~^ ERROR constant contains unimplemented expression type
-    //~| ERROR constant contains unimplemented expression type
+    //~^ ERROR `while` is not allowed in a `const`
     //~| WARN denote infinite loops with
     [(); { for _ in 0usize.. {}; 0}];
     //~^ ERROR calls in constants are limited to constant functions
+    //~| ERROR `for` is not allowed in a `const`
     //~| ERROR references in constants may only refer to immutable values
-    //~| ERROR constant contains unimplemented expression type
-    //~| ERROR constant contains unimplemented expression type
     //~| ERROR evaluation of constant value failed
 }