From 3f50076fb3166b3f71b4a52e8ebc4cd59572ac74 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 4 Nov 2024 12:53:02 -0800 Subject: [PATCH] compiler: gate `extern "{abi}"` in ast_lowering By moving this stability check into AST lowering, we effectively make it impossible to accidentally miss, as it must happen to generate HIR. Also, we put the ABI-stability code next to code that actually uses it! This allows code that wants to reason about backend ABI implementations to stop worrying about high-level concerns like syntax stability, while still leaving it as the authority on what ABIs actually exist. It also makes it easy to refactor things to have more consistent errors. For now, we only apply this to generalize the existing messages a bit. --- Cargo.lock | 2 +- compiler/rustc_abi/Cargo.toml | 2 - compiler/rustc_abi/src/callconv.rs | 3 +- compiler/rustc_abi/src/extern_abi.rs | 105 +------------ compiler/rustc_abi/src/lib.rs | 4 +- compiler/rustc_ast_lowering/Cargo.toml | 1 + compiler/rustc_ast_lowering/src/item.rs | 16 +- compiler/rustc_ast_lowering/src/lib.rs | 1 + compiler/rustc_ast_lowering/src/stability.rs | 147 ++++++++++++++++++ compiler/rustc_ast_passes/src/feature_gate.rs | 44 +----- compiler/rustc_target/src/spec/mod.rs | 5 +- tests/ui/extern/fictional-abi.rs | 3 + tests/ui/extern/fictional-abi.stderr | 11 ++ 13 files changed, 186 insertions(+), 158 deletions(-) create mode 100644 compiler/rustc_ast_lowering/src/stability.rs create mode 100644 tests/ui/extern/fictional-abi.rs create mode 100644 tests/ui/extern/fictional-abi.stderr diff --git a/Cargo.lock b/Cargo.lock index dce991816658..14634fe3b463 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3336,7 +3336,6 @@ dependencies = [ "rand 0.8.5", "rand_xoshiro", "rustc_data_structures", - "rustc_feature", "rustc_index", "rustc_macros", "rustc_serialize", @@ -3398,6 +3397,7 @@ dependencies = [ "rustc_ast_pretty", "rustc_data_structures", "rustc_errors", + "rustc_feature", "rustc_fluent_macro", "rustc_hir", "rustc_index", diff --git a/compiler/rustc_abi/Cargo.toml b/compiler/rustc_abi/Cargo.toml index 3acd25e54611..1013f1d3958d 100644 --- a/compiler/rustc_abi/Cargo.toml +++ b/compiler/rustc_abi/Cargo.toml @@ -9,7 +9,6 @@ bitflags = "2.4.1" rand = { version = "0.8.4", default-features = false, optional = true } rand_xoshiro = { version = "0.6.0", optional = true } rustc_data_structures = { path = "../rustc_data_structures", optional = true } -rustc_feature = { path = "../rustc_feature", optional = true } rustc_index = { path = "../rustc_index", default-features = false } rustc_macros = { path = "../rustc_macros", optional = true } rustc_serialize = { path = "../rustc_serialize", optional = true } @@ -24,7 +23,6 @@ default = ["nightly", "randomize"] # without depending on rustc_data_structures, rustc_macros and rustc_serialize nightly = [ "dep:rustc_data_structures", - "dep:rustc_feature", "dep:rustc_macros", "dep:rustc_serialize", "dep:rustc_span", diff --git a/compiler/rustc_abi/src/callconv.rs b/compiler/rustc_abi/src/callconv.rs index daa365bf6e1d..9fb70b80c9ef 100644 --- a/compiler/rustc_abi/src/callconv.rs +++ b/compiler/rustc_abi/src/callconv.rs @@ -1,6 +1,5 @@ #[cfg(feature = "nightly")] -use crate::{BackendRepr, FieldsShape, TyAbiInterface, TyAndLayout}; -use crate::{Primitive, Size, Variants}; +use crate::{BackendRepr, FieldsShape, Primitive, Size, TyAbiInterface, TyAndLayout, Variants}; mod reg; diff --git a/compiler/rustc_abi/src/extern_abi.rs b/compiler/rustc_abi/src/extern_abi.rs index 130834d560f7..faa952b6e2cf 100644 --- a/compiler/rustc_abi/src/extern_abi.rs +++ b/compiler/rustc_abi/src/extern_abi.rs @@ -1,7 +1,6 @@ use std::fmt; use rustc_macros::{Decodable, Encodable, HashStable_Generic}; -use rustc_span::{Span, Symbol, sym}; #[cfg(test)] mod tests; @@ -95,14 +94,14 @@ impl Abi { #[derive(Copy, Clone)] pub struct AbiData { - abi: Abi, + pub abi: Abi, /// Name of this ABI as we like it called. - name: &'static str, + pub name: &'static str, } #[allow(non_upper_case_globals)] -const AbiDatas: &[AbiData] = &[ +pub const AbiDatas: &[AbiData] = &[ AbiData { abi: Abi::Rust, name: "Rust" }, AbiData { abi: Abi::C { unwind: false }, name: "C" }, AbiData { abi: Abi::C { unwind: true }, name: "C-unwind" }, @@ -169,104 +168,6 @@ pub fn all_names() -> Vec<&'static str> { AbiDatas.iter().map(|d| d.name).collect() } -pub fn enabled_names(features: &rustc_feature::Features, span: Span) -> Vec<&'static str> { - AbiDatas - .iter() - .map(|d| d.name) - .filter(|name| is_enabled(features, span, name).is_ok()) - .collect() -} - -pub enum AbiDisabled { - Unstable { feature: Symbol, explain: &'static str }, - Unrecognized, -} - -pub fn is_enabled( - features: &rustc_feature::Features, - span: Span, - name: &str, -) -> Result<(), AbiDisabled> { - let s = is_stable(name); - if let Err(AbiDisabled::Unstable { feature, .. }) = s { - if features.enabled(feature) || span.allows_unstable(feature) { - return Ok(()); - } - } - s -} - -/// Returns whether the ABI is stable to use. -/// -/// Note that there is a separate check determining whether the ABI is even supported -/// on the current target; see `fn is_abi_supported` in `rustc_target::spec`. -pub fn is_stable(name: &str) -> Result<(), AbiDisabled> { - match name { - // Stable - "Rust" | "C" | "C-unwind" | "cdecl" | "cdecl-unwind" | "stdcall" | "stdcall-unwind" - | "fastcall" | "fastcall-unwind" | "aapcs" | "aapcs-unwind" | "win64" | "win64-unwind" - | "sysv64" | "sysv64-unwind" | "system" | "system-unwind" | "efiapi" | "thiscall" - | "thiscall-unwind" => Ok(()), - "rust-intrinsic" => Err(AbiDisabled::Unstable { - feature: sym::intrinsics, - explain: "intrinsics are subject to change", - }), - "vectorcall" => Err(AbiDisabled::Unstable { - feature: sym::abi_vectorcall, - explain: "vectorcall is experimental and subject to change", - }), - "vectorcall-unwind" => Err(AbiDisabled::Unstable { - feature: sym::abi_vectorcall, - explain: "vectorcall-unwind ABI is experimental and subject to change", - }), - "rust-call" => Err(AbiDisabled::Unstable { - feature: sym::unboxed_closures, - explain: "rust-call ABI is subject to change", - }), - "rust-cold" => Err(AbiDisabled::Unstable { - feature: sym::rust_cold_cc, - explain: "rust-cold is experimental and subject to change", - }), - "ptx-kernel" => Err(AbiDisabled::Unstable { - feature: sym::abi_ptx, - explain: "PTX ABIs are experimental and subject to change", - }), - "unadjusted" => Err(AbiDisabled::Unstable { - feature: sym::abi_unadjusted, - explain: "unadjusted ABI is an implementation detail and perma-unstable", - }), - "msp430-interrupt" => Err(AbiDisabled::Unstable { - feature: sym::abi_msp430_interrupt, - explain: "msp430-interrupt ABI is experimental and subject to change", - }), - "x86-interrupt" => Err(AbiDisabled::Unstable { - feature: sym::abi_x86_interrupt, - explain: "x86-interrupt ABI is experimental and subject to change", - }), - "gpu-kernel" => Err(AbiDisabled::Unstable { - feature: sym::abi_gpu_kernel, - explain: "gpu-kernel ABI is experimental and subject to change", - }), - "avr-interrupt" | "avr-non-blocking-interrupt" => Err(AbiDisabled::Unstable { - feature: sym::abi_avr_interrupt, - explain: "avr-interrupt and avr-non-blocking-interrupt ABIs are experimental and subject to change", - }), - "riscv-interrupt-m" | "riscv-interrupt-s" => Err(AbiDisabled::Unstable { - feature: sym::abi_riscv_interrupt, - explain: "riscv-interrupt ABIs are experimental and subject to change", - }), - "C-cmse-nonsecure-call" => Err(AbiDisabled::Unstable { - feature: sym::abi_c_cmse_nonsecure_call, - explain: "C-cmse-nonsecure-call ABI is experimental and subject to change", - }), - "C-cmse-nonsecure-entry" => Err(AbiDisabled::Unstable { - feature: sym::cmse_nonsecure_entry, - explain: "C-cmse-nonsecure-entry ABI is experimental and subject to change", - }), - _ => Err(AbiDisabled::Unrecognized), - } -} - impl Abi { /// Default ABI chosen for `extern fn` declarations without an explicit ABI. pub const FALLBACK: Abi = Abi::C { unwind: false }; diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index fc34b2889330..259f1c18ea8e 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -66,9 +66,7 @@ mod extern_abi; pub use callconv::{Heterogeneous, HomogeneousAggregate, Reg, RegKind}; #[cfg(feature = "nightly")] -pub use extern_abi::{ - AbiDisabled, AbiUnsupported, ExternAbi, all_names, enabled_names, is_enabled, is_stable, lookup, -}; +pub use extern_abi::{AbiDatas, AbiUnsupported, ExternAbi, all_names, lookup}; #[cfg(feature = "nightly")] pub use layout::{FIRST_VARIANT, FieldIdx, Layout, TyAbiInterface, TyAndLayout, VariantIdx}; pub use layout::{LayoutCalculator, LayoutCalculatorError}; diff --git a/compiler/rustc_ast_lowering/Cargo.toml b/compiler/rustc_ast_lowering/Cargo.toml index 754f3c1a6e9a..ce95f4dfa1b8 100644 --- a/compiler/rustc_ast_lowering/Cargo.toml +++ b/compiler/rustc_ast_lowering/Cargo.toml @@ -13,6 +13,7 @@ rustc_ast = { path = "../rustc_ast" } rustc_ast_pretty = { path = "../rustc_ast_pretty" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } +rustc_feature = { path = "../rustc_feature" } rustc_fluent_macro = { path = "../rustc_fluent_macro" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 7379a3d2cde6..a77ebb7e9906 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -20,6 +20,7 @@ use super::errors::{ InvalidAbi, InvalidAbiReason, InvalidAbiSuggestion, MisplacedRelaxTraitBound, TupleStructWithDefault, }; +use super::stability::{enabled_names, gate_unstable_abi}; use super::{ AstOwner, FnDeclKind, ImplTraitContext, ImplTraitPosition, LoweringContext, ParamMode, ResolverAstLoweringExt, @@ -1479,11 +1480,16 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - pub(super) fn lower_abi(&mut self, abi: StrLit) -> ExternAbi { - rustc_abi::lookup(abi.symbol_unescaped.as_str()).unwrap_or_else(|err| { - self.error_on_invalid_abi(abi, err); + pub(super) fn lower_abi(&mut self, abi_str: StrLit) -> ExternAbi { + let ast::StrLit { symbol_unescaped, span, .. } = abi_str; + let extern_abi = rustc_abi::lookup(symbol_unescaped.as_str()).unwrap_or_else(|err| { + self.error_on_invalid_abi(abi_str, err); ExternAbi::Rust - }) + }); + let sess = self.tcx.sess; + let features = self.tcx.features(); + gate_unstable_abi(sess, features, span, extern_abi); + extern_abi } pub(super) fn lower_extern(&mut self, ext: Extern) -> ExternAbi { @@ -1495,7 +1501,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } fn error_on_invalid_abi(&self, abi: StrLit, err: rustc_abi::AbiUnsupported) { - let abi_names = rustc_abi::enabled_names(self.tcx.features(), abi.span) + let abi_names = enabled_names(self.tcx.features(), abi.span) .iter() .map(|s| Symbol::intern(s)) .collect::>(); diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 127b7e3684e9..8997fe24c1fa 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -84,6 +84,7 @@ mod index; mod item; mod pat; mod path; +mod stability; rustc_fluent_macro::fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_ast_lowering/src/stability.rs b/compiler/rustc_ast_lowering/src/stability.rs new file mode 100644 index 000000000000..5fd26a29abac --- /dev/null +++ b/compiler/rustc_ast_lowering/src/stability.rs @@ -0,0 +1,147 @@ +use std::fmt; + +use rustc_abi::ExternAbi; +use rustc_feature::Features; +use rustc_session::Session; +use rustc_session::parse::feature_err; +use rustc_span::symbol::sym; +use rustc_span::{Span, Symbol}; + +pub(crate) fn enabled_names(features: &rustc_feature::Features, span: Span) -> Vec<&'static str> { + rustc_abi::AbiDatas + .iter() + .filter(|data| extern_abi_enabled(features, span, data.abi).is_ok()) + .map(|d| d.name) + .collect() +} + +pub(crate) fn extern_abi_enabled( + features: &rustc_feature::Features, + span: Span, + abi: ExternAbi, +) -> Result<(), UnstableAbi> { + extern_abi_stability(abi).or_else(|unstable @ UnstableAbi { feature, .. }| { + if features.enabled(feature) || span.allows_unstable(feature) { + Ok(()) + } else { + Err(unstable) + } + }) +} + +#[allow(rustc::untranslatable_diagnostic)] +pub(crate) fn gate_unstable_abi(sess: &Session, features: &Features, span: Span, abi: ExternAbi) { + match extern_abi_enabled(features, span, abi) { + Ok(_) => (), + Err(unstable_abi) => { + let explain = unstable_abi.to_string(); + feature_err(sess, unstable_abi.feature, span, explain).emit(); + } + } +} + +pub(crate) struct UnstableAbi { + abi: ExternAbi, + feature: Symbol, + explain: GateReason, +} + +enum GateReason { + Experimental, + ImplDetail, +} + +impl fmt::Display for UnstableAbi { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let Self { abi, .. } = self; + let name = abi.to_string(); + let name = name.trim_matches('"'); + match self.explain { + GateReason::Experimental => { + write!(f, r#"the extern "{name}" ABI is experimental and subject to change"#) + } + GateReason::ImplDetail => { + write!( + f, + r#"the extern "{name}" ABI is an implementation detail and perma-unstable"# + ) + } + } + } +} + +pub(crate) fn extern_abi_stability(abi: ExternAbi) -> Result<(), UnstableAbi> { + match abi { + // stable ABIs + ExternAbi::Rust + | ExternAbi::C { .. } + | ExternAbi::Cdecl { .. } + | ExternAbi::Stdcall { .. } + | ExternAbi::Fastcall { .. } + | ExternAbi::Thiscall { .. } + | ExternAbi::Aapcs { .. } + | ExternAbi::Win64 { .. } + | ExternAbi::SysV64 { .. } + | ExternAbi::System { .. } + | ExternAbi::EfiApi => Ok(()), + // implementation details + ExternAbi::RustIntrinsic => { + Err(UnstableAbi { abi, feature: sym::intrinsics, explain: GateReason::ImplDetail }) + } + ExternAbi::Unadjusted => { + Err(UnstableAbi { abi, feature: sym::abi_unadjusted, explain: GateReason::ImplDetail }) + } + // experimental + ExternAbi::Vectorcall { .. } => Err(UnstableAbi { + abi, + feature: sym::abi_vectorcall, + explain: GateReason::Experimental, + }), + ExternAbi::RustCall => Err(UnstableAbi { + abi, + feature: sym::unboxed_closures, + explain: GateReason::Experimental, + }), + ExternAbi::RustCold => { + Err(UnstableAbi { abi, feature: sym::rust_cold_cc, explain: GateReason::Experimental }) + } + ExternAbi::GpuKernel => Err(UnstableAbi { + abi, + feature: sym::abi_gpu_kernel, + explain: GateReason::Experimental, + }), + ExternAbi::PtxKernel => { + Err(UnstableAbi { abi, feature: sym::abi_ptx, explain: GateReason::Experimental }) + } + ExternAbi::Msp430Interrupt => Err(UnstableAbi { + abi, + feature: sym::abi_msp430_interrupt, + explain: GateReason::Experimental, + }), + ExternAbi::X86Interrupt => Err(UnstableAbi { + abi, + feature: sym::abi_x86_interrupt, + explain: GateReason::Experimental, + }), + ExternAbi::AvrInterrupt | ExternAbi::AvrNonBlockingInterrupt => Err(UnstableAbi { + abi, + feature: sym::abi_avr_interrupt, + explain: GateReason::Experimental, + }), + ExternAbi::RiscvInterruptM | ExternAbi::RiscvInterruptS => Err(UnstableAbi { + abi, + feature: sym::abi_riscv_interrupt, + explain: GateReason::Experimental, + }), + ExternAbi::CCmseNonSecureCall => Err(UnstableAbi { + abi, + feature: sym::abi_c_cmse_nonsecure_call, + explain: GateReason::Experimental, + }), + ExternAbi::CCmseNonSecureEntry => Err(UnstableAbi { + abi, + feature: sym::cmse_nonsecure_entry, + explain: GateReason::Experimental, + }), + } +} diff --git a/compiler/rustc_ast_passes/src/feature_gate.rs b/compiler/rustc_ast_passes/src/feature_gate.rs index 13294efdaf6f..e5d8013058f6 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -1,9 +1,9 @@ use rustc_ast as ast; use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::{NodeId, PatKind, attr, token}; -use rustc_feature::{AttributeGate, BUILTIN_ATTRIBUTE_MAP, BuiltinAttribute, Features, GateIssue}; +use rustc_feature::{AttributeGate, BUILTIN_ATTRIBUTE_MAP, BuiltinAttribute, Features}; use rustc_session::Session; -use rustc_session::parse::{feature_err, feature_err_issue, feature_warn}; +use rustc_session::parse::{feature_err, feature_warn}; use rustc_span::source_map::Spanned; use rustc_span::{Span, Symbol, sym}; use thin_vec::ThinVec; @@ -72,35 +72,6 @@ struct PostExpansionVisitor<'a> { } impl<'a> PostExpansionVisitor<'a> { - #[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable - fn check_abi(&self, abi: ast::StrLit) { - let ast::StrLit { symbol_unescaped, span, .. } = abi; - - match rustc_abi::is_enabled(self.features, span, symbol_unescaped.as_str()) { - Ok(()) => (), - Err(rustc_abi::AbiDisabled::Unstable { feature, explain }) => { - feature_err_issue(&self.sess, feature, span, GateIssue::Language, explain).emit(); - } - Err(rustc_abi::AbiDisabled::Unrecognized) => { - if self.sess.opts.pretty.is_none_or(|ppm| ppm.needs_hir()) { - self.sess.dcx().span_delayed_bug( - span, - format!( - "unrecognized ABI not caught in lowering: {}", - symbol_unescaped.as_str() - ), - ); - } - } - } - } - - fn check_extern(&self, ext: ast::Extern) { - if let ast::Extern::Explicit(abi, _) = ext { - self.check_abi(abi); - } - } - /// Feature gate `impl Trait` inside `type Alias = $type_expr;`. fn check_impl_trait(&self, ty: &ast::Ty, in_associated_ty: bool) { struct ImplTraitVisitor<'a> { @@ -223,12 +194,9 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { fn visit_item(&mut self, i: &'a ast::Item) { match &i.kind { - ast::ItemKind::ForeignMod(foreign_module) => { - if let Some(abi) = foreign_module.abi { - self.check_abi(abi); - } + ast::ItemKind::ForeignMod(_foreign_module) => { + // handled during lowering } - ast::ItemKind::Struct(..) | ast::ItemKind::Enum(..) | ast::ItemKind::Union(..) => { for attr in attr::filter_by_name(&i.attrs, sym::repr) { for item in attr.meta_item_list().unwrap_or_else(ThinVec::new) { @@ -315,7 +283,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { match &ty.kind { ast::TyKind::BareFn(bare_fn_ty) => { // Function pointers cannot be `const` - self.check_extern(bare_fn_ty.ext); self.check_late_bound_lifetime_defs(&bare_fn_ty.generic_params); } ast::TyKind::Never => { @@ -418,9 +385,8 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { } fn visit_fn(&mut self, fn_kind: FnKind<'a>, span: Span, _: NodeId) { - if let Some(header) = fn_kind.header() { + if let Some(_header) = fn_kind.header() { // Stability of const fn methods are covered in `visit_assoc_item` below. - self.check_extern(header.ext); } if let FnKind::Closure(ast::ClosureBinder::For { generic_params, .. }, ..) = fn_kind { diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 03b3426fcec0..b243d9a28793 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -58,10 +58,7 @@ use crate::spec::crt_objects::CrtObjects; pub mod crt_objects; pub mod abi { - pub use rustc_abi::{ - AbiDisabled, AbiUnsupported, ExternAbi as Abi, all_names, enabled_names, is_enabled, - is_stable, lookup, - }; + pub use rustc_abi::{AbiUnsupported, ExternAbi as Abi, all_names, lookup}; } mod base; diff --git a/tests/ui/extern/fictional-abi.rs b/tests/ui/extern/fictional-abi.rs new file mode 100644 index 000000000000..60b24f8856ef --- /dev/null +++ b/tests/ui/extern/fictional-abi.rs @@ -0,0 +1,3 @@ +#![crate_type = "lib"] + +pub extern "fictional" fn lol() {} //~ ERROR: invalid ABI diff --git a/tests/ui/extern/fictional-abi.stderr b/tests/ui/extern/fictional-abi.stderr new file mode 100644 index 000000000000..9784a57776fc --- /dev/null +++ b/tests/ui/extern/fictional-abi.stderr @@ -0,0 +1,11 @@ +error[E0703]: invalid ABI: found `fictional` + --> $DIR/fictional-abi.rs:3:12 + | +LL | pub extern "fictional" fn lol() {} + | ^^^^^^^^^^^ invalid ABI + | + = note: invoke `rustc --print=calling-conventions` for a full list of supported calling conventions + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0703`.