Regression: Matching on function pointers fails when one of the formal params implements custom PartialEq

[archseer]

A contrived playground example:

#[derive(Eq)]
struct A {
  a: i64   
}

impl PartialEq for A {
    #[inline]
    fn eq(&self, other: &Self) -> bool {
        self.a.eq(&other.a)
    }
}

type Fn = fn(&[A]);

fn my_fn(args: &[A]) {
  println!("hello world");
}

const TEST: Fn = my_fn;

struct B(Fn);

fn main() {
  let s = B(my_fn);
  match s {
    B(TEST) => println!("matched"),
    _ => println!("didn't match")
  };
}

This code will build on stable and beta, but not on nightly.

error: to use a constant of type `A` in a pattern, `A` must be annotated with `#[derive(PartialEq, Eq)]`

It looks like it's related to #62307 possibly?

[nikomatsakis]

Compiler team triage: Definitely related to #62307. Going to assign to @pnkfelix for follow-up, although they are on leave until early September. It seems like this falls in a grey area: it was being accepted due to bugs in the existing code-base, however it is also a case that we likely could accept regardless of what rules we ultimately adopted. (Because the constant itself is a fn pointer.)

@archseer is this regression affecting a real project?

[archseer]

I would say this is definitely a bug and the old behavior was correct: we're comparing function pointers so it shouldn't matter whether params derive or implement PartialEq, since no concrete parameter will be compared, but the functions themselves.

Real projects being affected: I spotted the regression when updating my erlang VM implementation to latest nightly (and latest tokyo async/await):

I define an Export as an enum that can either be a native function or an erlang function: https://github.com/archseer/enigma/blob/f2030315c47e25fc795404185a8b23b0dce80bc0/enigma/src/exports_table.rs#L14

Then in the instruction engine I cheat and override some of the exports to define them as a instruction (for example apply/2, apply/3 both need to modify the flow to jump to a different instruction):

First I define a constant pointing to the function I want to override:
https://github.com/archseer/enigma/blob/f2030315c47e25fc795404185a8b23b0dce80bc0/enigma/src/instruction.rs#L790

Then I match on it and override:
https://github.com/archseer/enigma/blob/f2030315c47e25fc795404185a8b23b0dce80bc0/enigma/src/instruction.rs#L610

So the reason the code fails here is because bif::Fn is defined as pub type Fn = fn(&vm::Machine, &RcProcess, &[Term]) -> Result;, and Term has some custom code defined for PartialEq.

It's a bit of a hack, but this is more or less how erlang itself implements these specific instructions.

[Centril]

Reduced:

struct A;

fn my_fn<'a>(_: &'a [A]) {}

const TEST: for<'a> fn(&'a [A]) = my_fn;

struct B { f: for<'a> fn(&'a [A]) }

fn main() {
    let s = B { f: my_fn };
    match s {
        B { f: TEST } => {}
        _ => {}
    };
}

[Centril]

Changing to a raw pointer makes it compile successfully:

struct A;

fn my_fn(_: *const [A]) {}

const TEST: fn(*const [A]) = my_fn;

struct B { f: fn(*const [A]) }

fn main() {
    let s = B { f: my_fn };
    match s {
        B { f: TEST } => {}
        _ => {}
    };
}

[nikomatsakis]

check-in from compiler triage:

@pnkfelix is back next week. A few weeks back, @archseer reported a real project affected by this regression (sorry!). As I wrote last week, from a lang-team perspective this falls into a grey zone, though I personally am inclined to try and accept the code, both for backwards compat reasons and because I don't think there's a compelling reason not to (though that depends on what complications result, I suppose).

Overall, we need to settle our "constants in match patterns" story.

[nikomatsakis]

I'm going to leave this one nominated to bring it to felix's attention.

[archseer]

For the time being I've worked around the problem by taking the function pointers, casting them to usize then comparing that: archseer/enigma@81e4fea

[pnkfelix]

triage: P-high. Assigning to self. Lets see if I can come up with something to fix this before this regression leaks out to stable.

[pnkfelix]

(okay, after looking at the code, I think this should be pretty easy to fix and backport.)