Skip to content

Commit ded5ee0

Browse files
committed
Auto merge of #66170 - ecstatic-morse:hir-const-check, r=Centril,oli-obk
Add a HIR pass to check consts for `if`, `loop`, etc. Resolves #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 #62272). r? @Centril cc @eddyb (since they filed #62272)
2 parents 695fe96 + 7552bd6 commit ded5ee0

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+784
-357
lines changed

src/librustc/hir/map/mod.rs

+35
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,33 @@ impl<'hir> Entry<'hir> {
7979
}
8080
}
8181

82+
fn fn_sig(&self) -> Option<&'hir FnSig> {
83+
match &self.node {
84+
Node::Item(item) => {
85+
match &item.kind {
86+
ItemKind::Fn(sig, _, _) => Some(sig),
87+
_ => None,
88+
}
89+
}
90+
91+
Node::TraitItem(item) => {
92+
match &item.kind {
93+
TraitItemKind::Method(sig, _) => Some(sig),
94+
_ => None
95+
}
96+
}
97+
98+
Node::ImplItem(item) => {
99+
match &item.kind {
100+
ImplItemKind::Method(sig, _) => Some(sig),
101+
_ => None,
102+
}
103+
}
104+
105+
_ => None,
106+
}
107+
}
108+
82109
fn associated_body(self) -> Option<BodyId> {
83110
match self.node {
84111
Node::Item(item) => {
@@ -450,6 +477,14 @@ impl<'hir> Map<'hir> {
450477
}
451478
}
452479

480+
pub fn fn_sig_by_hir_id(&self, hir_id: HirId) -> Option<&'hir FnSig> {
481+
if let Some(entry) = self.find_entry(hir_id) {
482+
entry.fn_sig()
483+
} else {
484+
bug!("no entry for hir_id `{}`", hir_id)
485+
}
486+
}
487+
453488
/// Returns the `HirId` that corresponds to the definition of
454489
/// which this is the body of, i.e., a `fn`, `const` or `static`
455490
/// item (possibly associated), a closure, or a `hir::AnonConst`.

src/librustc/query/mod.rs

+5
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,11 @@ rustc_queries! {
329329
desc { |tcx| "checking for unstable API usage in {}", key.describe_as_module(tcx) }
330330
}
331331

332+
/// Checks the const bodies in the module for illegal operations (e.g. `if` or `loop`).
333+
query check_mod_const_bodies(key: DefId) -> () {
334+
desc { |tcx| "checking consts in {}", key.describe_as_module(tcx) }
335+
}
336+
332337
/// Checks the loops in the module.
333338
query check_mod_loops(key: DefId) -> () {
334339
desc { |tcx| "checking loops in {}", key.describe_as_module(tcx) }

src/librustc_interface/passes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,7 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> {
875875
tcx.ensure().check_mod_loops(local_def_id);
876876
tcx.ensure().check_mod_attrs(local_def_id);
877877
tcx.ensure().check_mod_unstable_api_usage(local_def_id);
878+
tcx.ensure().check_mod_const_bodies(local_def_id);
878879
});
879880
});
880881
});

src/librustc_mir/transform/check_consts/validation.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,14 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
461461
self.super_statement(statement, location);
462462
}
463463
StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => {
464-
self.check_op(ops::IfOrMatch);
464+
// FIXME: make this the `emit_error` impl of `ops::IfOrMatch` once the const
465+
// checker is no longer run in compatability mode.
466+
if !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
467+
self.tcx.sess.delay_span_bug(
468+
self.span,
469+
"complex control flow is forbidden in a const context",
470+
);
471+
}
465472
}
466473
// FIXME(eddyb) should these really do nothing?
467474
StatementKind::FakeRead(..) |

src/librustc_mir/transform/qualify_consts.rs

+12-3
Original file line numberDiff line numberDiff line change
@@ -723,8 +723,12 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
723723
bb = target;
724724
}
725725
_ => {
726-
self.not_const(ops::Loop);
727-
validator.check_op(ops::Loop);
726+
if !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
727+
self.tcx.sess.delay_span_bug(
728+
self.span,
729+
"complex control flow is forbidden in a const context",
730+
);
731+
}
728732
break;
729733
}
730734
}
@@ -1253,7 +1257,12 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
12531257
self.super_statement(statement, location);
12541258
}
12551259
StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => {
1256-
self.not_const(ops::IfOrMatch);
1260+
if !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
1261+
self.tcx.sess.delay_span_bug(
1262+
self.span,
1263+
"complex control flow is forbidden in a const context",
1264+
);
1265+
}
12571266
}
12581267
// FIXME(eddyb) should these really do nothing?
12591268
StatementKind::FakeRead(..) |

src/librustc_passes/check_const.rs

+160
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
//! This pass checks HIR bodies that may be evaluated at compile-time (e.g., `const`, `static`,
2+
//! `const fn`) for structured control flow (e.g. `if`, `while`), which is forbidden in a const
3+
//! context.
4+
//!
5+
//! By the time the MIR const-checker runs, these high-level constructs have been lowered to
6+
//! control-flow primitives (e.g., `Goto`, `SwitchInt`), making it tough to properly attribute
7+
//! errors. We still look for those primitives in the MIR const-checker to ensure nothing slips
8+
//! through, but errors for structured control flow in a `const` should be emitted here.
9+
10+
use rustc::hir::def_id::DefId;
11+
use rustc::hir::intravisit::{Visitor, NestedVisitorMap};
12+
use rustc::hir::map::Map;
13+
use rustc::hir;
14+
use rustc::session::Session;
15+
use rustc::ty::TyCtxt;
16+
use rustc::ty::query::Providers;
17+
use syntax::ast::Mutability;
18+
use syntax::span_err;
19+
use syntax_pos::Span;
20+
21+
use std::fmt;
22+
23+
#[derive(Copy, Clone)]
24+
enum ConstKind {
25+
Static,
26+
StaticMut,
27+
ConstFn,
28+
Const,
29+
AnonConst,
30+
}
31+
32+
impl ConstKind {
33+
fn for_body(body: &hir::Body, hir_map: &Map<'_>) -> Option<Self> {
34+
let is_const_fn = |id| hir_map.fn_sig_by_hir_id(id).unwrap().header.is_const();
35+
36+
let owner = hir_map.body_owner(body.id());
37+
let const_kind = match hir_map.body_owner_kind(owner) {
38+
hir::BodyOwnerKind::Const => Self::Const,
39+
hir::BodyOwnerKind::Static(Mutability::Mutable) => Self::StaticMut,
40+
hir::BodyOwnerKind::Static(Mutability::Immutable) => Self::Static,
41+
42+
hir::BodyOwnerKind::Fn if is_const_fn(owner) => Self::ConstFn,
43+
hir::BodyOwnerKind::Fn | hir::BodyOwnerKind::Closure => return None,
44+
};
45+
46+
Some(const_kind)
47+
}
48+
}
49+
50+
impl fmt::Display for ConstKind {
51+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
52+
let s = match self {
53+
Self::Static => "static",
54+
Self::StaticMut => "static mut",
55+
Self::Const | Self::AnonConst => "const",
56+
Self::ConstFn => "const fn",
57+
};
58+
59+
write!(f, "{}", s)
60+
}
61+
}
62+
63+
fn check_mod_const_bodies(tcx: TyCtxt<'_>, module_def_id: DefId) {
64+
let mut vis = CheckConstVisitor::new(tcx);
65+
tcx.hir().visit_item_likes_in_module(module_def_id, &mut vis.as_deep_visitor());
66+
}
67+
68+
pub(crate) fn provide(providers: &mut Providers<'_>) {
69+
*providers = Providers {
70+
check_mod_const_bodies,
71+
..*providers
72+
};
73+
}
74+
75+
#[derive(Copy, Clone)]
76+
struct CheckConstVisitor<'tcx> {
77+
sess: &'tcx Session,
78+
hir_map: &'tcx Map<'tcx>,
79+
const_kind: Option<ConstKind>,
80+
}
81+
82+
impl<'tcx> CheckConstVisitor<'tcx> {
83+
fn new(tcx: TyCtxt<'tcx>) -> Self {
84+
CheckConstVisitor {
85+
sess: &tcx.sess,
86+
hir_map: tcx.hir(),
87+
const_kind: None,
88+
}
89+
}
90+
91+
/// Emits an error when an unsupported expression is found in a const context.
92+
fn const_check_violated(&self, bad_op: &str, span: Span) {
93+
if self.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
94+
self.sess.span_warn(span, "skipping const checks");
95+
return;
96+
}
97+
98+
let const_kind = self.const_kind
99+
.expect("`const_check_violated` may only be called inside a const context");
100+
101+
span_err!(self.sess, span, E0744, "`{}` is not allowed in a `{}`", bad_op, const_kind);
102+
}
103+
104+
/// Saves the parent `const_kind` before calling `f` and restores it afterwards.
105+
fn recurse_into(&mut self, kind: Option<ConstKind>, f: impl FnOnce(&mut Self)) {
106+
let parent_kind = self.const_kind;
107+
self.const_kind = kind;
108+
f(self);
109+
self.const_kind = parent_kind;
110+
}
111+
}
112+
113+
impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> {
114+
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
115+
NestedVisitorMap::OnlyBodies(&self.hir_map)
116+
}
117+
118+
fn visit_anon_const(&mut self, anon: &'tcx hir::AnonConst) {
119+
let kind = Some(ConstKind::AnonConst);
120+
self.recurse_into(kind, |this| hir::intravisit::walk_anon_const(this, anon));
121+
}
122+
123+
fn visit_body(&mut self, body: &'tcx hir::Body) {
124+
let kind = ConstKind::for_body(body, self.hir_map);
125+
self.recurse_into(kind, |this| hir::intravisit::walk_body(this, body));
126+
}
127+
128+
fn visit_expr(&mut self, e: &'tcx hir::Expr) {
129+
match &e.kind {
130+
// Skip the following checks if we are not currently in a const context.
131+
_ if self.const_kind.is_none() => {}
132+
133+
hir::ExprKind::Loop(_, _, source) => {
134+
self.const_check_violated(source.name(), e.span);
135+
}
136+
137+
hir::ExprKind::Match(_, _, source) => {
138+
use hir::MatchSource::*;
139+
140+
let op = match source {
141+
Normal => Some("match"),
142+
IfDesugar { .. } | IfLetDesugar { .. } => Some("if"),
143+
TryDesugar => Some("?"),
144+
AwaitDesugar => Some(".await"),
145+
146+
// These are handled by `ExprKind::Loop` above.
147+
WhileDesugar | WhileLetDesugar | ForLoopDesugar => None,
148+
};
149+
150+
if let Some(op) = op {
151+
self.const_check_violated(op, e.span);
152+
}
153+
}
154+
155+
_ => {},
156+
}
157+
158+
hir::intravisit::walk_expr(self, e);
159+
}
160+
}

src/librustc_passes/error_codes.rs

+22
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,28 @@ async fn foo() {}
626626
Switch to the Rust 2018 edition to use `async fn`.
627627
"##,
628628

629+
E0744: r##"
630+
Control-flow expressions are not allowed inside a const context.
631+
632+
At the moment, `if` and `match`, as well as the looping constructs `for`,
633+
`while`, and `loop`, are forbidden inside a `const`, `static`, or `const fn`.
634+
635+
```compile_fail,E0744
636+
const _: i32 = {
637+
let mut x = 0;
638+
loop {
639+
x += 1;
640+
if x == 4 {
641+
break;
642+
}
643+
}
644+
645+
x
646+
};
647+
```
648+
649+
"##,
650+
629651
;
630652
E0226, // only a single explicit lifetime bound is permitted
631653
E0472, // asm! is unsupported on this target

src/librustc_passes/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use rustc::ty::query::Providers;
2323
pub mod error_codes;
2424

2525
pub mod ast_validation;
26+
mod check_const;
2627
pub mod hir_stats;
2728
pub mod layout_test;
2829
pub mod loops;
@@ -32,6 +33,7 @@ mod liveness;
3233
mod intrinsicck;
3334

3435
pub fn provide(providers: &mut Providers<'_>) {
36+
check_const::provide(providers);
3537
entry::provide(providers);
3638
loops::provide(providers);
3739
liveness::provide(providers);

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ const fn f(x: usize) -> usize {
77
for i in 0..x {
88
//~^ ERROR E0015
99
//~| ERROR E0017
10-
//~| ERROR E0019
11-
//~| ERROR E0019
1210
//~| ERROR E0080
11+
//~| ERROR E0744
1312
sum += i;
1413
}
1514
sum

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
fn main() {
22
[(); & { loop { continue } } ]; //~ ERROR mismatched types
3+
//~^ ERROR `loop` is not allowed in a `const`
34
[(); loop { break }]; //~ ERROR mismatched types
5+
//~^ ERROR `loop` is not allowed in a `const`
46
[(); {while true {break}; 0}];
5-
//~^ ERROR constant contains unimplemented expression type
6-
//~| ERROR constant contains unimplemented expression type
7+
//~^ ERROR `while` is not allowed in a `const`
78
//~| WARN denote infinite loops with
89
[(); { for _ in 0usize.. {}; 0}];
910
//~^ ERROR calls in constants are limited to constant functions
11+
//~| ERROR `for` is not allowed in a `const`
1012
//~| ERROR references in constants may only refer to immutable values
11-
//~| ERROR constant contains unimplemented expression type
12-
//~| ERROR constant contains unimplemented expression type
1313
//~| ERROR evaluation of constant value failed
1414
}

src/test/ui/borrowck/issue-64453.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ struct Project;
22
struct Value;
33

44
static settings_dir: String = format!("");
5-
//~^ ERROR [E0019]
6-
//~| ERROR [E0015]
7-
//~| ERROR [E0015]
5+
//~^ ERROR `match` is not allowed in a `static`
86

97
fn from_string(_: String) -> Value {
108
Value
@@ -13,7 +11,6 @@ fn set_editor(_: Value) {}
1311

1412
fn main() {
1513
let settings_data = from_string(settings_dir);
16-
//~^ ERROR cannot move out of static item `settings_dir` [E0507]
1714
let args: i32 = 0;
1815

1916
match args {

0 commit comments

Comments
 (0)