Emit some extern "custom" errors in ast_validation:wq

Author: folkertdev

compiler/rustc_ast/src/ast.rs

@@ -3480,6 +3480,37 @@ impl FnHeader {
             || matches!(constness, Const::Yes(_))
             || !matches!(ext, Extern::None)
     }
+
+    pub fn span(&self) -> Option<Span> {
+        fn append(a: &mut Option<Span>, b: Span) {
+            *a = match a {
+                None => Some(b),
+                Some(x) => Some(x.to(b)),
+            }
+        }
+
+        let mut full_span = None;
+
+        match self.safety {
+            Safety::Unsafe(span) | Safety::Safe(span) => append(&mut full_span, span),
+            Safety::Default => {}
+        };
+
+        if let Some(coroutine_kind) = self.coroutine_kind {
+            append(&mut full_span, coroutine_kind.span());
+        }
+
+        if let Const::Yes(span) = self.constness {
+            append(&mut full_span, span);
+        }
+
+        match self.ext {
+            Extern::Implicit(span) | Extern::Explicit(_, span) => append(&mut full_span, span),
+            Extern::None => {}
+        }
+
+        full_span
+    }
 }
 
 impl Default for FnHeader {

compiler/rustc_ast_passes/messages.ftl

@@ -1,3 +1,16 @@
+ast_passes_abi_custom_invalid_signature =
+    invalid signature for `extern "custom"` function
+    .note = functions with the `"custom"` ABI cannot have any parameters or return type
+    .suggestion = remove the parameters and return type
+
+ast_passes_abi_custom_safe_foreign_function =
+    foreign functions with the `"custom"` ABI cannot be safe
+    .suggestion = remove the `safe` keyword from this definition
+
+ast_passes_abi_custom_safe_function =
+    functions with the `"custom"` ABI must be unsafe
+    .suggestion = add the `unsafe` keyword to function definition
+
 ast_passes_assoc_const_without_body =
     associated constant in `impl` without body
     .suggestion = provide a definition for the constant

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -18,6 +18,7 @@
 
 use std::mem;
 use std::ops::{Deref, DerefMut};
+use std::str::FromStr;
 
 use itertools::{Either, Itertools};
 use rustc_abi::ExternAbi;
@@ -81,6 +82,7 @@ struct AstValidator<'a> {
 
     /// Used to ban explicit safety on foreign items when the extern block is not marked as unsafe.
     extern_mod_safety: Option<Safety>,
+    extern_mod_abi: Option<ExternAbi>,
 
     lint_node_id: NodeId,
 
@@ -121,10 +123,17 @@ impl<'a> AstValidator<'a> {
         self.outer_trait_or_trait_impl = old;
     }
 
-    fn with_in_extern_mod(&mut self, extern_mod_safety: Safety, f: impl FnOnce(&mut Self)) {
-        let old = mem::replace(&mut self.extern_mod_safety, Some(extern_mod_safety));
+    fn with_in_extern_mod(
+        &mut self,
+        extern_mod_safety: Safety,
+        abi: Option<ExternAbi>,
+        f: impl FnOnce(&mut Self),
+    ) {
+        let old_safety = mem::replace(&mut self.extern_mod_safety, Some(extern_mod_safety));
+        let old_abi = mem::replace(&mut self.extern_mod_abi, abi);
         f(self);
-        self.extern_mod_safety = old;
+        self.extern_mod_safety = old_safety;
+        self.extern_mod_abi = old_abi;
     }
 
     fn with_tilde_const(
@@ -370,6 +379,49 @@ impl<'a> AstValidator<'a> {
         }
     }
 
+    /// An `extern "custom"` function must be unsafe, and must not have any parameters or return
+    /// type.
+    fn check_custom_abi(&self, ctxt: FnCtxt, ident: &Ident, sig: &FnSig) {
+        let dcx = self.dcx();
+
+        // An `extern "custom"` function must be unsafe.
+        match sig.header.safety {
+            Safety::Unsafe(_) => { /* all good */ }
+            Safety::Safe(safe_span) => {
+                let safe_span = safe_span.with_hi(safe_span.hi() + rustc_span::BytePos(1));
+                dcx.emit_err(errors::AbiCustomSafeForeignFunction { span: sig.span, safe_span });
+            }
+            Safety::Default => match ctxt {
+                FnCtxt::Foreign => { /* all good */ }
+                FnCtxt::Free | FnCtxt::Assoc(_) => {
+                    self.dcx().emit_err(errors::AbiCustomSafeFunction {
+                        span: sig.span,
+                        unsafe_span: sig.span.shrink_to_lo(),
+                    });
+                }
+            },
+        }
+
+        // An `extern "custom"` function must not have any parameters or return type.
+        let mut spans: Vec<_> = sig.decl.inputs.iter().map(|p| p.span).collect();
+        if let FnRetTy::Ty(ref ret_ty) = sig.decl.output {
+            spans.push(ret_ty.span);
+        }
+
+        if !spans.is_empty() {
+            let header_span = sig.header.span().unwrap_or(sig.span.shrink_to_lo());
+            let suggestion_span = sig.span.with_lo(header_span.hi());
+            let padding = if header_span.is_empty() { "" } else { " " };
+
+            self.dcx().emit_err(errors::AbiCustomInvalidSignature {
+                spans,
+                symbol: ident.name,
+                suggestion_span,
+                padding,
+            });
+        }
+    }
+
     /// This ensures that items can only be `unsafe` (or unmarked) outside of extern
     /// blocks.
     ///
@@ -1005,7 +1057,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 if abi.is_none() {
                     self.handle_missing_abi(*extern_span, item.id);
                 }
-                self.with_in_extern_mod(*safety, |this| {
+
+                let extern_abi = abi.and_then(|abi| ExternAbi::from_str(abi.symbol.as_str()).ok());
+                self.with_in_extern_mod(*safety, extern_abi, |this| {
                     visit::walk_item(this, item);
                 });
                 self.extern_mod_span = old_item;
@@ -1145,6 +1199,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 self.check_foreign_fn_bodyless(*ident, body.as_deref());
                 self.check_foreign_fn_headerless(sig.header);
                 self.check_foreign_item_ascii_only(*ident);
+                if self.extern_mod_abi == Some(ExternAbi::Custom) {
+                    self.check_custom_abi(FnCtxt::Foreign, ident, sig);
+                }
             }
             ForeignItemKind::TyAlias(box TyAlias {
                 defaultness,
@@ -1352,6 +1409,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             self.check_item_safety(span, safety);
         }
 
+        if let FnKind::Fn(ctxt, _, fun) = fk
+            && let Extern::Explicit(str_lit, _) = fun.sig.header.ext
+            && let Ok(ExternAbi::Custom) = ExternAbi::from_str(str_lit.symbol.as_str())
+        {
+            self.check_custom_abi(ctxt, &fun.ident, &fun.sig);
+        }
+
         self.check_c_variadic_type(fk);
 
         // Functions cannot both be `const async` or `const gen`
@@ -1703,6 +1767,7 @@ pub fn check_crate(
         outer_impl_trait_span: None,
         disallow_tilde_const: Some(TildeConstReason::Item),
         extern_mod_safety: None,
+        extern_mod_abi: None,
         lint_node_id: CRATE_NODE_ID,
         is_sdylib_interface,
         lint_buffer: lints,

compiler/rustc_ast_passes/src/errors.rs

@@ -824,3 +824,51 @@ pub(crate) struct MissingAbi {
     #[suggestion(code = "extern \"<abi>\"", applicability = "has-placeholders")]
     pub span: Span,
 }
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_abi_custom_safe_foreign_function)]
+pub(crate) struct AbiCustomSafeForeignFunction {
+    #[primary_span]
+    pub span: Span,
+
+    #[suggestion(
+        ast_passes_suggestion,
+        applicability = "maybe-incorrect",
+        code = "",
+        style = "verbose"
+    )]
+    pub safe_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_abi_custom_safe_function)]
+pub(crate) struct AbiCustomSafeFunction {
+    #[primary_span]
+    pub span: Span,
+
+    #[suggestion(
+        ast_passes_suggestion,
+        applicability = "maybe-incorrect",
+        code = "unsafe ",
+        style = "verbose"
+    )]
+    pub unsafe_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_abi_custom_invalid_signature)]
+#[note]
+pub(crate) struct AbiCustomInvalidSignature {
+    #[primary_span]
+    pub spans: Vec<Span>,
+
+    #[suggestion(
+        ast_passes_suggestion,
+        applicability = "maybe-incorrect",
+        code = "{padding}fn {symbol}()",
+        style = "verbose"
+    )]
+    pub suggestion_span: Span,
+    pub symbol: Symbol,
+    pub padding: &'static str,
+}

compiler/rustc_passes/messages.ftl

@@ -12,14 +12,6 @@ passes_abi_custom_clothed_function =
     functions with the `"custom"` ABI must be naked
     .suggestion = add the `#[unsafe(naked)]` attribute to this function
 
-passes_abi_custom_safe_foreign_function =
-    foreign functions with the `"custom"` ABI cannot be safe
-    .suggestion = remove the `safe` keyword from this definition
-
-passes_abi_custom_safe_function =
-    functions with the `"custom"` ABI must be unsafe
-    .suggestion = add the `unsafe` keyword this function definition
-
 passes_abi_invalid_attribute =
     `#[rustc_abi]` can only be applied to function items, type aliases, and associated functions
 passes_abi_ne =

compiler/rustc_passes/src/custom_abi.rs

@@ -1,16 +1,14 @@
 use rustc_abi::ExternAbi;
 use rustc_hir::def::DefKind;
-use rustc_hir::def_id::{LocalDefId, LocalModDefId};
+use rustc_hir::def_id::LocalModDefId;
 use rustc_hir::intravisit::Visitor;
-use rustc_hir::{self as hir, ExprKind, FnSig};
+use rustc_hir::{self as hir, ExprKind};
 use rustc_middle::hir::nested_filter::OnlyBodies;
 use rustc_middle::query::Providers;
 use rustc_middle::ty::{self, TyCtxt};
-use rustc_span::{BytePos, sym};
+use rustc_span::sym;
 
-use crate::errors::{
-    AbiCustomCall, AbiCustomClothedFunction, AbiCustomSafeForeignFunction, AbiCustomSafeFunction,
-};
+use crate::errors::{AbiCustomCall, AbiCustomClothedFunction};
 
 pub(crate) fn provide(providers: &mut Providers) {
     *providers = Providers { check_mod_custom_abi, ..*providers };
@@ -25,24 +23,6 @@ fn check_mod_custom_abi(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
     for def_id in items.definitions() {
         let def_kind = tcx.def_kind(def_id);
 
-        // An `extern "custom"` function cannot be marked as `safe`.
-        if let DefKind::ForeignMod = def_kind
-            && let hir::Node::Item(item) = tcx.hir_node_by_def_id(def_id)
-            && let hir::ItemKind::ForeignMod { abi: ExternAbi::Custom, items } = item.kind
-        {
-            for item in items {
-                let hir_id = item.id.hir_id();
-                if let hir::Node::ForeignItem(foreign_item) = tcx.hir_node(hir_id)
-                    && let hir::ForeignItemKind::Fn(sig, _, _) = foreign_item.kind
-                    && sig.header.is_safe()
-                {
-                    let len = "safe ".len() as u32;
-                    let safe_span = sig.span.shrink_to_lo().with_hi(sig.span.lo() + BytePos(len));
-                    tcx.dcx().emit_err(AbiCustomSafeForeignFunction { span: sig.span, safe_span });
-                }
-            }
-        }
-
         // An `extern "custom"` function cannot be a `const fn`, because `naked_asm!` cannot be
         // evaluated at compile time, and `extern` blocks cannot declare `const fn` functions.
         // Therefore, to find all calls to `extern "custom"` functions, it suffices to traverse
@@ -51,56 +31,34 @@ fn check_mod_custom_abi(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
             continue;
         }
 
-        let body = match tcx.hir_node_by_def_id(def_id) {
+        let (sig, body) = match tcx.hir_node_by_def_id(def_id) {
             hir::Node::Item(hir::Item {
                 kind: hir::ItemKind::Fn { sig, body: body_id, .. },
                 ..
             })
             | hir::Node::ImplItem(hir::ImplItem {
                 kind: hir::ImplItemKind::Fn(sig, body_id),
                 ..
-            }) => {
-                check_signature(tcx, def_id, sig, true);
-                tcx.hir_body(*body_id)
-            }
-            hir::Node::TraitItem(hir::TraitItem {
-                kind: hir::TraitItemKind::Fn(sig, trait_fn),
+            })
+            | hir::Node::TraitItem(hir::TraitItem {
+                kind: hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)),
                 ..
-            }) => match trait_fn {
-                hir::TraitFn::Required(_) => {
-                    check_signature(tcx, def_id, sig, false);
-                    continue;
-                }
-                hir::TraitFn::Provided(body_id) => {
-                    check_signature(tcx, def_id, sig, true);
-                    tcx.hir_body(*body_id)
-                }
-            },
+            }) => (sig, tcx.hir_body(*body_id)),
             _ => continue,
         };
 
-        let mut visitor = CheckCustomAbi { tcx };
-        visitor.visit_body(body);
-    }
-}
-
-fn check_signature<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, sig: &FnSig<'tcx>, has_body: bool) {
-    if sig.header.abi == ExternAbi::Custom {
-        // Function definitions that use `extern "custom"` must be naked functions.
-        if has_body && !tcx.has_attr(def_id, sym::naked) {
-            tcx.dcx().emit_err(AbiCustomClothedFunction {
-                span: sig.span,
-                naked_span: sig.span.shrink_to_lo(),
-            });
+        if sig.header.abi == ExternAbi::Custom {
+            // Function definitions that use `extern "custom"` must be naked functions.
+            if !tcx.has_attr(def_id, sym::naked) {
+                tcx.dcx().emit_err(AbiCustomClothedFunction {
+                    span: sig.span,
+                    naked_span: sig.span.shrink_to_lo(),
+                });
+            }
         }
 
-        // Function definitions that use `extern "custom"` must unsafe.
-        if sig.header.is_safe() {
-            tcx.dcx().emit_err(AbiCustomSafeFunction {
-                span: sig.span,
-                unsafe_span: sig.span.shrink_to_lo(),
-            });
-        }
+        let mut visitor = CheckCustomAbi { tcx };
+        visitor.visit_body(body);
     }
 }