bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

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

Browse source
This commit is contained in:
Michael Goulet 2025-01-18 21:25:31 +00:00
parent 8e59cf95d5
commit b08f3d5bdb
11 changed files with 120 additions and 110 deletions
Download patch file Download diff file Expand all files Collapse all files

12
compiler/rustc_mir_build/messages.ftl
View file

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

7
compiler/rustc_mir_build/src/builder/mod.rs
View file

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

81
compiler/rustc_mir_build/src/check_inline.rs
View file

@ -1,81 +0,0 @@
use rustc_attr_parsing::InlineAttr;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::{Body, TerminatorKind};
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;
};
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> {
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]");
}
// FIXME(#127234): Coverage instrumentation currently doesn't handle inlined
// MIR correctly when Modified Condition/Decision Coverage is enabled.
if tcx.sess.instrument_coverage_mcdc() {
return Err("incompatible with MC/DC coverage");
}
let ty = tcx.type_of(def_id);
if match ty.instantiate_identity().kind() {
ty::FnDef(..) => tcx.fn_sig(def_id).instantiate_identity().c_variadic(),
ty::Closure(_, args) => args.as_closure().sig().c_variadic(),
_ => false,
} {
return Err("C variadic");
}
if codegen_attrs.flags.contains(CodegenFnAttrFlags::COLD) {
return Err("cold");
}
// Intrinsic fallback bodies are automatically made cross-crate inlineable,
// but at this stage we don't know whether codegen knows the intrinsic,
// so just conservatively don't inline it. This also ensures that we do not
// accidentally inline the body of an intrinsic that *must* be overridden.
if tcx.has_attr(def_id, sym::rustc_intrinsic) {
return Err("callee is an intrinsic");
}
Ok(())
}
pub fn is_inline_valid_on_body<'tcx>(
_: TyCtxt<'tcx>,
body: &Body<'tcx>,
) -> Result<(), &'static str> {
if body
.basic_blocks
.iter()
.any(|bb| matches!(bb.terminator().kind, TerminatorKind::TailCall { .. }))
{
return Err("can't inline functions with tail calls");
}
Ok(())
}

22
compiler/rustc_mir_build/src/errors.rs
View file

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

4
compiler/rustc_mir_build/src/lib.rs
View file

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

261
compiler/rustc_mir_build/src/lints.rs
View file

@ -1,261 +0,0 @@
use std::ops::ControlFlow;
use rustc_data_structures::graph::iterate::{
NodeStatus, TriColorDepthFirstSearch, TriColorVisitor,
};
use rustc_hir::def::DefKind;
use rustc_middle::mir::{self, BasicBlock, BasicBlocks, Body, Terminator, TerminatorKind};
use rustc_middle::ty::{self, GenericArg, GenericArgs, Instance, Ty, TyCtxt};
use rustc_session::lint::builtin::UNCONDITIONAL_RECURSION;
use rustc_span::Span;
use crate::errors::UnconditionalRecursion;
pub(crate) fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
check_call_recursion(tcx, body);
}
fn check_call_recursion<'tcx>(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_recursion<'tcx>(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
classifier: impl TerminatorClassifier<'tcx>,
) {
let def_id = body.source.def_id().expect_local();
if let DefKind::Fn | DefKind::AssocFn = tcx.def_kind(def_id) {
let mut vis = Search { tcx, body, classifier, reachable_recursive_calls: vec![] };
if let Some(NonRecursive) =
TriColorDepthFirstSearch::new(&body.basic_blocks).run_from_start(&mut vis)
{
return;
}
if vis.reachable_recursive_calls.is_empty() {
return;
}
vis.reachable_recursive_calls.sort();
let sp = tcx.def_span(def_id);
let hir_id = tcx.local_def_id_to_hir_id(def_id);
tcx.emit_node_span_lint(UNCONDITIONAL_RECURSION, hir_id, sp, UnconditionalRecursion {
span: sp,
call_sites: vis.reachable_recursive_calls,
});
}
}
/// 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,
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
terminator: &Terminator<'tcx>,
) -> bool;
}
struct NonRecursive;
struct Search<'mir, 'tcx, C: TerminatorClassifier<'tcx>> {
tcx: TyCtxt<'tcx>,
body: &'mir Body<'tcx>,
classifier: C,
reachable_recursive_calls: Vec<Span>,
}
struct CallRecursion<'tcx> {
trait_args: &'tcx [GenericArg<'tcx>],
}
struct RecursiveDrop<'tcx> {
/// The type that `Drop` is implemented for.
drop_for: Ty<'tcx>,
}
impl<'tcx> TerminatorClassifier<'tcx> for CallRecursion<'tcx> {
/// Returns `true` if `func` refers to the function we are searching in.
fn is_recursive_terminator(
&self,
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
terminator: &Terminator<'tcx>,
) -> bool {
let TerminatorKind::Call { func, args, .. } = &terminator.kind else {
return false;
};
// Resolving function type to a specific instance that is being called is expensive. To
// avoid the cost we check the number of arguments first, which is sufficient to reject
// most of calls as non-recursive.
if args.len() != body.arg_count {
return false;
}
let caller = body.source.def_id();
let typing_env = body.typing_env(tcx);
let func_ty = func.ty(body, tcx);
if let ty::FnDef(callee, args) = *func_ty.kind() {
let Ok(normalized_args) = tcx.try_normalize_erasing_regions(typing_env, args) else {
return false;
};
let (callee, call_args) = if let Ok(Some(instance)) =
Instance::try_resolve(tcx, typing_env, callee, normalized_args)
{
(instance.def_id(), instance.args)
} else {
(callee, normalized_args)
};
// FIXME(#57965): Make this work across function boundaries
// If this is a trait fn, the args on the trait have to match, or we might be
// calling into an entirely different method (for example, a call from the default
// method in the trait to `<A as Trait<B>>::method`, where `A` and/or `B` are
// specific types).
return callee == caller && &call_args[..self.trait_args.len()] == self.trait_args;
}
false
}
}
impl<'tcx> TerminatorClassifier<'tcx> for RecursiveDrop<'tcx> {
fn is_recursive_terminator(
&self,
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
terminator: &Terminator<'tcx>,
) -> bool {
let TerminatorKind::Drop { place, .. } = &terminator.kind else { return false };
let dropped_ty = place.ty(body, tcx).ty;
dropped_ty == self.drop_for
}
}
impl<'mir, 'tcx, C: TerminatorClassifier<'tcx>> TriColorVisitor<BasicBlocks<'tcx>>
for Search<'mir, 'tcx, C>
{
type BreakVal = NonRecursive;
fn node_examined(
&mut self,
bb: BasicBlock,
prior_status: Option<NodeStatus>,
) -> ControlFlow<Self::BreakVal> {
// Back-edge in the CFG (loop).
if let Some(NodeStatus::Visited) = prior_status {
return ControlFlow::Break(NonRecursive);
}
match self.body[bb].terminator().kind {
// These terminators return control flow to the caller.
TerminatorKind::UnwindTerminate(_)
| TerminatorKind::CoroutineDrop
| TerminatorKind::UnwindResume
| TerminatorKind::Return
| TerminatorKind::Unreachable
| TerminatorKind::Yield { .. } => ControlFlow::Break(NonRecursive),
// A InlineAsm without targets (diverging and contains no labels)
// is treated as non-recursing.
TerminatorKind::InlineAsm { ref targets, .. } => {
if !targets.is_empty() {
ControlFlow::Continue(())
} else {
ControlFlow::Break(NonRecursive)
}
}
// These do not.
TerminatorKind::Assert { .. }
| TerminatorKind::Call { .. }
| TerminatorKind::Drop { .. }
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::Goto { .. }
| TerminatorKind::SwitchInt { .. } => ControlFlow::Continue(()),
// Note that tail call terminator technically returns to the caller,
// but for purposes of this lint it makes sense to count it as possibly recursive,
// since it's still a call.
//
// If this'll be repurposed for something else, this might need to be changed.
TerminatorKind::TailCall { .. } => ControlFlow::Continue(()),
}
}
fn node_settled(&mut self, bb: BasicBlock) -> ControlFlow<Self::BreakVal> {
// When we examine a node for the last time, remember it if it is a recursive call.
let terminator = self.body[bb].terminator();
// FIXME(explicit_tail_calls): highlight tail calls as "recursive call site"
//
// We don't want to lint functions that recurse only through tail calls
// (such as `fn g() { become () }`), so just adding `| TailCall { ... }`
// here won't work.
//
// But at the same time we would like to highlight both calls in a function like
// `fn f() { if false { become f() } else { f() } }`, so we need to figure something out.
if self.classifier.is_recursive_terminator(self.tcx, self.body, terminator) {
self.reachable_recursive_calls.push(terminator.source_info.span);
}
ControlFlow::Continue(())
}
fn ignore_edge(&mut self, bb: BasicBlock, target: BasicBlock) -> bool {
let terminator = self.body[bb].terminator();
let ignore_unwind = terminator.unwind() == Some(&mir::UnwindAction::Cleanup(target))
&& terminator.successors().count() > 1;
if ignore_unwind || self.classifier.is_recursive_terminator(self.tcx, self.body, terminator)
{
return true;
}
match &terminator.kind {
TerminatorKind::FalseEdge { imaginary_target, .. } => imaginary_target == &target,
_ => false,
}
}
}