Skip to content

Commit 2959657

Browse files
committed
Auto merge of rust-lang#118833 - Urgau:lint_function_pointer_comparisons, r=<try>
Add lint against function pointer comparisons This is kind of a follow-up to rust-lang#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
2 parents c3def26 + 6c5c546 commit 2959657

26 files changed

+260
-163
lines changed

compiler/rustc_lint/messages.ftl

+4
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,10 @@ lint_unknown_lint =
538538
lint_unknown_tool_in_scoped_lint = unknown tool name `{$tool_name}` found in scoped lint: `{$tool_name}::{$lint_name}`
539539
.help = add `#![register_tool({$tool_name})]` to the crate root
540540
541+
lint_unpredictable_fn_pointer_comparisons = function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique
542+
.note_duplicated_fn = the address of the same function can very between different codegen units
543+
.note_deduplicated_fn = furthermore, different functions could have the same address after being merged together
544+
541545
lint_unsupported_group = `{$lint_group}` lint group is not supported with ´--force-warn´
542546
543547
lint_untranslatable_diag = diagnostics should be created using translatable messages

compiler/rustc_lint/src/lints.rs

+6
Original file line numberDiff line numberDiff line change
@@ -1644,6 +1644,12 @@ pub enum AmbiguousWidePointerComparisonsAddrSuggestion<'a> {
16441644
},
16451645
}
16461646

1647+
#[derive(LintDiagnostic)]
1648+
#[diag(lint_unpredictable_fn_pointer_comparisons)]
1649+
#[note(lint_note_duplicated_fn)]
1650+
#[note(lint_note_deduplicated_fn)]
1651+
pub struct UnpredictableFunctionPointerComparisons;
1652+
16471653
pub struct ImproperCTypes<'a> {
16481654
pub ty: Ty<'a>,
16491655
pub desc: &'a str,

compiler/rustc_lint/src/types.rs

+57-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use crate::{
77
InvalidNanComparisonsSuggestion, OnlyCastu8ToChar, OverflowingBinHex,
88
OverflowingBinHexSign, OverflowingBinHexSignBitSub, OverflowingBinHexSub, OverflowingInt,
99
OverflowingIntHelp, OverflowingLiteral, OverflowingUInt, RangeEndpointOutOfRange,
10-
UnusedComparisons, UseInclusiveRange, VariantSizeDifferencesDiag,
10+
UnpredictableFunctionPointerComparisons, UnusedComparisons, UseInclusiveRange,
11+
VariantSizeDifferencesDiag,
1112
},
1213
};
1314
use crate::{LateContext, LateLintPass, LintContext};
@@ -169,6 +170,32 @@ declare_lint! {
169170
"detects ambiguous wide pointer comparisons"
170171
}
171172

173+
declare_lint! {
174+
/// The `unpredictable_function_pointer_comparisons` lint checks comparison
175+
/// of function pointer as the operands.
176+
///
177+
/// ### Example
178+
///
179+
/// ```rust
180+
/// fn foo() {}
181+
/// # let a = foo as fn();
182+
///
183+
/// let _ = a == foo;
184+
/// ```
185+
///
186+
/// {{produces}}
187+
///
188+
/// ### Explanation
189+
///
190+
/// Function pointers comparisons do not produce meaningful result since
191+
/// they are never guaranteed to be unique and could vary between different
192+
/// code generation units. Furthermore, different functions could have the
193+
/// same address after being merged together.
194+
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS,
195+
Deny,
196+
"detects unpredictable function pointer comparisons"
197+
}
198+
172199
#[derive(Copy, Clone)]
173200
pub struct TypeLimits {
174201
/// Id of the last visited negated expression
@@ -181,7 +208,8 @@ impl_lint_pass!(TypeLimits => [
181208
UNUSED_COMPARISONS,
182209
OVERFLOWING_LITERALS,
183210
INVALID_NAN_COMPARISONS,
184-
AMBIGUOUS_WIDE_POINTER_COMPARISONS
211+
AMBIGUOUS_WIDE_POINTER_COMPARISONS,
212+
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS
185213
]);
186214

187215
impl TypeLimits {
@@ -758,6 +786,30 @@ fn lint_wide_pointer<'tcx>(
758786
);
759787
}
760788

789+
fn lint_fn_pointer<'tcx>(
790+
cx: &LateContext<'tcx>,
791+
e: &'tcx hir::Expr<'tcx>,
792+
_binop: hir::BinOpKind,
793+
l: &'tcx hir::Expr<'tcx>,
794+
r: &'tcx hir::Expr<'tcx>,
795+
) {
796+
let Some(l_ty) = cx.typeck_results().expr_ty_opt(l) else { return };
797+
let Some(r_ty) = cx.typeck_results().expr_ty_opt(r) else { return };
798+
799+
let l_ty = l_ty.peel_refs();
800+
let r_ty = r_ty.peel_refs();
801+
802+
if !l_ty.is_fn() || !r_ty.is_fn() {
803+
return;
804+
}
805+
806+
cx.emit_spanned_lint(
807+
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS,
808+
e.span,
809+
UnpredictableFunctionPointerComparisons,
810+
);
811+
}
812+
761813
impl<'tcx> LateLintPass<'tcx> for TypeLimits {
762814
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) {
763815
match e.kind {
@@ -775,6 +827,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
775827
} else {
776828
lint_nan(cx, e, binop, l, r);
777829
lint_wide_pointer(cx, e, binop.node, l, r);
830+
lint_fn_pointer(cx, e, binop.node, l, r);
778831
}
779832
}
780833
}
@@ -786,13 +839,15 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
786839
&& let Some(binop) = partialeq_binop(diag_item) =>
787840
{
788841
lint_wide_pointer(cx, e, binop, l, r);
842+
lint_fn_pointer(cx, e, binop, l, r);
789843
}
790844
hir::ExprKind::MethodCall(_, l, [r], _)
791845
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
792846
&& let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
793847
&& let Some(binop) = partialeq_binop(diag_item) =>
794848
{
795849
lint_wide_pointer(cx, e, binop, l, r);
850+
lint_fn_pointer(cx, e, binop, l, r);
796851
}
797852
_ => {}
798853
};

library/core/tests/ptr.rs

+1
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ fn test_const_nonnull_new() {
303303
#[test]
304304
#[cfg(unix)] // printf may not be available on other platforms
305305
#[allow(deprecated)] // For SipHasher
306+
#[cfg_attr(not(bootstrap), allow(unpredictable_function_pointer_comparisons))]
306307
pub fn test_variadic_fnptr() {
307308
use core::ffi;
308309
use core::hash::{Hash, SipHasher};

src/tools/clippy/clippy_lints/src/declared_lints.rs

-1
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
683683
crate::unit_types::LET_UNIT_VALUE_INFO,
684684
crate::unit_types::UNIT_ARG_INFO,
685685
crate::unit_types::UNIT_CMP_INFO,
686-
crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
687686
crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO,
688687
crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
689688
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,

src/tools/clippy/clippy_lints/src/lib.rs

-2
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ mod unicode;
328328
mod uninit_vec;
329329
mod unit_return_expecting_ord;
330330
mod unit_types;
331-
mod unnamed_address;
332331
mod unnecessary_box_returns;
333332
mod unnecessary_map_on_constructor;
334333
mod unnecessary_owned_empty_strings;
@@ -862,7 +861,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
862861
store.register_early_pass(|| Box::new(option_env_unwrap::OptionEnvUnwrap));
863862
store.register_late_pass(move |_| Box::new(wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports)));
864863
store.register_late_pass(|_| Box::<redundant_pub_crate::RedundantPubCrate>::default());
865-
store.register_late_pass(|_| Box::new(unnamed_address::UnnamedAddress));
866864
store.register_late_pass(|_| Box::<dereference::Dereferencing<'_>>::default());
867865
store.register_late_pass(|_| Box::new(option_if_let_else::OptionIfLetElse));
868866
store.register_late_pass(|_| Box::new(future_not_send::FutureNotSend));

src/tools/clippy/clippy_lints/src/renamed_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,5 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
5959
("clippy::unknown_clippy_lints", "unknown_lints"),
6060
("clippy::unused_label", "unused_labels"),
6161
("clippy::vtable_address_comparisons", "ambiguous_wide_pointer_comparisons"),
62+
("clippy::fn_address_comparisons", "unpredictable_function_pointer_comparisons"),
6263
];

src/tools/clippy/clippy_lints/src/unnamed_address.rs

-60
This file was deleted.

src/tools/clippy/tests/ui/fn_address_comparisons.rs

-23
This file was deleted.

src/tools/clippy/tests/ui/fn_address_comparisons.stderr

-17
This file was deleted.

src/tools/clippy/tests/ui/rename.fixed

+2
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
#![allow(unknown_lints)]
5353
#![allow(unused_labels)]
5454
#![allow(ambiguous_wide_pointer_comparisons)]
55+
#![allow(unpredictable_function_pointer_comparisons)]
5556
#![warn(clippy::almost_complete_range)]
5657
#![warn(clippy::disallowed_names)]
5758
#![warn(clippy::blocks_in_if_conditions)]
@@ -109,5 +110,6 @@
109110
#![warn(unknown_lints)]
110111
#![warn(unused_labels)]
111112
#![warn(ambiguous_wide_pointer_comparisons)]
113+
#![warn(unpredictable_function_pointer_comparisons)]
112114

113115
fn main() {}

src/tools/clippy/tests/ui/rename.rs

+2
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
#![allow(unknown_lints)]
5353
#![allow(unused_labels)]
5454
#![allow(ambiguous_wide_pointer_comparisons)]
55+
#![allow(unpredictable_function_pointer_comparisons)]
5556
#![warn(clippy::almost_complete_letter_range)]
5657
#![warn(clippy::blacklisted_name)]
5758
#![warn(clippy::block_in_if_condition_expr)]
@@ -109,5 +110,6 @@
109110
#![warn(clippy::unknown_clippy_lints)]
110111
#![warn(clippy::unused_label)]
111112
#![warn(clippy::vtable_address_comparisons)]
113+
#![warn(clippy::fn_address_comparisons)]
112114

113115
fn main() {}

0 commit comments

Comments
 (0)