Skip to content
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

Rollup of 7 pull requests #133877

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9d1f790
Add warn-by-default lint against unpredictable fn pointer comparisons
Urgau Nov 9, 2023
5e34c2e
Drop uplifted `clippy::fn_address_comparisons`
Urgau Dec 10, 2023
8ce6357
Allow fn pointers comparisons lint in UI tests
Urgau Dec 10, 2023
7b06fcf
Allow fn pointers comparisons lint in library
Urgau Dec 12, 2023
f613636
Rename `core_pattern_type` and `core_pattern_types` lib feature gate…
oli-obk Dec 4, 2024
1b449e1
Do not emit empty suggestion
estebank Dec 4, 2024
05c34cc
Fix suggestion when shorthand self has erroneous type
compiler-errors Mar 8, 2024
c620505
On `const` pattern errors, point at the `const` item definition
estebank Nov 20, 2024
cc492ed
Tweak unevaluated constant in pattern error
estebank Nov 20, 2024
c0f0008
Tweak ptr in pattern error
estebank Nov 20, 2024
87ddc1e
Point at generic param through which a const is used in a pattern
estebank Nov 20, 2024
253eb95
Tweak output of some const pattern errors
estebank Nov 20, 2024
a6040bc
Specify type kind of constant that can't be used in patterns
estebank Nov 20, 2024
fb2f6a4
Reword message for non-structural type constant in pattern
estebank Nov 20, 2024
335d05a
Add additional context for non-sructural type constant used in pattern
estebank Nov 20, 2024
27a1880
Add context to fall-through "const pattern of non-structural type" error
estebank Nov 20, 2024
d136b31
Add more context to fall-through "const pattern of non-structural typ…
estebank Nov 20, 2024
da58406
fix test
estebank Nov 20, 2024
81291ec
No need to create placeholders for GAT args in confirm_object_candidate
compiler-errors Dec 4, 2024
4e6a401
review comments: reword messages and simplify logic
estebank Dec 4, 2024
03aec5d
fn_sig_for_fn_abi should return a ty::FnSig, no need for a binder
compiler-errors Nov 16, 2024
6ee20ac
Rollup merge of #118833 - Urgau:lint_function_pointer_comparisons, r=…
fmease Dec 4, 2024
81bbc9c
Rollup merge of #122161 - compiler-errors:shorthand-self, r=fmease
fmease Dec 4, 2024
7660ac7
Rollup merge of #133233 - estebank:const-errors, r=Nadrieril
fmease Dec 4, 2024
3e8e092
Rollup merge of #133843 - estebank:empty-semi-sugg, r=jieyouxu
fmease Dec 4, 2024
c4e6438
Rollup merge of #133863 - oli-obk:push-pystoxvtvssx, r=lqd
fmease Dec 4, 2024
9ec5f40
Rollup merge of #133872 - compiler-errors:simplify-gat-check, r=oli-obk
fmease Dec 4, 2024
ccb6845
Rollup merge of #133874 - compiler-errors:fn-sig-binder, r=oli-obk
fmease Dec 4, 2024
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
12 changes: 12 additions & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2566,6 +2566,18 @@ pub enum SelfKind {
Explicit(P<Ty>, Mutability),
}

impl SelfKind {
pub fn to_ref_suggestion(&self) -> String {
match self {
SelfKind::Region(None, mutbl) => mutbl.ref_prefix_str().to_string(),
SelfKind::Region(Some(lt), mutbl) => format!("&{lt} {}", mutbl.prefix_str()),
SelfKind::Value(_) | SelfKind::Explicit(_, _) => {
unreachable!("if we had an explicit self, we wouldn't be here")
}
}
}
}

pub type ExplicitSelf = Spanned<SelfKind>;

impl Param {
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3394,7 +3394,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let Some(ty) = self.node_ty_opt(tail_expr.hir_id) else {
return;
};
if self.can_eq(self.param_env, expected_ty, ty) {
if self.can_eq(self.param_env, expected_ty, ty)
// FIXME: this happens with macro calls. Need to figure out why the stmt
// `println!();` doesn't include the `;` in its `Span`. (#133845)
// We filter these out to avoid ICEs with debug assertions on caused by
// empty suggestions.
&& stmt.span.hi() != tail_expr.span.hi()
{
err.span_suggestion_short(
stmt.span.with_lo(tail_expr.span.hi()),
"remove this semicolon",
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,12 @@ lint_unnameable_test_items = cannot test inner items
lint_unnecessary_qualification = unnecessary qualification
.suggestion = remove the unnecessary path segments

lint_unpredictable_fn_pointer_comparisons = function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique
.note_duplicated_fn = the address of the same function can vary between different codegen units
.note_deduplicated_fn = furthermore, different functions could have the same address after being merged together
.note_visit_fn_addr_eq = for more information visit <https://doc.rust-lang.org/nightly/core/ptr/fn.fn_addr_eq.html>
.fn_addr_eq_suggestion = refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint

lint_unqualified_local_imports = `use` of a local item without leading `self::`, `super::`, or `crate::`

lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
Expand Down
36 changes: 36 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1815,6 +1815,42 @@ pub(crate) enum AmbiguousWidePointerComparisonsAddrSuggestion<'a> {
},
}

#[derive(LintDiagnostic)]
pub(crate) enum UnpredictableFunctionPointerComparisons<'a> {
#[diag(lint_unpredictable_fn_pointer_comparisons)]
#[note(lint_note_duplicated_fn)]
#[note(lint_note_deduplicated_fn)]
#[note(lint_note_visit_fn_addr_eq)]
Suggestion {
#[subdiagnostic]
sugg: UnpredictableFunctionPointerComparisonsSuggestion<'a>,
},
#[diag(lint_unpredictable_fn_pointer_comparisons)]
#[note(lint_note_duplicated_fn)]
#[note(lint_note_deduplicated_fn)]
#[note(lint_note_visit_fn_addr_eq)]
Warn,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(
lint_fn_addr_eq_suggestion,
style = "verbose",
applicability = "maybe-incorrect"
)]
pub(crate) struct UnpredictableFunctionPointerComparisonsSuggestion<'a> {
pub ne: &'a str,
pub cast_right: String,
pub deref_left: &'a str,
pub deref_right: &'a str,
#[suggestion_part(code = "{ne}std::ptr::fn_addr_eq({deref_left}")]
pub left: Span,
#[suggestion_part(code = ", {deref_right}")]
pub middle: Span,
#[suggestion_part(code = "{cast_right})")]
pub right: Span,
}

pub(crate) struct ImproperCTypes<'a> {
pub ty: Ty<'a>,
pub desc: &'a str,
Expand Down
138 changes: 134 additions & 4 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ use crate::lints::{
AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion,
AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad,
AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons,
InvalidNanComparisonsSuggestion, UnusedComparisons, VariantSizeDifferencesDiag,
InvalidNanComparisonsSuggestion, UnpredictableFunctionPointerComparisons,
UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons,
VariantSizeDifferencesDiag,
};
use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};

Expand Down Expand Up @@ -166,6 +168,35 @@ declare_lint! {
"detects ambiguous wide pointer comparisons"
}

declare_lint! {
/// The `unpredictable_function_pointer_comparisons` lint checks comparison
/// of function pointer as the operands.
///
/// ### Example
///
/// ```rust
/// fn a() {}
/// fn b() {}
///
/// let f: fn() = a;
/// let g: fn() = b;
///
/// let _ = f == g;
/// ```
///
/// {{produces}}
///
/// ### 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 functions could have the
/// same address after being merged together.
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS,
Warn,
"detects unpredictable function pointer comparisons"
}

#[derive(Copy, Clone, Default)]
pub(crate) struct TypeLimits {
/// Id of the last visited negated expression
Expand All @@ -178,7 +209,8 @@ impl_lint_pass!(TypeLimits => [
UNUSED_COMPARISONS,
OVERFLOWING_LITERALS,
INVALID_NAN_COMPARISONS,
AMBIGUOUS_WIDE_POINTER_COMPARISONS
AMBIGUOUS_WIDE_POINTER_COMPARISONS,
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS
]);

impl TypeLimits {
Expand Down Expand Up @@ -255,7 +287,7 @@ fn lint_nan<'tcx>(
cx.emit_span_lint(INVALID_NAN_COMPARISONS, e.span, lint);
}

#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Copy, Clone)]
enum ComparisonOp {
BinOp(hir::BinOpKind),
Other,
Expand Down Expand Up @@ -383,6 +415,100 @@ fn lint_wide_pointer<'tcx>(
);
}

fn lint_fn_pointer<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx hir::Expr<'tcx>,
cmpop: ComparisonOp,
l: &'tcx hir::Expr<'tcx>,
r: &'tcx hir::Expr<'tcx>,
) {
let peel_refs = |mut ty: Ty<'tcx>| -> (Ty<'tcx>, usize) {
let mut refs = 0;

while let ty::Ref(_, inner_ty, _) = ty.kind() {
ty = *inner_ty;
refs += 1;
}

(ty, refs)
};

// Left and right operands can have borrows, remove them
let l = l.peel_borrows();
let r = r.peel_borrows();

let Some(l_ty) = cx.typeck_results().expr_ty_opt(l) else { return };
let Some(r_ty) = cx.typeck_results().expr_ty_opt(r) else { return };

// Remove any references as `==` will deref through them (and count the
// number of references removed, for latter).
let (l_ty, l_ty_refs) = peel_refs(l_ty);
let (r_ty, r_ty_refs) = peel_refs(r_ty);

if !l_ty.is_fn() || !r_ty.is_fn() {
return;
}

// Let's try to suggest `ptr::fn_addr_eq` if/when possible.

let is_eq_ne = matches!(cmpop, ComparisonOp::BinOp(hir::BinOpKind::Eq | hir::BinOpKind::Ne));

if !is_eq_ne {
// Neither `==` nor `!=`, we can't suggest `ptr::fn_addr_eq`, just show the warning.
return cx.emit_span_lint(
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS,
e.span,
UnpredictableFunctionPointerComparisons::Warn,
);
}

let (Some(l_span), Some(r_span)) =
(l.span.find_ancestor_inside(e.span), r.span.find_ancestor_inside(e.span))
else {
// No appropriate spans for the left and right operands, just show the warning.
return cx.emit_span_lint(
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS,
e.span,
UnpredictableFunctionPointerComparisons::Warn,
);
};

let ne = if cmpop == ComparisonOp::BinOp(hir::BinOpKind::Ne) { "!" } else { "" };

// `ptr::fn_addr_eq` only works with raw pointer, deref any references.
let deref_left = &*"*".repeat(l_ty_refs);
let deref_right = &*"*".repeat(r_ty_refs);

let left = e.span.shrink_to_lo().until(l_span.shrink_to_lo());
let middle = l_span.shrink_to_hi().until(r_span.shrink_to_lo());
let right = r_span.shrink_to_hi().until(e.span.shrink_to_hi());

// We only check for a right cast as `FnDef` == `FnPtr` is not possible,
// only `FnPtr == FnDef` is possible.
let cast_right = if !r_ty.is_fn_ptr() {
let fn_sig = r_ty.fn_sig(cx.tcx);
format!(" as {fn_sig}")
} else {
String::new()
};

cx.emit_span_lint(
UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS,
e.span,
UnpredictableFunctionPointerComparisons::Suggestion {
sugg: UnpredictableFunctionPointerComparisonsSuggestion {
ne,
deref_left,
deref_right,
left,
middle,
right,
cast_right,
},
},
);
}

impl<'tcx> LateLintPass<'tcx> for TypeLimits {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) {
match e.kind {
Expand All @@ -399,7 +525,9 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
cx.emit_span_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons);
} else {
lint_nan(cx, e, binop, l, r);
lint_wide_pointer(cx, e, ComparisonOp::BinOp(binop.node), l, r);
let cmpop = ComparisonOp::BinOp(binop.node);
lint_wide_pointer(cx, e, cmpop, l, r);
lint_fn_pointer(cx, e, cmpop, l, r);
}
}
}
Expand All @@ -411,13 +539,15 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
&& let Some(cmpop) = diag_item_cmpop(diag_item) =>
{
lint_wide_pointer(cx, e, cmpop, l, r);
lint_fn_pointer(cx, e, cmpop, l, r);
}
hir::ExprKind::MethodCall(_, l, [r], _)
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
&& let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
&& let Some(cmpop) = diag_item_cmpop(diag_item) =>
{
lint_wide_pointer(cx, e, cmpop, l, r);
lint_fn_pointer(cx, e, cmpop, l, r);
}
_ => {}
};
Expand Down
37 changes: 25 additions & 12 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,17 @@ mir_build_call_to_unsafe_fn_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =

mir_build_confused = missing patterns are not covered because `{$variable}` is interpreted as a constant pattern, not a new variable

mir_build_const_param_in_pattern = const parameters cannot be referenced in patterns
mir_build_const_defined_here = constant defined here

mir_build_const_pattern_depends_on_generic_parameter =
constant pattern depends on a generic parameter
mir_build_const_param_in_pattern = constant parameters cannot be referenced in patterns
.label = can't be used in patterns
mir_build_const_param_in_pattern_def = constant defined here

mir_build_const_pattern_depends_on_generic_parameter = constant pattern cannot depend on generic parameters
.label = `const` depends on a generic parameter

mir_build_could_not_eval_const_pattern = could not evaluate constant pattern
.label = could not evaluate constant

mir_build_deref_raw_pointer_requires_unsafe =
dereference of raw pointer is unsafe and requires unsafe block
Expand Down Expand Up @@ -147,7 +152,8 @@ mir_build_inline_assembly_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =

mir_build_interpreted_as_const = introduce a variable instead

mir_build_invalid_pattern = `{$non_sm_ty}` cannot be used in patterns
mir_build_invalid_pattern = {$prefix} `{$non_sm_ty}` cannot be used in patterns
.label = {$prefix} can't be used in patterns

mir_build_irrefutable_let_patterns_if_let = irrefutable `if let` {$count ->
[one] pattern
Expand Down Expand Up @@ -244,10 +250,12 @@ mir_build_mutation_of_layout_constrained_field_requires_unsafe_unsafe_op_in_unsa
.label = mutation of layout constrained field

mir_build_nan_pattern = cannot use NaN in patterns
.label = evaluates to `NaN`, which is not allowed in patterns
.note = NaNs compare inequal to everything, even themselves, so this pattern would never match
.help = try using the `is_nan` method instead

mir_build_non_const_path = runtime values cannot be referenced in patterns
.label = references a runtime value

mir_build_non_empty_never_pattern =
mismatched types
Expand All @@ -265,13 +273,15 @@ mir_build_non_exhaustive_patterns_type_not_empty = non-exhaustive patterns: type
.suggestion = ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
.help = ensure that all possible cases are being handled by adding a match arm with a wildcard pattern

mir_build_non_partial_eq_match =
to use a constant of type `{$non_peq_ty}` in a pattern, the type must implement `PartialEq`
mir_build_non_partial_eq_match = constant of non-structural type `{$ty}` in a pattern
.label = constant of non-structural type

mir_build_pattern_not_covered = refutable pattern in {$origin}
.pattern_ty = the matched value is of type `{$pattern_ty}`

mir_build_pointer_pattern = function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
mir_build_pointer_pattern = function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon
.label = can't be used in patterns
.note = see https://github.com/rust-lang/rust/issues/70861 for details

mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabited, but this variant contains private fields which may become inhabited in the future

Expand All @@ -283,6 +293,8 @@ mir_build_rustc_box_attribute_error = `#[rustc_box]` attribute used incorrectly
.missing_box = `#[rustc_box]` requires the `owned_box` lang item

mir_build_static_in_pattern = statics cannot be referenced in patterns
.label = can't be used in patterns
mir_build_static_in_pattern_def = `static` defined here

mir_build_suggest_attempted_int_lit = alternatively, you could prepend the pattern with an underscore to define a new named variable; identifiers cannot begin with digits

Expand Down Expand Up @@ -310,12 +322,12 @@ mir_build_trailing_irrefutable_let_patterns = trailing irrefutable {$count ->
*[other] them
} into the body

mir_build_type_not_structural =
to use a constant of type `{$non_sm_ty}` in a pattern, `{$non_sm_ty}` must be annotated with `#[derive(PartialEq)]`

mir_build_type_not_structural = constant of non-structural type `{$ty}` in a pattern
.label = constant of non-structural type
mir_build_type_not_structural_def = `{$ty}` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
mir_build_type_not_structural_more_info = see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details

mir_build_type_not_structural_tip = the traits must be derived, manual `impl`s are not sufficient
mir_build_type_not_structural_tip =
the `PartialEq` trait must be derived, manual `impl`s are not sufficient; see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details

mir_build_unconditional_recursion = function cannot return without recursing
.label = cannot return without recursing
Expand All @@ -334,6 +346,7 @@ mir_build_union_field_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
.label = access to union field

mir_build_union_pattern = cannot use unions in constant patterns
.label = can't use a `union` here

mir_build_unreachable_making_this_unreachable = collectively making this unreachable

Expand Down
Loading
Loading