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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_driver_impl/src/signal_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ pub(super) fn install() {
libc::sigaltstack(&alt_stack, ptr::null_mut());

let mut sa: libc::sigaction = mem::zeroed();
sa.sa_sigaction = print_stack_trace as libc::sighandler_t;
sa.sa_sigaction =
print_stack_trace as unsafe extern "C" fn(libc::c_int) as libc::sighandler_t;
sa.sa_flags = libc::SA_NODEFER | libc::SA_RESETHAND | libc::SA_ONSTACK;
libc::sigemptyset(&mut sa.sa_mask);
for (signum, _signame) in KILL_SIGNALS {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@ lint_forgetting_copy_types = calls to `std::mem::forget` with a value that imple
lint_forgetting_references = calls to `std::mem::forget` with a reference instead of an owned value does nothing
.label = argument has type `{$arg_ty}`

lint_function_casts_as_integer = direct cast of function item into an integer
.cast_as_fn = first cast to a function pointer `{$cast_from_ty}`

lint_hidden_glob_reexport = private item shadows public glob re-export
.note_glob_reexport = the name `{$name}` in the {$namespace} namespace is supposed to be publicly re-exported here
.note_private_item = but the private item here shadows it
Expand Down
62 changes: 62 additions & 0 deletions compiler/rustc_lint/src/function_cast_as_integer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use rustc_hir as hir;
use rustc_middle::ty;
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::BytePos;

use crate::lints::{FunctionCastsAsIntegerDiag, FunctionCastsAsIntegerSugg};
use crate::{LateContext, LateLintPass};

declare_lint! {
/// The `function_casts_as_integer` lint detects cases where a function item is casted
/// into an integer.
///
/// ### Example
///
/// ```rust
/// fn foo() {}
/// let x = foo as usize;
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// When casting a function item into an integer, it's implicitly creating a
/// function pointer that will in turn be casted into an integer. By making
/// it explicit, it improves readability of the code and prevents bugs.
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.

Copy link
Member

Choose a reason for hiding this comment

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

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

"casting a function into an integer",
}

declare_lint_pass!(
/// Lint for casts of functions into integers.
FunctionCastsAsInteger => [FUNCTION_CASTS_AS_INTEGER]
);

impl<'tcx> LateLintPass<'tcx> for FunctionCastsAsInteger {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
let hir::ExprKind::Cast(cast_from_expr, cast_to_expr) = expr.kind else { return };
let cast_to_ty = cx.typeck_results().expr_ty(expr);
// Casting to a function (pointer?), so all good.
if matches!(cast_to_ty.kind(), ty::FnDef(..) | ty::FnPtr(..)) {
return;
}
let cast_from_ty = cx.typeck_results().expr_ty(cast_from_expr);
if matches!(cast_from_ty.kind(), ty::FnDef(..)) {
cx.tcx.emit_node_span_lint(
FUNCTION_CASTS_AS_INTEGER,
expr.hir_id,
cast_to_expr.span.with_lo(cast_from_expr.span.hi() + BytePos(1)),
FunctionCastsAsIntegerDiag {
sugg: FunctionCastsAsIntegerSugg {
suggestion: cast_from_expr.span.shrink_to_hi(),
// We get the function pointer to have a nice display.
cast_from_ty: cx.typeck_results().expr_ty_adjusted(cast_from_expr),
cast_to_ty,
},
},
);
}
}
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ mod errors;
mod expect;
mod for_loops_over_fallibles;
mod foreign_modules;
mod function_cast_as_integer;
mod if_let_rescope;
mod impl_trait_overcaptures;
mod internal;
Expand Down Expand Up @@ -92,6 +93,7 @@ use deref_into_dyn_supertrait::*;
use drop_forget_useless::*;
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
use for_loops_over_fallibles::*;
use function_cast_as_integer::*;
use if_let_rescope::IfLetRescope;
use impl_trait_overcaptures::ImplTraitOvercaptures;
use internal::*;
Expand Down Expand Up @@ -247,6 +249,7 @@ late_lint_methods!(
IfLetRescope: IfLetRescope::default(),
StaticMutRefs: StaticMutRefs,
UnqualifiedLocalImports: UnqualifiedLocalImports,
FunctionCastsAsInteger: FunctionCastsAsInteger,
CheckTransmutes: CheckTransmutes,
LifetimeSyntax: LifetimeSyntax,
]
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3190,6 +3190,27 @@ pub(crate) struct ReservedMultihash {
pub suggestion: Span,
}

#[derive(LintDiagnostic)]
#[diag(lint_function_casts_as_integer)]
pub(crate) struct FunctionCastsAsIntegerDiag<'tcx> {
#[subdiagnostic]
pub(crate) sugg: FunctionCastsAsIntegerSugg<'tcx>,
}

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

#[derive(Debug)]
pub(crate) struct MismatchedLifetimeSyntaxes {
pub lifetime_name: String,
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_session/src/filesearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fn current_dll_path() -> Result<PathBuf, String> {

#[cfg(not(target_os = "aix"))]
unsafe {
let addr = current_dll_path as usize as *mut _;
let addr = current_dll_path as fn() -> Result<PathBuf, String> as *mut _;
let mut info = std::mem::zeroed();
if libc::dladdr(addr, &mut info) == 0 {
return Err("dladdr failed".into());
Expand Down Expand Up @@ -152,7 +152,10 @@ fn current_dll_path() -> Result<PathBuf, String> {
unsafe {
GetModuleHandleExW(
GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
PCWSTR(current_dll_path as *mut u16),
PCWSTR(
current_dll_path as fn() -> Result<std::path::PathBuf, std::string::String>
as *mut u16,
),
&mut module,
)
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,7 @@ pub const fn forget<T: ?Sized>(_: T);
/// // Crucially, we `as`-cast to a raw pointer before `transmute`ing to a function pointer.
/// // This avoids an integer-to-pointer `transmute`, which can be problematic.
/// // Transmuting between raw pointers and function pointers (i.e., two pointer types) is fine.
/// let pointer = foo as *const ();
/// let pointer = foo as fn() -> i32 as *const ();
/// let function = unsafe {
/// std::mem::transmute::<*const (), fn() -> i32>(pointer)
/// };
Expand Down
1 change: 1 addition & 0 deletions library/coretests/tests/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ fn ptr_metadata() {

#[test]
fn ptr_metadata_bounds() {
#[allow(unknown_lints, function_casts_as_integer)]
fn metadata_eq_method_address<T: ?Sized>() -> usize {
// The `Metadata` associated type has an `Ord` bound, so this is valid:
<<T as Pointee>::Metadata as PartialEq>::eq as usize
Expand Down
2 changes: 2 additions & 0 deletions library/panic_unwind/src/seh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ pub(crate) unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
// In any case, we basically need to do something like this until we can
// express more operations in statics (and we may never be able to).
unsafe {
#[allow(function_casts_as_integer)]
atomic_store_seqcst(
(&raw mut THROW_INFO.pmfnUnwind).cast(),
ptr_t::new(exception_cleanup as *mut u8).raw(),
Expand All @@ -341,6 +342,7 @@ pub(crate) unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
(&raw mut CATCHABLE_TYPE.pType).cast(),
ptr_t::new((&raw mut TYPE_DESCRIPTOR).cast()).raw(),
);
#[allow(function_casts_as_integer)]
atomic_store_seqcst(
(&raw mut CATCHABLE_TYPE.copyFunction).cast(),
ptr_t::new(exception_copy as *mut u8).raw(),
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ impl Backtrace {
if !Backtrace::enabled() {
return Backtrace { inner: Inner::Disabled };
}
Backtrace::create(Backtrace::capture as usize)
Backtrace::create(Backtrace::capture as fn() -> Backtrace as usize)
}

/// Forcibly captures a full backtrace, regardless of environment variable
Expand All @@ -309,7 +309,7 @@ impl Backtrace {
#[stable(feature = "backtrace", since = "1.65.0")]
#[inline(never)] // want to make sure there's a frame here to remove
pub fn force_capture() -> Backtrace {
Backtrace::create(Backtrace::force_capture as usize)
Backtrace::create(Backtrace::force_capture as fn() -> Backtrace as usize)
}

/// Forcibly captures a disabled backtrace, regardless of environment
Expand Down
4 changes: 3 additions & 1 deletion library/std/src/sys/pal/unix/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ mod imp {
}

action.sa_flags = SA_SIGINFO | SA_ONSTACK;
action.sa_sigaction = signal_handler as sighandler_t;
action.sa_sigaction = signal_handler
as unsafe extern "C" fn(i32, *mut libc::siginfo_t, *mut libc::c_void)
as sighandler_t;
// SAFETY: only overriding signals if the default is set
unsafe { sigaction(signal, &action, ptr::null_mut()) };
}
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/windows/compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ macro_rules! compat_fn_with_fallback {
/// When that is called it attempts to load the requested symbol.
/// If it succeeds, `PTR` is set to the address of that symbol.
/// If it fails, then `PTR` is set to `fallback`.
static PTR: Atomic<*mut c_void> = AtomicPtr::new(load as *mut _);
static PTR: Atomic<*mut c_void> = AtomicPtr::new(load as unsafe extern "system" fn($($argname: $argtype),*) -> $rettype as *mut _);

unsafe extern "system" fn load($($argname: $argtype),*) -> $rettype {
unsafe {
Expand All @@ -171,7 +171,7 @@ macro_rules! compat_fn_with_fallback {
PTR.store(f.as_ptr(), Ordering::Relaxed);
mem::transmute(f)
} else {
PTR.store(fallback as *mut _, Ordering::Relaxed);
PTR.store(fallback as unsafe extern "system" fn($($argname: $argtype),*) -> $rettype as *mut _, Ordering::Relaxed);
fallback
}
}
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/clippy_lints/src/disallowed_macros.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

use clippy_config::Conf;
use clippy_config::types::{DisallowedPath, create_disallowed_map};
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/tests/ui/cast_enum_constructor.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::cast_enum_constructor)]
#![allow(clippy::fn_to_numeric_cast)]
#![allow(clippy::fn_to_numeric_cast, function_casts_as_integer)]

fn main() {
enum Foo {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![feature(float_minimum_maximum)]
#![warn(clippy::confusing_method_to_numeric_cast)]
#![allow(function_casts_as_integer)]

fn main() {
let _ = u16::MAX as usize; //~ confusing_method_to_numeric_cast
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![feature(float_minimum_maximum)]
#![warn(clippy::confusing_method_to_numeric_cast)]
#![allow(function_casts_as_integer)]

fn main() {
let _ = u16::max as usize; //~ confusing_method_to_numeric_cast
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: casting function pointer `u16::max` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:5:13
--> tests/ui/confusing_method_to_numeric_cast.rs:6:13
|
LL | let _ = u16::max as usize;
| ^^^^^^^^^^^^^^^^^
Expand All @@ -13,7 +13,7 @@ LL + let _ = u16::MAX as usize;
|

error: casting function pointer `u16::min` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:6:13
--> tests/ui/confusing_method_to_numeric_cast.rs:7:13
|
LL | let _ = u16::min as usize;
| ^^^^^^^^^^^^^^^^^
Expand All @@ -25,7 +25,7 @@ LL + let _ = u16::MIN as usize;
|

error: casting function pointer `u16::max_value` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:7:13
--> tests/ui/confusing_method_to_numeric_cast.rs:8:13
|
LL | let _ = u16::max_value as usize;
| ^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -37,7 +37,7 @@ LL + let _ = u16::MAX as usize;
|

error: casting function pointer `u16::min_value` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:8:13
--> tests/ui/confusing_method_to_numeric_cast.rs:9:13
|
LL | let _ = u16::min_value as usize;
| ^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -49,7 +49,7 @@ LL + let _ = u16::MIN as usize;
|

error: casting function pointer `f32::maximum` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:10:13
--> tests/ui/confusing_method_to_numeric_cast.rs:11:13
|
LL | let _ = f32::maximum as usize;
| ^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -61,7 +61,7 @@ LL + let _ = f32::MAX as usize;
|

error: casting function pointer `f32::max` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:11:13
--> tests/ui/confusing_method_to_numeric_cast.rs:12:13
|
LL | let _ = f32::max as usize;
| ^^^^^^^^^^^^^^^^^
Expand All @@ -73,7 +73,7 @@ LL + let _ = f32::MAX as usize;
|

error: casting function pointer `f32::minimum` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:12:13
--> tests/ui/confusing_method_to_numeric_cast.rs:13:13
|
LL | let _ = f32::minimum as usize;
| ^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -85,7 +85,7 @@ LL + let _ = f32::MIN as usize;
|

error: casting function pointer `f32::min` to `usize`
--> tests/ui/confusing_method_to_numeric_cast.rs:13:13
--> tests/ui/confusing_method_to_numeric_cast.rs:14:13
|
LL | let _ = f32::min as usize;
| ^^^^^^^^^^^^^^^^^
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/tests/ui/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#![expect(incomplete_features)] // `unsafe_fields` is incomplete for the time being
#![feature(unsafe_fields)] // `clone()` cannot be derived automatically on unsafe fields


#[derive(Copy)]
struct Qux;

Expand Down
Loading
Loading