Skip to content

Commit

Permalink
Auto merge of #133877 - fmease:rollup-909elhb, r=fmease
Browse files Browse the repository at this point in the history
Rollup of 7 pull requests

Successful merges:

 - #118833 (Add lint against function pointer comparisons)
 - #122161 (Fix suggestion when shorthand `self` has erroneous type)
 - #133233 (Add context to "const in pattern" errors)
 - #133843 (Do not emit empty suggestion)
 - #133863 (Rename `core_pattern_type` and `core_pattern_types` lib feature  gates to `pattern_type_macro`)
 - #133872 (No need to create placeholders for GAT args in confirm_object_candidate)
 - #133874 (`fn_sig_for_fn_abi` should return a `ty::FnSig`, no need for a binder)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Dec 4, 2024
2 parents acabb52 + ccb6845 commit f54d5f3
Show file tree
Hide file tree
Showing 161 changed files with 2,064 additions and 895 deletions.
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

0 comments on commit f54d5f3

Please sign in to comment.