Add new function_casts_as_integer lint

[GuillaumeGomez]

The function_casts_as_integer lint detects cases where users cast a function pointer into an integer.

warn-by-default

Example

fn foo() {}
let x = foo as usize;

warning: casting a function into an integer implicitly
  --> $DIR/function_casts_as_integer.rs:9:17
   |
LL |     let x = foo as usize;
   |                 ^^^^^^^^
   |
help: add `fn() as usize`
   |
LL |     let x = foo as fn() as usize;
   |                 +++++++

Explanation

You should never cast a function directly into an integer but go through a cast as fn first to make it obvious what's going on. It also allows to prevent confusion with (associated) constants.

Related to #81686 and https://stackoverflow.com/questions/68701177/whats-the-meaning-of-casting-a-rust-enum-variant-to-a-numeric-data-type

r? @Urgau

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[flip1995]

Clippy has a few lints for fn to integer casts. But they are all restriction or style lints in Clippy. Adding a warn-by-default lint about this to rustc might be a bit aggressive 🤔

[GuillaumeGomez]

I know, I implemented one myself. 😉 I think it highlights the fact that this is a big issue and that the compiler should warn about it and eventually even forbid this fn to integer cast (you need to cast to an fn pointer first).

But in any case, it's up to the lang team.

[flip1995]

Agreed 👍 Just want to add this information as "prior art" for the lang team to make this decision. Even though it might've sounded like it, I'm not against adding this lint to rustc.

Clippy question: Do you think if this lint gets added to rustc, we can (partially) deprecate Clippy lints?

[GuillaumeGomez]

Hard to say. For example confusing_method_to_numeric_cast provides extra information about what (likely) went wrong. But with the current lint, they likely would already have seen the problem and fixed it. So by default I'd say yes. But we could eventually uplift part of them to add the extra context clippy has that this lint doesn't provide. Would make it much more interesting and even more useful.

[flip1995]

Yeah, a partial uplift might be good then, should this be accepted.

[GuillaumeGomez]

CI finally passed again!

[bors]

☔ The latest upstream changes (presumably #138677) made this pull request unmergeable. Please resolve the merge conflicts.

[GuillaumeGomez]

Oh no. T_T

[GuillaumeGomez]

CI passed. :)

[GuillaumeGomez]

Nominating it for @rust-lang/lang.

[traviscross]

Thanks for the PR and proposal. In simple examples, e.g. let x = f as f() as usize, this seems OK, but most real functions are going to have arguments and return values, and I could see it being rather annoying to have to write these out to satisfy the lint. I observe also that this, which is what people would reasonably reach for, does not work today:

fn f(x: u8) -> u8 { x }

fn main() {
  let x: usize = f as _ as usize;
  //~                 ^ ERROR type annotations needed
}

There are other deeper issues with these casts. See:

• fn_cast! macro #140803

I'm going to unnominate this one for now, as we likely are going to need to resolve #140803 first, and that might provide a clearer path forward for this as well.

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

[GuillaumeGomez]

Sorry but I disagree, you can just copy what the lint tells you. The whole point is to have these annotations appear in the code, in part to prevent confusing associated items with methods. So putting back the nomination.

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

[traviscross]

OK. Fair enough. We can discuss. What are your thoughts on the mentioned interaction with #140803 though?

[traviscross]

See also:

• ptr::fn_code_ptr for explicitly getting an opaque pointer of a function libs-team#589
  • ...and particularly ptr::fn_code_ptr for explicitly getting an opaque pointer of a function libs-team#589 (comment)
• Tracking Issue for ptr::fn_addr_eq #129322