Skip to content

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

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

Merged
merged 1 commit into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 12 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_mir_build/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
22 changes: 0 additions & 22 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
}
4 changes: 1 addition & 3 deletions compiler/rustc_mir_build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_mir_transform/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
}
}
}

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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]");
Expand Down Expand Up @@ -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> {
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_mir_transform/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand All @@ -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(
Expand Down
Loading
Loading