Lint on fn pointers comparisons in external macros

[Urgau]

This PR extends the recently stabilized unpredictable_function_pointer_comparisons lint to also lint on Option<{function pointer}> and as well as linting in external macros (as to catch assert_eq! and others).

assert_eq!(Some::<FnPtr>(func), Some(func as unsafe extern "C" fn()));
//~^ WARN function pointer comparisons

#[derive(PartialEq, Eq)]
struct A {
    f: fn(),
    //~^ WARN function pointer comparisons
}

Fixes #134527

[rustbot]

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[jieyouxu]

r? compiler

[compiler-errors]

This seems like it could be more generalized, but fine for now.

[compiler-errors]

Specifically linting in macros seems like a much larger extension of the lint, though. Was this ever discussed by T-lang?

[compiler-errors]

Though I agree with it in theory.

[Urgau]

Specifically linting in macros seems like a much larger extension of the lint, though. Was this ever discussed by T-lang?

Yeah, linting on external macros made me hesitate to put it in the same PR as linting on Option of fn ptrs given that it's a much bigger extension, but it's still in line with the goal of the lint as FCP-ed by T-lang (so it's fine? 🤷).

Anyway, cc @traviscross

[traviscross]

@rustbot labels +I-lang-nominated +T-lang

We did talk about the derive(PartialEq) case, and we had left that undecided, so let's nominate to discuss.

Here are some notes on that unanswered question, from an earlier set of minutes:

TC: Separately, on the 2024-01-10 call, we discussed some interesting use cases for function pointer comparison, especially when it's indirected through PartialEq. We had earlier said we didn't want to lint when such comparisons were indirected through generics, but we didn't address the non-generic case of simply composing such comparisons.

One example of how this is used is in the standard library, in Waker::will_wake:

https://doc.rust-lang.org/core/task/struct.Waker.html#method.will_wake

It's comparing multiple function pointers via a #[derive(PartialEq)] on the RawWakerVTable.

We decided on 2024-01-01 that this case was interesting and we wanted to think about it further. We opened a discussion thread about this:

https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Function.20pointer.20comparison.20and.20.60PartialEq.60

Since then, another interesting use case in the standard library was raised, in the formatting machinery:

https://doc.rust-lang.org/src/core/fmt/rt.rs.html

What do we think about these, and would we lint on derived PartialEq cases like these or no?

[Urgau]

I have extracted the Option<{fn ptr}> part into #134586 (which was merged).

This PR is now only about linting in external macros.

[compiler-errors]

This is still waiting on T-lang

@rustbot team

[traviscross]

This is good to go from the lang side. The impl looks good to me. What do you think, @compiler-errors?

@rustbot ready

[Urgau]

Based on earlier approval, #134536 (review).

@bors r=compiler-errors rollup

[bors]

📌 Commit 64d61fb has been approved by compiler-errors

It is now in the queue for this repository.

[traviscross]

@bors r-
@bors r=compiler-errors,traviscross rollup

[bors]

📌 Commit 64d61fb has been approved by compiler-errors,traviscross

It is now in the queue for this repository.

[Urgau]

@bors r=compiler-errors,traviscross rollup

[bors]

📌 Commit 14b3e63 has been approved by compiler-errors,traviscross

It is now in the queue for this repository.