Skip to content

Add new function_casts_as_integer lint #141470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented May 23, 2025

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 rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 23, 2025
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch 2 times, most recently from 07f2c3c to 4978962 Compare May 23, 2025 20:54
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch 2 times, most recently from 3db3153 to d8b1955 Compare May 24, 2025 10:10
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 24, 2025
@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch from d8b1955 to 45984df Compare May 24, 2025 18:45
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch from 45984df to a6107b4 Compare May 24, 2025 19:09
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch from a6107b4 to 24d757e Compare May 24, 2025 22:47
@rustbot
Copy link
Collaborator

rustbot commented May 24, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2025
@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch from 24d757e to 3529162 Compare May 27, 2025 14:12
@rustbot
Copy link
Collaborator

rustbot commented May 27, 2025

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

/// a cast as `fn` first to make it obvious what's going on. It also allows
/// to prevent confusion with (associated) constants.
pub FUNCTION_CASTS_AS_INTEGER,
Warn,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch from 4f843ad to 1645fd5 Compare May 27, 2025 18:47
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch from 1645fd5 to bc71088 Compare May 27, 2025 19:40
@rustbot
Copy link
Collaborator

rustbot commented May 27, 2025

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as resolved.

@GuillaumeGomez GuillaumeGomez force-pushed the function_casts_as_integer branch from bc71088 to 1adb4d1 Compare May 28, 2025 09:34
@GuillaumeGomez
Copy link
Member Author

CI finally passed. \o/

@GuillaumeGomez GuillaumeGomez added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 28, 2025
/// to prevent confusion with (associated) constants.
pub FUNCTION_CASTS_AS_INTEGER,
Warn,
"Casting a function into an integer",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's by convention always lowercase in the compiler.

Suggested change
"Casting a function into an integer",
"casting a function into an integer",

Comment on lines +10 to +11
/// The `function_casts_as_integer` lint detects cases where users cast a function into an
/// integer.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to mention the "users", might be auto-generated code.

Suggested change
/// The `function_casts_as_integer` lint detects cases where users cast a function into an
/// integer.
/// The `function_casts_as_integer` lint detects cases where a function item is casted
/// into an integer.

Comment on lines +24 to +26
/// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really explain what is the problem this lint is checking for, ie. why is it bad. What about saying that by casting a function item to an integer it's implicitly creating a function pointer and it's that one that is casted to an integer, and so by making it explicit it improves re-ability of the code and prevents bugs (in particular with enum variants).

Comment on lines +64 to +83
#[derive(LintDiagnostic)]
#[diag(lint_function_casts_as_integer)]
struct FunctionCastsAsIntegerMsg<'tcx> {
#[subdiagnostic]
sugg: FunctionCastsAsIntegerSugg<'tcx>,
}

#[derive(Subdiagnostic)]
#[suggestion(
lint_cast_as_fn,
code = " as {cast_from_ty}",
applicability = "machine-applicable",
style = "verbose"
)]
struct FunctionCastsAsIntegerSugg<'tcx> {
#[primary_span]
pub suggestion: Span,
pub cast_from_ty: Ty<'tcx>,
pub cast_to_ty: Ty<'tcx>,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be moved to src/lints.rs where all the other #[derive(LintDiagnostic)] struct lives.


#[derive(LintDiagnostic)]
#[diag(lint_function_casts_as_integer)]
struct FunctionCastsAsIntegerMsg<'tcx> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By convention we name these with the Diag suffix.

Suggested change
struct FunctionCastsAsIntegerMsg<'tcx> {
struct FunctionCastsAsIntegerDiag<'tcx> {

@@ -1,4 +1,3 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated changes?

use rustc_span::Span;
use rustc_span::Symbol;
use rustc_span::{Span, Symbol};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, unrelated changes?

Comment on lines +2 to +3

#![deny(function_casts_as_integer)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better not changing the lint level in it's main test.

Suggested change
#![deny(function_casts_as_integer)]
//@ check-pass


fn foo() {}

fn main() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add tests for enum variants.

enum MyEnum {
    Variant,
}

let _a = MyEnum::Variant as usize; //~ WARN: function_casts_as_integer

@Urgau Urgau added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-unix Operating system: Unix-like S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants