Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse: recover on const fn() / async fn() #70421

Merged
merged 1 commit into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc_parse/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,7 @@ impl<'a> Parser<'a> {
}

/// Is the current token the start of an `FnHeader` / not a valid parse?
fn check_fn_front_matter(&mut self) -> bool {
pub(super) fn check_fn_front_matter(&mut self) -> bool {
// We use an over-approximation here.
// `const const`, `fn const` won't parse, but we're not stepping over other syntax either.
const QUALS: [Symbol; 4] = [kw::Const, kw::Async, kw::Unsafe, kw::Extern];
Expand All @@ -1521,7 +1521,7 @@ impl<'a> Parser<'a> {
/// FnQual = "const"? "async"? "unsafe"? Extern? ;
/// FnFrontMatter = FnQual? "fn" ;
/// ```
fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
let constness = self.parse_constness();
let asyncness = self.parse_asyncness();
let unsafety = self.parse_unsafety();
Expand Down
44 changes: 28 additions & 16 deletions src/librustc_parse/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,16 @@ impl<'a> Parser<'a> {
} else if self.eat_keyword(kw::Underscore) {
// A type to be inferred `_`
TyKind::Infer
} else if self.token_is_bare_fn_keyword() {
} else if self.check_fn_front_matter() {
// Function pointer type
self.parse_ty_bare_fn(Vec::new())?
self.parse_ty_bare_fn(lo, Vec::new())?
} else if self.check_keyword(kw::For) {
// Function pointer type or bound list (trait object type) starting with a poly-trait.
// `for<'lt> [unsafe] [extern "ABI"] fn (&'lt S) -> T`
// `for<'lt> Trait1<'lt> + Trait2 + 'a`
let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
if self.token_is_bare_fn_keyword() {
self.parse_ty_bare_fn(lifetime_defs)?
if self.check_fn_front_matter() {
self.parse_ty_bare_fn(lo, lifetime_defs)?
} else {
let path = self.parse_path(PathStyle::Type)?;
let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus();
Expand Down Expand Up @@ -291,13 +291,6 @@ impl<'a> Parser<'a> {
Ok(TyKind::Typeof(expr))
}

/// Is the current token one of the keywords that signals a bare function type?
fn token_is_bare_fn_keyword(&mut self) -> bool {
self.check_keyword(kw::Fn)
|| self.check_keyword(kw::Unsafe)
|| self.check_keyword(kw::Extern)
}

/// Parses a function pointer type (`TyKind::BareFn`).
/// ```
/// [unsafe] [extern "ABI"] fn (S) -> T
Expand All @@ -306,12 +299,31 @@ impl<'a> Parser<'a> {
/// | | | Return type
/// Function Style ABI Parameter types
/// ```
fn parse_ty_bare_fn(&mut self, generic_params: Vec<GenericParam>) -> PResult<'a, TyKind> {
let unsafety = self.parse_unsafety();
let ext = self.parse_extern()?;
self.expect_keyword(kw::Fn)?;
/// We actually parse `FnHeader FnDecl`, but we error on `const` and `async` qualifiers.
fn parse_ty_bare_fn(&mut self, lo: Span, params: Vec<GenericParam>) -> PResult<'a, TyKind> {
let ast::FnHeader { ext, unsafety, constness, asyncness } = self.parse_fn_front_matter()?;
let decl = self.parse_fn_decl(|_| false, AllowPlus::No)?;
Ok(TyKind::BareFn(P(BareFnTy { ext, unsafety, generic_params, decl })))
let whole_span = lo.to(self.prev_token.span);
if let ast::Const::Yes(span) = constness {
self.error_fn_ptr_bad_qualifier(whole_span, span, "const");
}
if let ast::Async::Yes { span, .. } = asyncness {
self.error_fn_ptr_bad_qualifier(whole_span, span, "async");
}
Ok(TyKind::BareFn(P(BareFnTy { ext, unsafety, generic_params: params, decl })))
}

/// Emit an error for the given bad function pointer qualifier.
fn error_fn_ptr_bad_qualifier(&self, span: Span, qual_span: Span, qual: &str) {
self.struct_span_err(span, &format!("an `fn` pointer type cannot be `{}`", qual))
.span_label(qual_span, format!("`{}` because of this", qual))
.span_suggestion_short(
qual_span,
&format!("remove the `{}` qualifier", qual),
String::new(),
Applicability::MaybeIncorrect,
)
.emit();
}
Comment on lines +316 to 327
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this to parser/diagnostics.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally prefer to keep parsing of various things more self-contained, and diagnostics.rs is larger than ty.rs in any case. :)


/// Parses an `impl B0 + ... + Bn` type.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
type A = extern::foo::bar; //~ ERROR expected `fn`, found `::`
type A = extern::foo::bar; //~ ERROR expected type, found keyword `extern`

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: expected `fn`, found `::`
--> $DIR/keyword-extern-as-identifier-type.rs:1:16
error: expected type, found keyword `extern`
--> $DIR/keyword-extern-as-identifier-type.rs:1:10
|
LL | type A = extern::foo::bar;
| ^^ expected `fn`
| ^^^^^^ expected type

error: aborting due to previous error

2 changes: 1 addition & 1 deletion src/test/ui/parser/issue-63116.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ error: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `;`
LL | impl W <s(f;Y(;]
| ^ expected one of 7 possible tokens

error: expected one of `!`, `&&`, `&`, `(`, `)`, `*`, `+`, `,`, `->`, `...`, `::`, `<`, `>`, `?`, `[`, `_`, `dyn`, `extern`, `fn`, `for`, `impl`, `unsafe`, lifetime, or path, found `;`
error: expected one of `!`, `&&`, `&`, `(`, `)`, `*`, `+`, `,`, `->`, `...`, `::`, `<`, `>`, `?`, `[`, `_`, `async`, `const`, `dyn`, `extern`, `fn`, `for`, `impl`, `unsafe`, lifetime, or path, found `;`
--> $DIR/issue-63116.rs:3:15
|
LL | impl W <s(f;Y(;]
Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/parser/recover-const-async-fn-ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// edition:2018

type T0 = const fn(); //~ ERROR an `fn` pointer type cannot be `const`
type T1 = const extern "C" fn(); //~ ERROR an `fn` pointer type cannot be `const`
type T2 = const unsafe extern fn(); //~ ERROR an `fn` pointer type cannot be `const`
type T3 = async fn(); //~ ERROR an `fn` pointer type cannot be `async`
type T4 = async extern fn(); //~ ERROR an `fn` pointer type cannot be `async`
type T5 = async unsafe extern "C" fn(); //~ ERROR an `fn` pointer type cannot be `async`
type T6 = const async unsafe extern "C" fn();
//~^ ERROR an `fn` pointer type cannot be `const`
//~| ERROR an `fn` pointer type cannot be `async`

type FT0 = for<'a> const fn(); //~ ERROR an `fn` pointer type cannot be `const`
type FT1 = for<'a> const extern "C" fn(); //~ ERROR an `fn` pointer type cannot be `const`
type FT2 = for<'a> const unsafe extern fn(); //~ ERROR an `fn` pointer type cannot be `const`
type FT3 = for<'a> async fn(); //~ ERROR an `fn` pointer type cannot be `async`
type FT4 = for<'a> async extern fn(); //~ ERROR an `fn` pointer type cannot be `async`
type FT5 = for<'a> async unsafe extern "C" fn(); //~ ERROR an `fn` pointer type cannot be `async`
type FT6 = for<'a> const async unsafe extern "C" fn();
//~^ ERROR an `fn` pointer type cannot be `const`
//~| ERROR an `fn` pointer type cannot be `async`

fn main() {
let _recovery_witness: () = 0; //~ ERROR mismatched types
}
155 changes: 155 additions & 0 deletions src/test/ui/parser/recover-const-async-fn-ptr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:3:11
|
LL | type T0 = const fn();
| -----^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:4:11
|
LL | type T1 = const extern "C" fn();
| -----^^^^^^^^^^^^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:5:11
|
LL | type T2 = const unsafe extern fn();
| -----^^^^^^^^^^^^^^^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:6:11
|
LL | type T3 = async fn();
| -----^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier
Comment on lines +28 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you feel it would make sense to add some explanation of what async does to functions in the first place? Something along the lines of

error: an `fn` pointer type cannot be `async`
  --> $DIR/recover-const-async-fn-ptr.rs:6:11
   |
LL | type T3 = async fn();
   |           -----^^^^^
   |           |
   |           `async` because of this
   |           help: remove the `async` qualifier
  = note: `async` functions only modify the return type of the function to be wrapped in a `std::future::Future<Output = ReturnType>`, an `async fn()` would be the same as `fn() -> Future<Output = ()>`

Copy link
Contributor Author

@Centril Centril Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would have to be fn() -> Box<dyn Future<Output = ()>> or some such as fn() -> impl A is not valid. I think I'd prefer to keep things simple now, as I did this PR mostly as a refactoring (improving diagnostics is secondary).


error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:7:11
|
LL | type T4 = async extern fn();
| -----^^^^^^^^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier

error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:8:11
|
LL | type T5 = async unsafe extern "C" fn();
| -----^^^^^^^^^^^^^^^^^^^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier

error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:9:11
|
LL | type T6 = const async unsafe extern "C" fn();
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:9:11
|
LL | type T6 = const async unsafe extern "C" fn();
| ^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier

error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:13:12
|
LL | type FT0 = for<'a> const fn();
| ^^^^^^^^-----^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:14:12
|
LL | type FT1 = for<'a> const extern "C" fn();
| ^^^^^^^^-----^^^^^^^^^^^^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:15:12
|
LL | type FT2 = for<'a> const unsafe extern fn();
| ^^^^^^^^-----^^^^^^^^^^^^^^^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:16:12
|
LL | type FT3 = for<'a> async fn();
| ^^^^^^^^-----^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier

error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:17:12
|
LL | type FT4 = for<'a> async extern fn();
| ^^^^^^^^-----^^^^^^^^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier

error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:18:12
|
LL | type FT5 = for<'a> async unsafe extern "C" fn();
| ^^^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier

error: an `fn` pointer type cannot be `const`
--> $DIR/recover-const-async-fn-ptr.rs:19:12
|
LL | type FT6 = for<'a> const async unsafe extern "C" fn();
| ^^^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| `const` because of this
| help: remove the `const` qualifier

error: an `fn` pointer type cannot be `async`
--> $DIR/recover-const-async-fn-ptr.rs:19:12
|
LL | type FT6 = for<'a> const async unsafe extern "C" fn();
| ^^^^^^^^^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^
| |
| `async` because of this
| help: remove the `async` qualifier

error[E0308]: mismatched types
--> $DIR/recover-const-async-fn-ptr.rs:24:33
|
LL | let _recovery_witness: () = 0;
| -- ^ expected `()`, found integer
| |
| expected due to this

error: aborting due to 17 previous errors

For more information about this error, try `rustc --explain E0308`.