Auto merge of #98827 - aDotInTheVoid:suggest-extern-block, r=nagisa

Suggest using block for `extern "abi" fn` with no body

`@rustbot` modify labels: +A-diagnostics
This commit is contained in:
bors 2022-07-07 04:18:47 +00:00
commit e78e747f53
12 changed files with 124 additions and 30 deletions

View File

@ -2667,13 +2667,16 @@ impl Item {
#[derive(Clone, Copy, Encodable, Decodable, Debug)]
pub enum Extern {
None,
Implicit,
Explicit(StrLit),
Implicit(Span),
Explicit(StrLit, Span),
}
impl Extern {
pub fn from_abi(abi: Option<StrLit>) -> Extern {
abi.map_or(Extern::Implicit, Extern::Explicit)
pub fn from_abi(abi: Option<StrLit>, span: Span) -> Extern {
match abi {
Some(name) => Extern::Explicit(name, span),
None => Extern::Implicit(span),
}
}
}

View File

@ -1272,8 +1272,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
pub(super) fn lower_extern(&mut self, ext: Extern) -> abi::Abi {
match ext {
Extern::None => abi::Abi::Rust,
Extern::Implicit => abi::Abi::FALLBACK,
Extern::Explicit(abi) => self.lower_abi(abi),
Extern::Implicit(_) => abi::Abi::FALLBACK,
Extern::Explicit(abi, _) => self.lower_abi(abi),
}
}

View File

@ -13,7 +13,9 @@ use rustc_ast::walk_list;
use rustc_ast::*;
use rustc_ast_pretty::pprust::{self, State};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{error_code, pluralize, struct_span_err, Applicability};
use rustc_errors::{
error_code, pluralize, struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed,
};
use rustc_parse::validate_attr;
use rustc_session::lint::builtin::{
DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, PATTERNS_IN_FNS_WITHOUT_BODY,
@ -476,6 +478,17 @@ impl<'a> AstValidator<'a> {
}
fn error_item_without_body(&self, sp: Span, ctx: &str, msg: &str, sugg: &str) {
self.error_item_without_body_with_help(sp, ctx, msg, sugg, |_| ());
}
fn error_item_without_body_with_help(
&self,
sp: Span,
ctx: &str,
msg: &str,
sugg: &str,
help: impl FnOnce(&mut DiagnosticBuilder<'_, ErrorGuaranteed>),
) {
let source_map = self.session.source_map();
let end = source_map.end_point(sp);
let replace_span = if source_map.span_to_snippet(end).map(|s| s == ";").unwrap_or(false) {
@ -483,15 +496,15 @@ impl<'a> AstValidator<'a> {
} else {
sp.shrink_to_hi()
};
self.err_handler()
.struct_span_err(sp, msg)
.span_suggestion(
replace_span,
&format!("provide a definition for the {}", ctx),
sugg,
Applicability::HasPlaceholders,
)
.emit();
let mut err = self.err_handler().struct_span_err(sp, msg);
err.span_suggestion(
replace_span,
&format!("provide a definition for the {}", ctx),
sugg,
Applicability::HasPlaceholders,
);
help(&mut err);
err.emit();
}
fn check_impl_item_provided<T>(&self, sp: Span, body: &Option<T>, ctx: &str, sugg: &str) {
@ -630,7 +643,8 @@ impl<'a> AstValidator<'a> {
match (fk.ctxt(), fk.header()) {
(Some(FnCtxt::Foreign), _) => return,
(Some(FnCtxt::Free), Some(header)) => match header.ext {
Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }) | Extern::Implicit
Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }, _)
| Extern::Implicit(_)
if matches!(header.unsafety, Unsafe::Yes(_)) =>
{
return;
@ -842,7 +856,7 @@ impl<'a> AstValidator<'a> {
.emit();
});
self.check_late_bound_lifetime_defs(&bfty.generic_params);
if let Extern::Implicit = bfty.ext {
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);
}
@ -1190,8 +1204,38 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
if body.is_none() {
let msg = "free function without a body";
self.error_item_without_body(item.span, "function", msg, " { <body> }");
let ext = sig.header.ext;
let f = |e: &mut DiagnosticBuilder<'_, _>| {
if let Extern::Implicit(start_span) | Extern::Explicit(_, start_span) = &ext
{
let start_suggestion = if let Extern::Explicit(abi, _) = ext {
format!("extern \"{}\" {{", abi.symbol_unescaped)
} else {
"extern {".to_owned()
};
let end_suggestion = " }".to_owned();
let end_span = item.span.shrink_to_hi();
e
.multipart_suggestion(
"if you meant to declare an externally defined function, use an `extern` block",
vec![(*start_span, start_suggestion), (end_span, end_suggestion)],
Applicability::MaybeIncorrect,
);
}
};
self.error_item_without_body_with_help(
item.span,
"function",
msg,
" { <body> }",
f,
);
}
self.visit_vis(&item.vis);
self.visit_ident(item.ident);
let kind =
@ -1556,7 +1600,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
if let FnKind::Fn(
_,
_,
FnSig { span: sig_span, header: FnHeader { ext: Extern::Implicit, .. }, .. },
FnSig { span: sig_span, header: FnHeader { ext: Extern::Implicit(_), .. }, .. },
_,
_,
_,

View File

@ -283,7 +283,7 @@ impl<'a> PostExpansionVisitor<'a> {
}
fn check_extern(&self, ext: ast::Extern, constness: ast::Const) {
if let ast::Extern::Explicit(abi) = ext {
if let ast::Extern::Explicit(abi, _) = ext {
self.check_abi(abi, constness);
}
}

View File

@ -1734,10 +1734,10 @@ impl<'a> State<'a> {
match header.ext {
ast::Extern::None => {}
ast::Extern::Implicit => {
ast::Extern::Implicit(_) => {
self.word_nbsp("extern");
}
ast::Extern::Explicit(abi) => {
ast::Extern::Explicit(abi, _) => {
self.word_nbsp("extern");
self.print_literal(&abi.as_lit());
self.nbsp();

View File

@ -1353,7 +1353,16 @@ impl<'a> Parser<'a> {
/// Parses `extern string_literal?`.
fn parse_extern(&mut self) -> Extern {
if self.eat_keyword(kw::Extern) { Extern::from_abi(self.parse_abi()) } else { Extern::None }
if self.eat_keyword(kw::Extern) {
let mut extern_span = self.prev_token.span;
let abi = self.parse_abi();
if let Some(abi) = abi {
extern_span = extern_span.to(abi.span);
}
Extern::from_abi(abi, extern_span)
} else {
Extern::None
}
}
/// Parses a string literal as an ABI spec.

6
src/test/ui/extern/not-in-block.rs vendored Normal file
View File

@ -0,0 +1,6 @@
#![crate_type = "lib"]
extern fn none_fn(x: bool) -> i32;
//~^ ERROR free function without a body
extern "C" fn c_fn(x: bool) -> i32;
//~^ ERROR free function without a body

32
src/test/ui/extern/not-in-block.stderr vendored Normal file
View File

@ -0,0 +1,32 @@
error: free function without a body
--> $DIR/not-in-block.rs:3:1
|
LL | extern fn none_fn(x: bool) -> i32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: provide a definition for the function
|
LL | extern fn none_fn(x: bool) -> i32 { <body> }
| ~~~~~~~~~~
help: if you meant to declare an externally defined function, use an `extern` block
|
LL | extern { fn none_fn(x: bool) -> i32; }
| ~~~~~~~~ +
error: free function without a body
--> $DIR/not-in-block.rs:5:1
|
LL | extern "C" fn c_fn(x: bool) -> i32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: provide a definition for the function
|
LL | extern "C" fn c_fn(x: bool) -> i32 { <body> }
| ~~~~~~~~~~
help: if you meant to declare an externally defined function, use an `extern` block
|
LL | extern "C" { fn c_fn(x: bool) -> i32; }
| ~~~~~~~~~~~~ +
error: aborting due to 2 previous errors

View File

@ -94,7 +94,7 @@ impl ExcessiveBools {
fn check_fn_sig(&self, cx: &EarlyContext<'_>, fn_sig: &FnSig, span: Span) {
match fn_sig.header.ext {
Extern::Implicit | Extern::Explicit(_) => return,
Extern::Implicit(_) | Extern::Explicit(_, _) => return,
Extern::None => (),
}

View File

@ -600,8 +600,8 @@ pub fn eq_ty(l: &Ty, r: &Ty) -> bool {
pub fn eq_ext(l: &Extern, r: &Extern) -> bool {
use Extern::*;
match (l, r) {
(None, None) | (Implicit, Implicit) => true,
(Explicit(l), Explicit(r)) => eq_str_lit(l, r),
(None, None) | (Implicit(_), Implicit(_)) => true,
(Explicit(l,_), Explicit(r,_)) => eq_str_lit(l, r),
_ => false,
}
}

View File

@ -148,7 +148,7 @@ impl<'a> Item<'a> {
Item {
unsafety: fm.unsafety,
abi: format_extern(
ast::Extern::from_abi(fm.abi),
ast::Extern::from_abi(fm.abi, DUMMY_SP),
config.force_explicit_abi(),
true,
),

View File

@ -138,8 +138,8 @@ pub(crate) fn format_extern(
) -> Cow<'static, str> {
let abi = match ext {
ast::Extern::None => "Rust".to_owned(),
ast::Extern::Implicit => "C".to_owned(),
ast::Extern::Explicit(abi) => abi.symbol_unescaped.to_string(),
ast::Extern::Implicit(_) => "C".to_owned(),
ast::Extern::Explicit(abi, _) => abi.symbol_unescaped.to_string(),
};
if abi == "Rust" && !is_mod {