Errors for `if` and `match` in a `const` are not descriptive

[ecstatic-morse]

Consider the following code (Playground):

const _: i32 = match 1 {
    2 => 3,
    4 => 5,
    _ => 0,
};

const _: i32 = if true {
    4
} else {
    5
};

This is not valid today, since if and match are forbidden in constants. That restriction can be lifted soon on nightly, so we would like to emit good errors that point the user to the offending code and suggest a feature gate if it would help.

However, the errors emitted by the nightly compiler are not great:

error[E0019]: constant contains unimplemented expression type
 --> src/lib.rs:1:22
  |
1 | const _: i32 = match 1 {
  |                      ^

error[E0019]: constant contains unimplemented expression type
 --> src/lib.rs:2:5
  |
2 |     2 => 3,
  |     ^

error[E0019]: constant contains unimplemented expression type
 --> src/lib.rs:7:19
  |
7 | const _: i32 = if true {
  |                   ^^^^

error[E0019]: constant contains unimplemented expression type
  --> src/lib.rs:7:16
   |
7  |   const _: i32 = if true {
   |  ________________^
8  | |     4
9  | | } else {
10 | |     5
11 | | };
   | |_^

error: aborting due to 4 previous errors

At present, each const has two errors associated with it. One is triggered by the FakeRead emitted for the borrow checker and points to the value being branched on (1 and true) respectively. The other is triggered by the SwitchInt terminator itself. While the span of this error is good for the if (it points to the whole if-else construct), it is very confusing for the match, where it points to the pattern in the first match arm despite the SwitchInt handling all arms simultaneously.

I've thought a bit about how to improve this, but I don't have a great solution. I'm hoping someone can help. I don't think we want to rely only on the presence of FakeRead to reject branchy code in a const; SwitchInt seems like the natural candidate (maybe with a fallback to FakeRead to detect single-arm match statements). However, I don't know how to improve the span for the SwitchInt for the match, or if doing so would cause other diagnostics to regress.

I think the ideal approach would be to check for this at the HIR or AST level. This would give us very precise spans, and we could continue checking for SwitchInt in the MIR to make sure the high-level check didn't miss anything. Is this feasible?

[Centril]

I think the ideal approach would be to check for this at the HIR or AST level. This would give us very precise spans, and we could continue checking for SwitchInt in the MIR to make sure the high-level check didn't miss anything. Is this feasible?

You could use .delay_span_bug in the MIR check and use a simple HIR visitor to reject certain ExprKind::s, e.g. ::Match(...) and ::Loop(...) should be pretty all-encompassing.