Emit diagnostic when using const storing Fn as pattern

[estebank]

fn f1() { println!("1"); }
fn f2() { println!("2"); }

struct S(fn());
const CONST: S = S(f1);

fn main() {
    let x = S(f2);

    match x {
        CONST => {
            println!("match");
        }
        _ => {}
    }
}

error: pattern contains `Fn`, use pattern guard instead
  --> <anon>.rs:14:9
   |
13 |         CONST => {
   |         ^^^^^ pattern contains `Fn`
   |
help: use a pattern guard instead:
   |         x if x == CONST => {

Fix #40657.

[rust-highfive]

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

[durka]

The error message seems vague and confusing to me. What's the reason why a function can't be in a match pattern?

[estebank]

@durka the current compiler output is an ICE. Only reason I see to disallow it is not to change behaviour.

[petrochenkov]

@estebank
You can add checks for raw pointers in constants and fix #34784 as well.

[nikomatsakis]

I"m not sure if this check is in quite the right spot. I think I would expect it to be in the lower_path() function instead...although, reading through the code, this seems like a not terrible place as well. What I had in mind feels somehow strictly more correct, but would involve copying a non-trivial number of patterns over into lower_path() (i.e., those cases where lower_variant_or_leaf() is a non-bug).

[durka]

I guess I don't understand why the example program at the top is an unreasonable program. What's wrong with checking which fn I have in a variable? As a user, with this PR I get:

1. an inscrutable error message
2. suggested code that doesn't compile

Yes it's better than an ICE, but this is not a good situation.

[arielb1]

@estebank

This should be using lowercase-f fn rather than uppercase-f Fn.

[eddyb]

Why can't we handle this as fn pointers not being "structurally matchable"?

EDIT: Or, you know, we could implement it. Although I'm not sure we support raw pointer comparisons right now?

EDIT2: Nevermind, an explicit fn cast is a const-evaluation error. This should be handled in const-eval (there's a coercion that is completely ignored right now), it's not pattern-specific.

[eddyb]

i.e. just before evaluating an expression we should check for a reification adjustment on that expression and signal!(e, UnimplementedConstVal("casting fn pointers")) like for as casts.

[arielb1]

@eddyb

Why don't we want to support reifying fn pointers in const-eval? With MIRI, we'll have to, and still face this problem.

[eddyb]

@arielb1 It really comes down to whether we want to allow it in patterns - if we don't, we should ban it through "structural match", if we want to fix it so it works it's relatively straight-forward.

[arielb1]

Where structural_match = altering lower_const_expr?

[eddyb]

I mean that constants of types that contain function pointers can't be matched on - same thing with floats.

[alexcrichton]

Hey @estebank thanks again for the PR! It sounds like there's a few changes requested, though, do you think you'll have a chance to look into them?

[estebank]

@alexcrichton I will, but probably not before the weekend. Would you prefer to keep this PR open for the comment history or creating a clean PR when I get back to this?

[alexcrichton]

Oh yeah it's fine to keep open, just doing some PR triaging this morning :)

[alexcrichton]

Ah ok coming back around to this, I'm gonna close this for now to help clear out the queue, but @estebank please feel free to resubmit!

[estebank]

@alexcrichton I spent the last weekend looking into this, and hit a wall as the amount of code needed for getting it to work appropriately was getting bigger and bigger. I'll probably get back to this sometime in the future, but I can help if somebody else wants to take it before then.