Skip to content

Commit

Permalink
Rollup merge of #118833 - Urgau:lint_function_pointer_comparisons, r=…
Browse files Browse the repository at this point in the history
…cjgillot

Add lint against function pointer comparisons

This is kind of a follow-up to rust-lang/rust#117758 where we added a lint against wide pointer comparisons for being ambiguous and unreliable; well function pointer comparisons are also unreliable. We should IMO follow a similar logic and warn people about it.

-----

## `unpredictable_function_pointer_comparisons`

*warn-by-default*

The `unpredictable_function_pointer_comparisons` lint checks comparison of function pointer as the operands.

### Example

```rust
fn foo() {}
let a = foo as fn();

let _ = a == foo;
```

### Explanation

Function pointers comparisons do not produce meaningful result since they are never guaranteed to be unique and could vary between different code generation units. Furthermore different function could have the same address after being merged together.

----

This PR also uplift the very similar `clippy::fn_address_comparisons` lint, which only linted on if one of the operand was an `ty::FnDef` while this PR lints proposes to lint on all `ty::FnPtr` and `ty::FnDef`.

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

~~Edit: Blocked on rust-lang/libs-team#323 being accepted and it's follow-up pr~~
  • Loading branch information
fmease authored Dec 5, 2024
2 parents f850d15 + c8d800e commit 7cc19d7
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 170 deletions.
1 change: 0 additions & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::unit_types::LET_UNIT_VALUE_INFO,
crate::unit_types::UNIT_ARG_INFO,
crate::unit_types::UNIT_CMP_INFO,
crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO,
crate::unnecessary_literal_bound::UNNECESSARY_LITERAL_BOUND_INFO,
crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ declare_with_version! { RENAMED(RENAMED_VERSION): &[(&str, &str)] = &[
#[clippy::version = "1.53.0"]
("clippy::filter_map", "clippy::manual_filter_map"),
#[clippy::version = ""]
("clippy::fn_address_comparisons", "unpredictable_function_pointer_comparisons"),
#[clippy::version = ""]
("clippy::identity_conversion", "clippy::useless_conversion"),
#[clippy::version = "pre 1.29.0"]
("clippy::if_let_redundant_pattern_matching", "clippy::redundant_pattern_matching"),
Expand Down
2 changes: 0 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,6 @@ mod uninhabited_references;
mod uninit_vec;
mod unit_return_expecting_ord;
mod unit_types;
mod unnamed_address;
mod unnecessary_box_returns;
mod unnecessary_literal_bound;
mod unnecessary_map_on_constructor;
Expand Down Expand Up @@ -786,7 +785,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_early_pass(|| Box::new(option_env_unwrap::OptionEnvUnwrap));
store.register_late_pass(move |_| Box::new(wildcard_imports::WildcardImports::new(conf)));
store.register_late_pass(|_| Box::<redundant_pub_crate::RedundantPubCrate>::default());
store.register_late_pass(|_| Box::new(unnamed_address::UnnamedAddress));
store.register_late_pass(|_| Box::<dereference::Dereferencing<'_>>::default());
store.register_late_pass(|_| Box::new(option_if_let_else::OptionIfLetElse));
store.register_late_pass(|_| Box::new(future_not_send::FutureNotSend));
Expand Down
60 changes: 0 additions & 60 deletions clippy_lints/src/unnamed_address.rs

This file was deleted.

23 changes: 0 additions & 23 deletions tests/ui/fn_address_comparisons.rs

This file was deleted.

17 changes: 0 additions & 17 deletions tests/ui/fn_address_comparisons.stderr

This file was deleted.

2 changes: 2 additions & 0 deletions tests/ui/rename.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![allow(ambiguous_wide_pointer_comparisons)]
#![allow(unpredictable_function_pointer_comparisons)]
#![allow(clippy::reversed_empty_ranges)]
#![warn(clippy::almost_complete_range)] //~ ERROR: lint `clippy::almost_complete_letter_range`
#![warn(clippy::disallowed_names)] //~ ERROR: lint `clippy::blacklisted_name`
Expand All @@ -74,6 +75,7 @@
#![warn(clippy::mixed_read_write_in_expression)] //~ ERROR: lint `clippy::eval_order_dependence`
#![warn(clippy::manual_find_map)] //~ ERROR: lint `clippy::find_map`
#![warn(clippy::manual_filter_map)] //~ ERROR: lint `clippy::filter_map`
#![warn(unpredictable_function_pointer_comparisons)] //~ ERROR: lint `clippy::fn_address_comparisons`
#![warn(clippy::useless_conversion)] //~ ERROR: lint `clippy::identity_conversion`
#![warn(clippy::redundant_pattern_matching)] //~ ERROR: lint `clippy::if_let_redundant_pattern_matching`
#![warn(clippy::match_result_ok)] //~ ERROR: lint `clippy::if_let_some_result`
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![allow(ambiguous_wide_pointer_comparisons)]
#![allow(unpredictable_function_pointer_comparisons)]
#![allow(clippy::reversed_empty_ranges)]
#![warn(clippy::almost_complete_letter_range)] //~ ERROR: lint `clippy::almost_complete_letter_range`
#![warn(clippy::blacklisted_name)] //~ ERROR: lint `clippy::blacklisted_name`
Expand All @@ -74,6 +75,7 @@
#![warn(clippy::eval_order_dependence)] //~ ERROR: lint `clippy::eval_order_dependence`
#![warn(clippy::find_map)] //~ ERROR: lint `clippy::find_map`
#![warn(clippy::filter_map)] //~ ERROR: lint `clippy::filter_map`
#![warn(clippy::fn_address_comparisons)] //~ ERROR: lint `clippy::fn_address_comparisons`
#![warn(clippy::identity_conversion)] //~ ERROR: lint `clippy::identity_conversion`
#![warn(clippy::if_let_redundant_pattern_matching)] //~ ERROR: lint `clippy::if_let_redundant_pattern_matching`
#![warn(clippy::if_let_some_result)] //~ ERROR: lint `clippy::if_let_some_result`
Expand Down
Loading

0 comments on commit 7cc19d7

Please sign in to comment.