Rollup merge of #135705 - compiler-errors:lint, r=estebank

Consolidate ad-hoc MIR lints into real pass-manager-based MIR lints

It feels much cleaner to do all MIR-related things using the pass manager.

Author: matthiaskrgr

compiler/rustc_mir_build/messages.ftl

@@ -118,12 +118,6 @@ mir_build_extern_static_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
     .note = extern statics are not controlled by the Rust type system: invalid data, aliasing violations or data races will cause undefined behavior
     .label = use of extern static
 
-mir_build_force_inline =
-    `{$callee}` is incompatible with `#[rustc_force_inline]`
-    .attr = annotation here
-    .callee = `{$callee}` defined here
-    .note = incompatible due to: {$reason}
-
 mir_build_inform_irrefutable = `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
 
 mir_build_initializing_type_with_requires_unsafe =
@@ -330,12 +324,6 @@ mir_build_type_not_structural_more_info = see https://doc.rust-lang.org/stable/s
 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
-    .help = a `loop` may express intention better if this is on purpose
-
-mir_build_unconditional_recursion_call_site_label = recursive call site
-
 mir_build_union_field_requires_unsafe =
     access to union field is unsafe and requires unsafe block
     .note = the field may not be properly initialized: using uninitialized data will cause undefined behavior

compiler/rustc_mir_build/src/builder/mod.rs

@@ -26,10 +26,8 @@ use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt, TypeVisitableExt, TypingMode
 use rustc_middle::{bug, span_bug};
 use rustc_span::{Span, Symbol, sym};
 
-use super::lints;
 use crate::builder::expr::as_place::PlaceBuilder;
 use crate::builder::scope::DropKind;
-use crate::check_inline;
 
 pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
     tcx: TyCtxt<'tcx>,
@@ -48,7 +46,7 @@ pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
 }
 
 /// Construct the MIR for a given `DefId`.
-pub(crate) fn mir_build<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx> {
+pub(crate) fn build_mir<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx> {
     let tcx = tcx.tcx;
     tcx.ensure_with_value().thir_abstract_const(def);
     if let Err(e) = tcx.check_match(def) {
@@ -80,9 +78,6 @@ pub(crate) fn mir_build<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx
         }
     };
 
-    lints::check(tcx, &body);
-    check_inline::check_force_inline(tcx, &body);
-
     // The borrow checker will replace all the regions here with its own
     // inference variables. There's no point having non-erased regions here.
     // The exception is `body.user_type_annotations`, which is used unmodified

compiler/rustc_mir_build/src/errors.rs

@@ -11,16 +11,6 @@ use rustc_span::{Span, Symbol};
 
 use crate::fluent_generated as fluent;
 
-#[derive(LintDiagnostic)]
-#[diag(mir_build_unconditional_recursion)]
-#[help]
-pub(crate) struct UnconditionalRecursion {
-    #[label]
-    pub(crate) span: Span,
-    #[label(mir_build_unconditional_recursion_call_site_label)]
-    pub(crate) call_sites: Vec<Span>,
-}
-
 #[derive(LintDiagnostic)]
 #[diag(mir_build_call_to_deprecated_safe_fn_requires_unsafe)]
 pub(crate) struct CallToDeprecatedSafeFnRequiresUnsafe {
@@ -1107,15 +1097,3 @@ impl<'a> Subdiagnostic for Rust2024IncompatiblePatSugg<'a> {
         );
     }
 }
-
-#[derive(Diagnostic)]
-#[diag(mir_build_force_inline)]
-#[note]
-pub(crate) struct InvalidForceInline {
-    #[primary_span]
-    pub attr_span: Span,
-    #[label(mir_build_callee)]
-    pub callee_span: Span,
-    pub callee: String,
-    pub reason: &'static str,
-}

compiler/rustc_mir_build/src/lib.rs

@@ -15,11 +15,9 @@
 // "Go to file" feature to silently ignore all files in the module, probably
 // because it assumes that "build" is a build-output directory. See #134365.
 mod builder;
-pub mod check_inline;
 mod check_tail_calls;
 mod check_unsafety;
 mod errors;
-pub mod lints;
 mod thir;
 
 use rustc_middle::util::Providers;
@@ -29,7 +27,7 @@ rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
 pub fn provide(providers: &mut Providers) {
     providers.check_match = thir::pattern::check_match;
     providers.lit_to_const = thir::constant::lit_to_const;
-    providers.hooks.build_mir = builder::mir_build;
+    providers.hooks.build_mir = builder::build_mir;
     providers.closure_saved_names_of_captured_variables =
         builder::closure_saved_names_of_captured_variables;
     providers.check_unsafety = check_unsafety::check_unsafety;

compiler/rustc_mir_transform/messages.ftl

@@ -27,6 +27,12 @@ mir_transform_force_inline =
     .callee = `{$callee}` defined here
     .note = could not be inlined due to: {$reason}
 
+mir_transform_force_inline_attr =
+    `{$callee}` is incompatible with `#[rustc_force_inline]`
+    .attr = annotation here
+    .callee = `{$callee}` defined here
+    .note = incompatible due to: {$reason}
+
 mir_transform_force_inline_justification =
     `{$callee}` is required to be inlined to: {$sym}
 
@@ -66,6 +72,12 @@ mir_transform_unaligned_packed_ref = reference to packed field is unaligned
     .note_ub = creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
     .help = copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
 
+mir_transform_unconditional_recursion = function cannot return without recursing
+    .label = cannot return without recursing
+    .help = a `loop` may express intention better if this is on purpose
+
+mir_transform_unconditional_recursion_call_site_label = recursive call site
+
 mir_transform_undefined_transmute = pointers cannot be transmuted to integers during const eval
     .note = at compile-time, pointers do not have an integer value
     .note2 = avoiding this restriction via `union` or raw pointers leads to compile-time undefined behavior

compiler/rustc_mir_build/src/lints.rs renamed to compiler/rustc_mir_transform/src/check_call_recursion.rs

@@ -10,25 +10,54 @@ use rustc_session::lint::builtin::UNCONDITIONAL_RECURSION;
 use rustc_span::Span;
 
 use crate::errors::UnconditionalRecursion;
+use crate::pass_manager::MirLint;
 
-pub(crate) fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
-    check_call_recursion(tcx, body);
+pub(super) struct CheckCallRecursion;
+
+impl<'tcx> MirLint<'tcx> for CheckCallRecursion {
+    fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
+        let def_id = body.source.def_id().expect_local();
+
+        if let DefKind::Fn | DefKind::AssocFn = tcx.def_kind(def_id) {
+            // If this is trait/impl method, extract the trait's args.
+            let trait_args = match tcx.trait_of_item(def_id.to_def_id()) {
+                Some(trait_def_id) => {
+                    let trait_args_count = tcx.generics_of(trait_def_id).count();
+                    &GenericArgs::identity_for_item(tcx, def_id)[..trait_args_count]
+                }
+                _ => &[],
+            };
+
+            check_recursion(tcx, body, CallRecursion { trait_args })
+        }
+    }
 }
 
-fn check_call_recursion<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
-    let def_id = body.source.def_id().expect_local();
+/// Requires drop elaboration to have been performed.
+pub(super) struct CheckDropRecursion;
 
-    if let DefKind::Fn | DefKind::AssocFn = tcx.def_kind(def_id) {
-        // If this is trait/impl method, extract the trait's args.
-        let trait_args = match tcx.trait_of_item(def_id.to_def_id()) {
-            Some(trait_def_id) => {
-                let trait_args_count = tcx.generics_of(trait_def_id).count();
-                &GenericArgs::identity_for_item(tcx, def_id)[..trait_args_count]
-            }
-            _ => &[],
-        };
+impl<'tcx> MirLint<'tcx> for CheckDropRecursion {
+    fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
+        let def_id = body.source.def_id().expect_local();
 
-        check_recursion(tcx, body, CallRecursion { trait_args })
+        // First check if `body` is an `fn drop()` of `Drop`
+        if let DefKind::AssocFn = tcx.def_kind(def_id)
+        && let Some(trait_ref) =
+            tcx.impl_of_method(def_id.to_def_id()).and_then(|def_id| tcx.impl_trait_ref(def_id))
+        && let Some(drop_trait) = tcx.lang_items().drop_trait()
+        && drop_trait == trait_ref.instantiate_identity().def_id
+        // avoid erroneous `Drop` impls from causing ICEs below
+        && let sig = tcx.fn_sig(def_id).instantiate_identity()
+        && sig.inputs().skip_binder().len() == 1
+        {
+            // It was. Now figure out for what type `Drop` is implemented and then
+            // check for recursion.
+            if let ty::Ref(_, dropped_ty, _) =
+                tcx.liberate_late_bound_regions(def_id.to_def_id(), sig.input(0)).kind()
+            {
+                check_recursion(tcx, body, RecursiveDrop { drop_for: *dropped_ty });
+            }
+        }
     }
 }
 
@@ -61,30 +90,6 @@ fn check_recursion<'tcx>(
     }
 }
 
-/// Requires drop elaboration to have been performed first.
-pub fn check_drop_recursion<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
-    let def_id = body.source.def_id().expect_local();
-
-    // First check if `body` is an `fn drop()` of `Drop`
-    if let DefKind::AssocFn = tcx.def_kind(def_id)
-        && let Some(trait_ref) =
-            tcx.impl_of_method(def_id.to_def_id()).and_then(|def_id| tcx.impl_trait_ref(def_id))
-        && let Some(drop_trait) = tcx.lang_items().drop_trait()
-        && drop_trait == trait_ref.instantiate_identity().def_id
-        // avoid erroneous `Drop` impls from causing ICEs below
-        && let sig = tcx.fn_sig(def_id).instantiate_identity()
-        && sig.inputs().skip_binder().len() == 1
-    {
-        // It was. Now figure out for what type `Drop` is implemented and then
-        // check for recursion.
-        if let ty::Ref(_, dropped_ty, _) =
-            tcx.liberate_late_bound_regions(def_id.to_def_id(), sig.input(0)).kind()
-        {
-            check_recursion(tcx, body, RecursiveDrop { drop_for: *dropped_ty });
-        }
-    }
-}
-
 trait TerminatorClassifier<'tcx> {
     fn is_recursive_terminator(
         &self,

compiler/rustc_mir_build/src/check_inline.rs renamed to compiler/rustc_mir_transform/src/check_inline.rs

@@ -1,3 +1,6 @@
+//! Check that a body annotated with `#[rustc_force_inline]` will not fail to inline based on its
+//! definition alone (irrespective of any specific caller).
+
 use rustc_attr_parsing::InlineAttr;
 use rustc_hir::def_id::DefId;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
@@ -6,30 +9,37 @@ use rustc_middle::ty;
 use rustc_middle::ty::TyCtxt;
 use rustc_span::sym;
 
-/// Check that a body annotated with `#[rustc_force_inline]` will not fail to inline based on its
-/// definition alone (irrespective of any specific caller).
-pub(crate) fn check_force_inline<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
-    let def_id = body.source.def_id();
-    if !tcx.hir().body_owner_kind(def_id).is_fn_or_closure() || !def_id.is_local() {
-        return;
-    }
-    let InlineAttr::Force { attr_span, .. } = tcx.codegen_fn_attrs(def_id).inline else {
-        return;
-    };
+use crate::pass_manager::MirLint;
 
-    if let Err(reason) =
-        is_inline_valid_on_fn(tcx, def_id).and_then(|_| is_inline_valid_on_body(tcx, body))
-    {
-        tcx.dcx().emit_err(crate::errors::InvalidForceInline {
-            attr_span,
-            callee_span: tcx.def_span(def_id),
-            callee: tcx.def_path_str(def_id),
-            reason,
-        });
+pub(super) struct CheckForceInline;
+
+impl<'tcx> MirLint<'tcx> for CheckForceInline {
+    fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
+        let def_id = body.source.def_id();
+        if !tcx.hir().body_owner_kind(def_id).is_fn_or_closure() || !def_id.is_local() {
+            return;
+        }
+        let InlineAttr::Force { attr_span, .. } = tcx.codegen_fn_attrs(def_id).inline else {
+            return;
+        };
+
+        if let Err(reason) =
+            is_inline_valid_on_fn(tcx, def_id).and_then(|_| is_inline_valid_on_body(tcx, body))
+        {
+            tcx.dcx().emit_err(crate::errors::InvalidForceInline {
+                attr_span,
+                callee_span: tcx.def_span(def_id),
+                callee: tcx.def_path_str(def_id),
+                reason,
+            });
+        }
     }
 }
 
-pub fn is_inline_valid_on_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Result<(), &'static str> {
+pub(super) fn is_inline_valid_on_fn<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    def_id: DefId,
+) -> Result<(), &'static str> {
     let codegen_attrs = tcx.codegen_fn_attrs(def_id);
     if tcx.has_attr(def_id, sym::rustc_no_mir_inline) {
         return Err("#[rustc_no_mir_inline]");
@@ -65,7 +75,7 @@ pub fn is_inline_valid_on_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Result<(
     Ok(())
 }
 
-pub fn is_inline_valid_on_body<'tcx>(
+pub(super) fn is_inline_valid_on_body<'tcx>(
     _: TyCtxt<'tcx>,
     body: &Body<'tcx>,
 ) -> Result<(), &'static str> {

compiler/rustc_mir_transform/src/errors.rs

@@ -9,6 +9,28 @@ use rustc_span::{Span, Symbol};
 
 use crate::fluent_generated as fluent;
 
+#[derive(LintDiagnostic)]
+#[diag(mir_transform_unconditional_recursion)]
+#[help]
+pub(crate) struct UnconditionalRecursion {
+    #[label]
+    pub(crate) span: Span,
+    #[label(mir_transform_unconditional_recursion_call_site_label)]
+    pub(crate) call_sites: Vec<Span>,
+}
+
+#[derive(Diagnostic)]
+#[diag(mir_transform_force_inline_attr)]
+#[note]
+pub(crate) struct InvalidForceInline {
+    #[primary_span]
+    pub attr_span: Span,
+    #[label(mir_transform_callee)]
+    pub callee_span: Span,
+    pub callee: String,
+    pub reason: &'static str,
+}
+
 #[derive(LintDiagnostic)]
 pub(crate) enum ConstMutate {
     #[diag(mir_transform_const_modify)]

compiler/rustc_mir_transform/src/inline.rs

@@ -21,8 +21,8 @@ use tracing::{debug, instrument, trace, trace_span};
 use crate::cost_checker::CostChecker;
 use crate::deref_separator::deref_finder;
 use crate::simplify::simplify_cfg;
-use crate::util;
 use crate::validate::validate_types;
+use crate::{check_inline, util};
 
 pub(crate) mod cycle;
 
@@ -575,7 +575,7 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
     check_mir_is_available(inliner, caller_body, callsite.callee)?;
 
     let callee_attrs = tcx.codegen_fn_attrs(callsite.callee.def_id());
-    rustc_mir_build::check_inline::is_inline_valid_on_fn(tcx, callsite.callee.def_id())?;
+    check_inline::is_inline_valid_on_fn(tcx, callsite.callee.def_id())?;
     check_codegen_attributes(inliner, callsite, callee_attrs)?;
 
     let terminator = caller_body[callsite.block].terminator.as_ref().unwrap();
@@ -590,7 +590,7 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
     }
 
     let callee_body = try_instance_mir(tcx, callsite.callee.def)?;
-    rustc_mir_build::check_inline::is_inline_valid_on_body(tcx, callee_body)?;
+    check_inline::is_inline_valid_on_body(tcx, callee_body)?;
     inliner.check_callee_mir_body(callsite, callee_body, callee_attrs)?;
 
     let Ok(callee_body) = callsite.callee.try_instantiate_mir_and_normalize_erasing_regions(