Auto merge of #133893 - fmease:rollup-11pi6fg, r=fmease

Rollup of 10 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)
 - #133761 (Update books)
 - #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)
 - #133890 (Add a new test ui/incoherent-inherent-impls/no-other-unrelated-errors to check E0116 does not cause unrelated errors)
 - #133892 (Revert #133817)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_ast/src/ast.rs

@@ -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 {

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -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",

compiler/rustc_lint/messages.ftl

@@ -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

compiler/rustc_lint/src/lints.rs

@@ -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,

compiler/rustc_lint/src/types.rs

@@ -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};
 
@@ -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
@@ -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 {
@@ -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,
@@ -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 {
@@ -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);
                     }
                 }
             }
@@ -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);
             }
             _ => {}
         };

compiler/rustc_mir_build/messages.ftl

@@ -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
@@ -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
@@ -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
@@ -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
 
@@ -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
 
@@ -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
@@ -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