From 8d7d488d3ba09ef5ac3fcdc12891b31a7d387f85 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 8 Jul 2021 21:58:05 +0200 Subject: [PATCH] Lint Abi in ast validation. --- Cargo.lock | 1 + compiler/rustc_ast_lowering/src/item.rs | 37 ++++++---------- compiler/rustc_ast_lowering/src/lib.rs | 26 +---------- compiler/rustc_ast_passes/Cargo.toml | 1 + .../rustc_ast_passes/src/ast_validation.rs | 43 ++++++++++++++++++- compiler/rustc_target/src/spec/abi.rs | 3 ++ 6 files changed, 61 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 74a316ed5f04e..f370a458fb35f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3556,6 +3556,7 @@ dependencies = [ "rustc_parse", "rustc_session", "rustc_span", + "rustc_target", "tracing", ] diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 014c240f11ee7..9766bd6751f63 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -283,7 +283,7 @@ impl<'hir> LoweringContext<'_, 'hir> { ); let sig = hir::FnSig { decl, - header: this.lower_fn_header(header, fn_sig_span, id), + header: this.lower_fn_header(header), span: this.lower_span(fn_sig_span), }; hir::ItemKind::Fn(sig, generics, body_id) @@ -295,17 +295,12 @@ impl<'hir> LoweringContext<'_, 'hir> { } ModKind::Unloaded => panic!("`mod` items should have been loaded by now"), }, - ItemKind::ForeignMod(ref fm) => { - if fm.abi.is_none() { - self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false }); - } - hir::ItemKind::ForeignMod { - abi: fm.abi.map_or(abi::Abi::C { unwind: false }, |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) => hir::ItemKind::ForeignMod { + abi: fm.abi.map_or(abi::Abi::FALLBACK, |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 asm) => { hir::ItemKind::GlobalAsm(self.lower_inline_asm(span, asm)) } @@ -811,7 +806,7 @@ impl<'hir> LoweringContext<'_, 'hir> { AssocItemKind::Fn(box FnKind(_, 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, i.id); + self.lower_method_sig(generics, sig, trait_item_def_id, false, None); (generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names))) } AssocItemKind::Fn(box FnKind(_, ref sig, ref generics, Some(ref body))) => { @@ -824,7 +819,6 @@ impl<'hir> LoweringContext<'_, 'hir> { trait_item_def_id, false, asyncness.opt_return_id(), - i.id, ); (generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id))) } @@ -901,7 +895,6 @@ 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)) @@ -1296,9 +1289,8 @@ impl<'hir> LoweringContext<'_, 'hir> { fn_def_id: LocalDefId, impl_trait_return_allow: bool, is_async: Option, - id: NodeId, ) -> (hir::Generics<'hir>, hir::FnSig<'hir>) { - let header = self.lower_fn_header(sig.header, sig.span, id); + let header = self.lower_fn_header(sig.header); let (generics, decl) = self.add_in_band_defs( generics, fn_def_id, @@ -1315,12 +1307,12 @@ impl<'hir> LoweringContext<'_, 'hir> { (generics, hir::FnSig { header, decl, span: self.lower_span(sig.span) }) } - fn lower_fn_header(&mut self, h: FnHeader, span: Span, id: NodeId) -> hir::FnHeader { + fn lower_fn_header(&mut self, h: FnHeader) -> 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, span, id), + abi: self.lower_extern(h.ext), } } @@ -1331,13 +1323,10 @@ impl<'hir> LoweringContext<'_, 'hir> { }) } - pub(super) fn lower_extern(&mut self, ext: Extern, span: Span, id: NodeId) -> abi::Abi { + pub(super) fn lower_extern(&mut self, ext: Extern) -> abi::Abi { match ext { Extern::None => abi::Abi::Rust, - Extern::Implicit => { - self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false }); - abi::Abi::C { unwind: false } - } + Extern::Implicit => abi::Abi::FALLBACK, Extern::Explicit(abi) => self.lower_abi(abi), } } diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index b71fcb7a349cc..89eced734e88c 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -53,7 +53,7 @@ use rustc_hir::definitions::{DefKey, DefPathData, Definitions}; use rustc_hir::intravisit; use rustc_hir::{ConstArg, GenericArg, InferKind, ParamName}; use rustc_index::vec::{Idx, IndexVec}; -use rustc_session::lint::builtin::{BARE_TRAIT_OBJECTS, MISSING_ABI}; +use rustc_session::lint::builtin::BARE_TRAIT_OBJECTS; use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer}; use rustc_session::utils::{FlattenNonterminals, NtToTokenstream}; use rustc_session::Session; @@ -62,7 +62,6 @@ use rustc_span::hygiene::ExpnId; use rustc_span::source_map::{respan, CachingSourceMapView, DesugaringKind}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{Span, DUMMY_SP}; -use rustc_target::spec::abi::Abi; use smallvec::{smallvec, SmallVec}; use std::collections::BTreeMap; @@ -1360,7 +1359,6 @@ 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, @@ -1368,7 +1366,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ImplTraitContext::disallowed(), ), unsafety: this.lower_unsafety(f.unsafety), - abi: this.lower_extern(f.ext, span, t.id), + abi: this.lower_extern(f.ext), decl: this.lower_fn_decl(&f.decl, None, false, None), param_names: this.lower_fn_params_to_names(&f.decl), })) @@ -2842,26 +2840,6 @@ 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>) -> Vec { diff --git a/compiler/rustc_ast_passes/Cargo.toml b/compiler/rustc_ast_passes/Cargo.toml index 6b931c598ed84..4a6eb80fb30ce 100644 --- a/compiler/rustc_ast_passes/Cargo.toml +++ b/compiler/rustc_ast_passes/Cargo.toml @@ -14,4 +14,5 @@ rustc_feature = { path = "../rustc_feature" } rustc_parse = { path = "../rustc_parse" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } +rustc_target = { path = "../rustc_target" } rustc_ast = { path = "../rustc_ast" } diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index c0ea710fdcb4e..9e51590dfb895 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -15,12 +15,13 @@ use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{error_code, pluralize, struct_span_err, Applicability}; use rustc_parse::validate_attr; -use rustc_session::lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY; +use rustc_session::lint::builtin::{MISSING_ABI, PATTERNS_IN_FNS_WITHOUT_BODY}; use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer}; use rustc_session::Session; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::Span; +use rustc_target::spec::abi; use std::mem; use std::ops::DerefMut; @@ -844,6 +845,10 @@ impl<'a> AstValidator<'a> { .emit(); }); self.check_late_bound_lifetime_defs(&bfty.generic_params); + if let Extern::Implicit = bfty.ext { + let sig_span = self.session.source_map().next_point(ty.span.shrink_to_lo()); + self.maybe_lint_missing_abi(sig_span, ty.id); + } } TyKind::TraitObject(ref bounds, ..) => { let mut any_lifetime_bounds = false; @@ -894,6 +899,26 @@ impl<'a> AstValidator<'a> { _ => {} } } + + fn maybe_lint_missing_abi(&mut self, span: Span, id: NodeId) { + // 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 + .session + .source_map() + .span_to_snippet(span) + .map(|snippet| snippet.starts_with("#[")) + .unwrap_or(true); + if !is_macro_callsite { + self.lint_buffer.buffer_lint_with_diagnostic( + MISSING_ABI, + id, + span, + "extern declarations without an explicit ABI are deprecated", + BuiltinLintDiagnostics::MissingAbi(span, abi::Abi::FALLBACK), + ) + } + } } /// Checks that generic parameters are in the correct order, @@ -1178,7 +1203,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { walk_list!(self, visit_attribute, &item.attrs); return; // Avoid visiting again. } - ItemKind::ForeignMod(ForeignMod { unsafety, .. }) => { + ItemKind::ForeignMod(ForeignMod { abi, unsafety, .. }) => { let old_item = mem::replace(&mut self.extern_mod, Some(item)); self.invalid_visibility( &item.vis, @@ -1187,6 +1212,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if let Unsafe::Yes(span) = unsafety { self.err_handler().span_err(span, "extern block cannot be declared unsafe"); } + if abi.is_none() { + self.maybe_lint_missing_abi(item.span, item.id); + } visit::walk_item(self, item); self.extern_mod = old_item; return; // Avoid visiting again. @@ -1526,6 +1554,17 @@ impl<'a> Visitor<'a> for AstValidator<'a> { .emit(); } + if let FnKind::Fn( + _, + _, + FnSig { span: sig_span, header: FnHeader { ext: Extern::Implicit, .. }, .. }, + _, + _, + ) = fk + { + self.maybe_lint_missing_abi(*sig_span, id); + } + // Functions without bodies cannot have patterns. if let FnKind::Fn(ctxt, _, sig, _, None) = fk { Self::check_decl_no_pat(&sig.decl, |span, ident, mut_ident| { diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index ee36090c54951..e3a2226eb9d15 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -87,6 +87,9 @@ pub fn all_names() -> Vec<&'static str> { } impl Abi { + /// Default ABI chosen for `extern fn` declarations without an explicit ABI. + pub const FALLBACK: Abi = Abi::C { unwind: false }; + #[inline] pub fn index(self) -> usize { // N.B., this ordering MUST match the AbiDatas array above.