Auto merge of rust-lang#76219 - Mark-Simulacrum:extern-require-abi, r=estebank

Add allow-by-default lint on implicit ABI in extern function pointers and items

This adds a new lint, missing_abi, which lints on omitted ABIs on extern blocks, function declarations, and function pointers.

It is currently not emitting the best possible diagnostics -- we need to track the span of "extern" at least or do some heuristic searching based on the available spans -- but seems good enough for an initial pass than can be expanded in future PRs.

This is a pretty large PR, but mostly due to updating a large number of tests to include ABIs; I can split that into a separate PR if it would be helpful, but test updates are already in dedicated commits.

Author: bors

Cargo.lock

@@ -3896,6 +3896,7 @@ dependencies = [
  "rustc_macros",
  "rustc_serialize",
  "rustc_span",
+ "rustc_target",
  "tracing",
 ]
 

compiler/rustc_ast_lowering/src/item.rs

@@ -310,19 +310,24 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     );
                     let sig = hir::FnSig {
                         decl,
-                        header: this.lower_fn_header(header),
+                        header: this.lower_fn_header(header, fn_sig_span, id),
                         span: fn_sig_span,
                     };
                     hir::ItemKind::Fn(sig, generics, body_id)
                 })
             }
             ItemKind::Mod(ref m) => hir::ItemKind::Mod(self.lower_mod(m)),
-            ItemKind::ForeignMod(ref fm) => hir::ItemKind::ForeignMod {
-                abi: fm.abi.map_or(abi::Abi::C, |abi| self.lower_abi(abi)),
-                items: self
-                    .arena
-                    .alloc_from_iter(fm.items.iter().map(|x| self.lower_foreign_item_ref(x))),
-            },
+            ItemKind::ForeignMod(ref fm) => {
+                if fm.abi.is_none() {
+                    self.maybe_lint_missing_abi(span, id, abi::Abi::C);
+                }
+                hir::ItemKind::ForeignMod {
+                    abi: fm.abi.map_or(abi::Abi::C, |abi| self.lower_abi(abi)),
+                    items: self
+                        .arena
+                        .alloc_from_iter(fm.items.iter().map(|x| self.lower_foreign_item_ref(x))),
+                }
+            }
             ItemKind::GlobalAsm(ref ga) => hir::ItemKind::GlobalAsm(self.lower_global_asm(ga)),
             ItemKind::TyAlias(_, ref gen, _, Some(ref ty)) => {
                 // We lower
@@ -801,13 +806,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
             AssocItemKind::Fn(_, ref sig, ref generics, None) => {
                 let names = self.lower_fn_params_to_names(&sig.decl);
                 let (generics, sig) =
-                    self.lower_method_sig(generics, sig, trait_item_def_id, false, None);
+                    self.lower_method_sig(generics, sig, trait_item_def_id, false, None, i.id);
                 (generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names)))
             }
             AssocItemKind::Fn(_, ref sig, ref generics, Some(ref body)) => {
                 let body_id = self.lower_fn_body_block(i.span, &sig.decl, Some(body));
                 let (generics, sig) =
-                    self.lower_method_sig(generics, sig, trait_item_def_id, false, None);
+                    self.lower_method_sig(generics, sig, trait_item_def_id, false, None, i.id);
                 (generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)))
             }
             AssocItemKind::TyAlias(_, ref generics, ref bounds, ref default) => {
@@ -877,6 +882,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     impl_item_def_id,
                     impl_trait_return_allow,
                     asyncness.opt_return_id(),
+                    i.id,
                 );
 
                 (generics, hir::ImplItemKind::Fn(sig, body_id))
@@ -1270,8 +1276,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
         fn_def_id: LocalDefId,
         impl_trait_return_allow: bool,
         is_async: Option<NodeId>,
+        id: NodeId,
     ) -> (hir::Generics<'hir>, hir::FnSig<'hir>) {
-        let header = self.lower_fn_header(sig.header);
+        let header = self.lower_fn_header(sig.header, sig.span, id);
         let (generics, decl) = self.add_in_band_defs(
             generics,
             fn_def_id,
@@ -1288,12 +1295,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
         (generics, hir::FnSig { header, decl, span: sig.span })
     }
 
-    fn lower_fn_header(&mut self, h: FnHeader) -> hir::FnHeader {
+    fn lower_fn_header(&mut self, h: FnHeader, span: Span, id: NodeId) -> hir::FnHeader {
         hir::FnHeader {
             unsafety: self.lower_unsafety(h.unsafety),
             asyncness: self.lower_asyncness(h.asyncness),
             constness: self.lower_constness(h.constness),
-            abi: self.lower_extern(h.ext),
+            abi: self.lower_extern(h.ext, span, id),
         }
     }
 
@@ -1304,10 +1311,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
         })
     }
 
-    pub(super) fn lower_extern(&mut self, ext: Extern) -> abi::Abi {
+    pub(super) fn lower_extern(&mut self, ext: Extern, span: Span, id: NodeId) -> abi::Abi {
         match ext {
             Extern::None => abi::Abi::Rust,
-            Extern::Implicit => abi::Abi::C,
+            Extern::Implicit => {
+                self.maybe_lint_missing_abi(span, id, abi::Abi::C);
+                abi::Abi::C
+            }
             Extern::Explicit(abi) => self.lower_abi(abi),
         }
     }

compiler/rustc_ast_lowering/src/lib.rs

@@ -53,13 +53,15 @@ use rustc_hir::definitions::{DefKey, DefPathData, Definitions};
 use rustc_hir::intravisit;
 use rustc_hir::{ConstArg, GenericArg, ParamName};
 use rustc_index::vec::{Idx, IndexVec};
-use rustc_session::lint::{builtin::BARE_TRAIT_OBJECTS, BuiltinLintDiagnostics, LintBuffer};
+use rustc_session::lint::builtin::{BARE_TRAIT_OBJECTS, MISSING_ABI};
+use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer};
 use rustc_session::parse::ParseSess;
 use rustc_session::Session;
 use rustc_span::hygiene::ExpnId;
 use rustc_span::source_map::{respan, DesugaringKind};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::Span;
+use rustc_target::spec::abi::Abi;
 
 use smallvec::{smallvec, SmallVec};
 use std::collections::BTreeMap;
@@ -1288,14 +1290,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             }
             TyKind::BareFn(ref f) => self.with_in_scope_lifetime_defs(&f.generic_params, |this| {
                 this.with_anonymous_lifetime_mode(AnonymousLifetimeMode::PassThrough, |this| {
+                    let span = this.sess.source_map().next_point(t.span.shrink_to_lo());
                     hir::TyKind::BareFn(this.arena.alloc(hir::BareFnTy {
                         generic_params: this.lower_generic_params(
                             &f.generic_params,
                             &NodeMap::default(),
                             ImplTraitContext::disallowed(),
                         ),
                         unsafety: this.lower_unsafety(f.unsafety),
-                        abi: this.lower_extern(f.ext),
+                        abi: this.lower_extern(f.ext, span, t.id),
                         decl: this.lower_fn_decl(&f.decl, None, false, None),
                         param_names: this.lower_fn_params_to_names(&f.decl),
                     }))
@@ -2777,6 +2780,26 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             )
         }
     }
+
+    fn maybe_lint_missing_abi(&mut self, span: Span, id: NodeId, default: Abi) {
+        // FIXME(davidtwco): This is a hack to detect macros which produce spans of the
+        // call site which do not have a macro backtrace. See #61963.
+        let is_macro_callsite = self
+            .sess
+            .source_map()
+            .span_to_snippet(span)
+            .map(|snippet| snippet.starts_with("#["))
+            .unwrap_or(true);
+        if !is_macro_callsite {
+            self.resolver.lint_buffer().buffer_lint_with_diagnostic(
+                MISSING_ABI,
+                id,
+                span,
+                "extern declarations without an explicit ABI are deprecated",
+                BuiltinLintDiagnostics::MissingAbi(span, default),
+            )
+        }
+    }
 }
 
 fn body_ids(bodies: &BTreeMap<hir::BodyId, hir::Body<'_>>) -> Vec<hir::BodyId> {

compiler/rustc_error_codes/src/error_codes/E0044.md

@@ -3,13 +3,13 @@ You cannot use type or const parameters on foreign items.
 Example of erroneous code:
 
 ```compile_fail,E0044
-extern { fn some_func<T>(x: T); }
+extern "C" { fn some_func<T>(x: T); }
 ```
 
 To fix this, replace the generic parameter with the specializations that you
 need:
 
 ```
-extern { fn some_func_i32(x: i32); }
-extern { fn some_func_i64(x: i64); }
+extern "C" { fn some_func_i32(x: i32); }
+extern "C" { fn some_func_i64(x: i64); }
 ```

compiler/rustc_error_codes/src/error_codes/E0130.md

@@ -3,7 +3,7 @@ A pattern was declared as an argument in a foreign function declaration.
 Erroneous code example:
 
 ```compile_fail,E0130
-extern {
+extern "C" {
     fn foo((a, b): (u32, u32)); // error: patterns aren't allowed in foreign
                                 //        function declarations
 }
@@ -17,15 +17,15 @@ struct SomeStruct {
     b: u32,
 }
 
-extern {
+extern "C" {
     fn foo(s: SomeStruct); // ok!
 }
 ```
 
 Or:
 
 ```
-extern {
+extern "C" {
     fn foo(a: (u32, u32)); // ok!
 }
 ```

compiler/rustc_error_codes/src/error_codes/E0454.md

@@ -3,13 +3,13 @@ A link name was given with an empty name.
 Erroneous code example:
 
 ```compile_fail,E0454
-#[link(name = "")] extern {}
+#[link(name = "")] extern "C" {}
 // error: `#[link(name = "")]` given with empty name
 ```
 
 The rust compiler cannot link to an external library if you don't give it its
 name. Example:
 
 ```no_run
-#[link(name = "some_lib")] extern {} // ok!
+#[link(name = "some_lib")] extern "C" {} // ok!
 ```

compiler/rustc_error_codes/src/error_codes/E0455.md

@@ -4,15 +4,15 @@ as frameworks are specific to that operating system.
 Erroneous code example:
 
 ```ignore (should-compile_fail-but-cannot-doctest-conditionally-without-macos)
-#[link(name = "FooCoreServices", kind = "framework")] extern {}
+#[link(name = "FooCoreServices", kind = "framework")] extern "C" {}
 // OS used to compile is Linux for example
 ```
 
 To solve this error you can use conditional compilation:
 
 ```
 #[cfg_attr(target="macos", link(name = "FooCoreServices", kind = "framework"))]
-extern {}
+extern "C" {}
 ```
 
 Learn more in the [Conditional Compilation][conditional-compilation] section

compiler/rustc_error_codes/src/error_codes/E0458.md

@@ -3,7 +3,7 @@ An unknown "kind" was specified for a link attribute.
 Erroneous code example:
 
 ```compile_fail,E0458
-#[link(kind = "wonderful_unicorn")] extern {}
+#[link(kind = "wonderful_unicorn")] extern "C" {}
 // error: unknown kind: `wonderful_unicorn`
 ```
 

compiler/rustc_error_codes/src/error_codes/E0459.md

@@ -3,13 +3,13 @@ A link was used without a name parameter.
 Erroneous code example:
 
 ```compile_fail,E0459
-#[link(kind = "dylib")] extern {}
+#[link(kind = "dylib")] extern "C" {}
 // error: `#[link(...)]` specified without `name = "foo"`
 ```
 
 Please add the name parameter to allow the rust compiler to find the library
 you want. Example:
 
 ```no_run
-#[link(kind = "dylib", name = "some_lib")] extern {} // ok!
+#[link(kind = "dylib", name = "some_lib")] extern "C" {} // ok!
 ```

compiler/rustc_error_codes/src/error_codes/E0617.md

@@ -3,7 +3,7 @@ Attempted to pass an invalid type of variable into a variadic function.
 Erroneous code example:
 
 ```compile_fail,E0617
-extern {
+extern "C" {
     fn printf(c: *const i8, ...);
 }
 
@@ -21,7 +21,7 @@ to import from `std::os::raw`).
 In this case, `c_double` has the same size as `f64` so we can use it directly:
 
 ```no_run
-# extern {
+# extern "C" {
 #     fn printf(c: *const i8, ...);
 # }
 unsafe {

compiler/rustc_error_codes/src/error_codes/E0633.md

@@ -6,7 +6,7 @@ Erroneous code example:
 #![feature(unwind_attributes)]
 
 #[unwind()] // error: expected one argument
-pub extern fn something() {}
+pub extern "C" fn something() {}
 
 fn main() {}
 ```

compiler/rustc_error_codes/src/error_codes/E0724.md

@@ -18,7 +18,7 @@ the function inside of an `extern` block.
 ```
 #![feature(ffi_returns_twice)]
 
-extern {
+extern "C" {
    #[ffi_returns_twice] // ok!
    pub fn foo();
 }

compiler/rustc_lint/src/context.rs

@@ -600,6 +600,10 @@ pub trait LintContext: Sized {
                 BuiltinLintDiagnostics::PatternsInFnsWithoutBody(span, ident) => {
                     db.span_suggestion(span, "remove `mut` from the parameter", ident.to_string(), Applicability::MachineApplicable);
                 }
+                BuiltinLintDiagnostics::MissingAbi(span, default_abi) => {
+                    db.span_label(span, "ABI should be specified here");
+                    db.help(&format!("the default ABI is {}", default_abi.name()));
+                }
             }
             // Rewrap `db`, and pass control to the user.
             decorate(LintDiagnosticBuilder::new(db));

compiler/rustc_lint_defs/Cargo.toml

@@ -11,3 +11,4 @@ rustc_data_structures = { path = "../rustc_data_structures" }
 rustc_span = { path = "../rustc_span" }
 rustc_serialize = { path = "../rustc_serialize" }
 rustc_macros = { path = "../rustc_macros" }
+rustc_target = { path = "../rustc_target" }

compiler/rustc_lint_defs/src/builtin.rs

@@ -2917,6 +2917,7 @@ declare_lint_pass! {
         FUNCTION_ITEM_REFERENCES,
         USELESS_DEPRECATED,
         UNSUPPORTED_NAKED_FUNCTIONS,
+        MISSING_ABI,
     ]
 }
 
@@ -2944,3 +2945,28 @@ declare_lint! {
 }
 
 declare_lint_pass!(UnusedDocComment => [UNUSED_DOC_COMMENTS]);
+
+declare_lint! {
+    /// The `missing_abi` lint detects cases where the ABI is omitted from
+    /// extern declarations.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(missing_abi)]
+    ///
+    /// extern fn foo() {}
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Historically, Rust implicitly selected C as the ABI for extern
+    /// declarations. We expect to add new ABIs, like `C-unwind`, in the future,
+    /// though this has not yet happened, and especially with their addition
+    /// seeing the ABI easily will make code review easier.
+    pub MISSING_ABI,
+    Allow,
+    "No declared ABI for extern declaration"
+}

compiler/rustc_lint_defs/src/lib.rs

@@ -6,6 +6,7 @@ use rustc_ast::node_id::{NodeId, NodeMap};
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
 use rustc_span::edition::Edition;
 use rustc_span::{sym, symbol::Ident, MultiSpan, Span, Symbol};
+use rustc_target::spec::abi::Abi;
 
 pub mod builtin;
 
@@ -252,6 +253,7 @@ pub enum BuiltinLintDiagnostics {
     UnusedImports(String, Vec<(Span, String)>),
     RedundantImport(Vec<(Span, bool)>, Ident),
     DeprecatedMacro(Option<Symbol>, Span),
+    MissingAbi(Span, Abi),
     UnusedDocComment(Span),
     PatternsInFnsWithoutBody(Span, Ident),
 }

compiler/rustc_llvm/src/lib.rs

@@ -38,7 +38,7 @@ pub fn initialize_available_targets() {
         ($cfg:meta, $($method:ident),*) => { {
             #[cfg($cfg)]
             fn init() {
-                extern {
+                extern "C" {
                     $(fn $method();)*
                 }
                 unsafe {