Skip to content

Commit

Permalink
Rollup merge of #138028 - workingjubilee:is-rustic-abi, r=compiler-er…
Browse files Browse the repository at this point in the history
…rors

compiler: add `ExternAbi::is_rustic_abi`

Various parts of the compiler were hand-rolling this extremely simple check that is nonetheless easy to get wrong as the compiler evolves over time. Discourage them from being so "original" again by replacing it with a single implementation on the type that represents these ABIs. This simplifies a surprising amount of code as a result.

Also fixes #132981, an ICE that emerged due to other checks being made stricter.
  • Loading branch information
workingjubilee authored Mar 5, 2025
2 parents 49b74ce + 8a68987 commit 2d6b3c2
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 43 deletions.
11 changes: 11 additions & 0 deletions compiler/rustc_abi/src/extern_abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,17 @@ impl StableOrd for ExternAbi {
}

impl ExternAbi {
/// An ABI "like Rust"
///
/// These ABIs are fully controlled by the Rust compiler, which means they
/// - support unwinding with `-Cpanic=unwind`, unlike `extern "C"`
/// - often diverge from the C ABI
/// - are subject to change between compiler versions
pub fn is_rustic_abi(self) -> bool {
use ExternAbi::*;
matches!(self, Rust | RustCall | RustIntrinsic | RustCold)
}

pub fn supports_varargs(self) -> bool {
// * C and Cdecl obviously support varargs.
// * C can be based on Aapcs, SysV64 or Win64, so they must support varargs.
Expand Down
30 changes: 11 additions & 19 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::iter;
use std::ops::ControlFlow;

use rustc_abi::{BackendRepr, ExternAbi, TagEncoding, VariantIdx, Variants, WrappingRange};
use rustc_abi::{BackendRepr, TagEncoding, VariantIdx, Variants, WrappingRange};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::DiagMessage;
use rustc_hir::intravisit::VisitorExt;
Expand Down Expand Up @@ -1349,7 +1349,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {

ty::FnPtr(sig_tys, hdr) => {
let sig = sig_tys.with(hdr);
if self.is_internal_abi(sig.abi()) {
if sig.abi().is_rustic_abi() {
return FfiUnsafe {
ty,
reason: fluent::lint_improper_ctypes_fnptr_reason,
Expand Down Expand Up @@ -1552,13 +1552,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
self.check_type_for_ffi_and_report_errors(span, ty, true, false);
}

fn is_internal_abi(&self, abi: ExternAbi) -> bool {
matches!(
abi,
ExternAbi::Rust | ExternAbi::RustCall | ExternAbi::RustCold | ExternAbi::RustIntrinsic
)
}

/// Find any fn-ptr types with external ABIs in `ty`.
///
/// For example, `Option<extern "C" fn()>` returns `extern "C" fn()`
Expand All @@ -1567,17 +1560,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
hir_ty: &hir::Ty<'tcx>,
ty: Ty<'tcx>,
) -> Vec<(Ty<'tcx>, Span)> {
struct FnPtrFinder<'a, 'b, 'tcx> {
visitor: &'a ImproperCTypesVisitor<'b, 'tcx>,
struct FnPtrFinder<'tcx> {
spans: Vec<Span>,
tys: Vec<Ty<'tcx>>,
}

impl<'a, 'b, 'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'a, 'b, 'tcx> {
impl<'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'tcx> {
fn visit_ty(&mut self, ty: &'_ hir::Ty<'_, AmbigArg>) {
debug!(?ty);
if let hir::TyKind::BareFn(hir::BareFnTy { abi, .. }) = ty.kind
&& !self.visitor.is_internal_abi(*abi)
&& !abi.is_rustic_abi()
{
self.spans.push(ty.span);
}
Expand All @@ -1586,12 +1578,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
}

impl<'a, 'b, 'tcx> ty::visit::TypeVisitor<TyCtxt<'tcx>> for FnPtrFinder<'a, 'b, 'tcx> {
impl<'tcx> ty::visit::TypeVisitor<TyCtxt<'tcx>> for FnPtrFinder<'tcx> {
type Result = ();

fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
if let ty::FnPtr(_, hdr) = ty.kind()
&& !self.visitor.is_internal_abi(hdr.abi)
&& !hdr.abi.is_rustic_abi()
{
self.tys.push(ty);
}
Expand All @@ -1600,7 +1592,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
}

let mut visitor = FnPtrFinder { visitor: self, spans: Vec::new(), tys: Vec::new() };
let mut visitor = FnPtrFinder { spans: Vec::new(), tys: Vec::new() };
ty.visit_with(&mut visitor);
visitor.visit_ty_unambig(hir_ty);

Expand All @@ -1615,13 +1607,13 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations {

match it.kind {
hir::ForeignItemKind::Fn(sig, _, _) => {
if vis.is_internal_abi(abi) {
if abi.is_rustic_abi() {
vis.check_fn(it.owner_id.def_id, sig.decl)
} else {
vis.check_foreign_fn(it.owner_id.def_id, sig.decl);
}
}
hir::ForeignItemKind::Static(ty, _, _) if !vis.is_internal_abi(abi) => {
hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => {
vis.check_foreign_static(it.owner_id, ty.span);
}
hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (),
Expand Down Expand Up @@ -1775,7 +1767,7 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions {
};

let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition };
if vis.is_internal_abi(abi) {
if abi.is_rustic_abi() {
vis.check_fn(id, decl);
} else {
vis.check_foreign_fn(id, decl);
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool {

// Rust calls cannot themselves create foreign unwinds.
// We assume this is true for intrinsics as well.
if let ExternAbi::RustIntrinsic
| ExternAbi::Rust
| ExternAbi::RustCall
| ExternAbi::RustCold = sig.abi()
{
if sig.abi().is_rustic_abi() {
continue;
};

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_monomorphize/src/mono_checks/abi_check.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! This module ensures that if a function's ABI requires a particular target feature,
//! that target feature is enabled both on the callee and all callers.
use rustc_abi::{BackendRepr, ExternAbi, RegKind};
use rustc_abi::{BackendRepr, RegKind};
use rustc_hir::CRATE_HIR_ID;
use rustc_middle::mir::{self, traversal};
use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt};
Expand Down Expand Up @@ -115,8 +115,8 @@ fn check_call_site_abi<'tcx>(
span: Span,
caller: InstanceKind<'tcx>,
) {
if callee.fn_sig(tcx).abi() == ExternAbi::Rust {
// "Rust" ABI never passes arguments in vector registers.
if callee.fn_sig(tcx).abi().is_rustic_abi() {
// we directly handle the soundness of Rust ABIs
return;
}
let typing_env = ty::TypingEnv::fully_monomorphized();
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,7 @@ fn fn_abi_sanity_check<'tcx>(
) {
let tcx = cx.tcx();

if spec_abi == ExternAbi::Rust
|| spec_abi == ExternAbi::RustCall
|| spec_abi == ExternAbi::RustCold
{
if spec_abi.is_rustic_abi() {
if arg.layout.is_zst() {
// Casting closures to function pointers depends on ZST closure types being
// omitted entirely in the calling convention.
Expand Down Expand Up @@ -687,7 +684,7 @@ fn fn_abi_adjust_for_abi<'tcx>(

let tcx = cx.tcx();

if abi == ExternAbi::Rust || abi == ExternAbi::RustCall || abi == ExternAbi::RustIntrinsic {
if abi.is_rustic_abi() {
fn_abi.adjust_for_rust_abi(cx, abi);

// Look up the deduced parameter attributes for this function, if we have its def ID and
Expand Down
11 changes: 0 additions & 11 deletions tests/crashes/132981.rs

This file was deleted.

6 changes: 6 additions & 0 deletions tests/ui/abi/rust-cold-works-with-rustic-args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//@build-pass
//@compile-flags: -Clink-dead-code=true --crate-type lib
// We used to not handle all "rustic" ABIs in a (relatively) uniform way,
// so we failed to fix up arguments for actually passing through the ABI...
#![feature(rust_cold_cc)]
pub extern "rust-cold" fn foo(_: [usize; 3]) {}

0 comments on commit 2d6b3c2

Please sign in to comment.