Skip to content

Provide a better error and a suggestion for Fn traits with lifetime params #104531

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

Merged
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
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2423,7 +2423,7 @@ impl<'a> Parser<'a> {
}

/// Parses the parameter list of a function, including the `(` and `)` delimiters.
fn parse_fn_params(&mut self, req_name: ReqName) -> PResult<'a, Vec<Param>> {
pub(super) fn parse_fn_params(&mut self, req_name: ReqName) -> PResult<'a, Vec<Param>> {
let mut first_param = true;
// Parse the arguments, starting out with `self` being allowed...
let (mut params, _) = self.parse_paren_comma_seq(|p| {
Expand Down
95 changes: 93 additions & 2 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -933,15 +933,20 @@ impl<'a> Parser<'a> {
has_parens: bool,
modifiers: BoundModifiers,
) -> PResult<'a, GenericBound> {
let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
let path = if self.token.is_keyword(kw::Fn)
let mut lifetime_defs = self.parse_late_bound_lifetime_defs()?;
let mut path = if self.token.is_keyword(kw::Fn)
&& self.look_ahead(1, |tok| tok.kind == TokenKind::OpenDelim(Delimiter::Parenthesis))
&& let Some(path) = self.recover_path_from_fn()
{
path
} else {
self.parse_path(PathStyle::Type)?
};

if self.may_recover() && self.token == TokenKind::OpenDelim(Delimiter::Parenthesis) {
self.recover_fn_trait_with_lifetime_params(&mut path, &mut lifetime_defs)?;
}

if has_parens {
if self.token.is_like_plus() {
// Someone has written something like `&dyn (Trait + Other)`. The correct code
Expand Down Expand Up @@ -1016,6 +1021,92 @@ impl<'a> Parser<'a> {
}
}

/// Recover from `Fn`-family traits (Fn, FnMut, FnOnce) with lifetime arguments
/// (e.g. `FnOnce<'a>(&'a str) -> bool`). Up to generic arguments have already
/// been eaten.
fn recover_fn_trait_with_lifetime_params(
&mut self,
fn_path: &mut ast::Path,
lifetime_defs: &mut Vec<GenericParam>,
) -> PResult<'a, ()> {
let fn_path_segment = fn_path.segments.last_mut().unwrap();
let generic_args = if let Some(p_args) = &fn_path_segment.args {
p_args.clone().into_inner()
} else {
Comment on lines +1033 to +1035
Copy link
Contributor

Choose a reason for hiding this comment

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

This can now be

Suggested change
let generic_args = if let Some(p_args) = &fn_path_segment.args {
p_args.clone().into_inner()
} else {
let Some(p_args) = &fn_path_segment.args {
p_args.clone().into_inner()
} else {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this valid syntax? I got the following error:

error: expected one of `(`, `.`, `::`, `;`, `?`, `else`, or an operator, found `{`
    --> compiler/rustc_parse/src/parser/ty.rs:1033:50
     |
1033 |         let Some(p_args) = &fn_path_segment.args {
     |                                                  ^ expected one of 7 possible tokens

error: could not compile `rustc_parse` due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `rustc_parse` due to previous error

Copy link
Member

@fmease fmease Dec 29, 2022

Choose a reason for hiding this comment

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

With let/else syntax, it's

let Some(p_args) = &fn_path_segment.args else {
    // ... ... ...
    return Ok(());
};
let p_args = p_args.clone().into_inner();

or

let Some(p_args) = fn_path_segment.args.cloned().map(|arg| arg) else {
    // ... ... ...
    return Ok(());
};

or

let Some(p_args) = fn_path.segments
    .last_mut()
    .unwrap()
    .args
    .cloned()
    .map(|arg| arg)
else {
    // ... ... ...
    return Ok(());
};

or something similar at your choosing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yeah, I mistyped what I meant. Thanks @fmease for clarifying it

// Normally it wouldn't come here because the upstream should have parsed
// generic parameters (otherwise it's impossible to call this function).
return Ok(());
};
let lifetimes =
if let ast::GenericArgs::AngleBracketed(ast::AngleBracketedArgs { span: _, args }) =
&generic_args
{
args.into_iter()
.filter_map(|arg| {
if let ast::AngleBracketedArg::Arg(generic_arg) = arg
&& let ast::GenericArg::Lifetime(lifetime) = generic_arg {
Some(lifetime)
} else {
None
}
})
.collect()
} else {
Vec::new()
};
// Only try to recover if the trait has lifetime params.
if lifetimes.is_empty() {
return Ok(());
}

// Parse `(T, U) -> R`.
let inputs_lo = self.token.span;
let inputs: Vec<_> =
self.parse_fn_params(|_| false)?.into_iter().map(|input| input.ty).collect();
let inputs_span = inputs_lo.to(self.prev_token.span);
let output = self.parse_ret_ty(AllowPlus::No, RecoverQPath::No, RecoverReturnSign::No)?;
let args = ast::ParenthesizedArgs {
span: fn_path_segment.span().to(self.prev_token.span),
inputs,
inputs_span,
output,
}
.into();
*fn_path_segment =
ast::PathSegment { ident: fn_path_segment.ident, args, id: ast::DUMMY_NODE_ID };

// Convert parsed `<'a>` in `Fn<'a>` into `for<'a>`.
let mut generic_params = lifetimes
.iter()
.map(|lt| GenericParam {
id: lt.id,
ident: lt.ident,
attrs: ast::AttrVec::new(),
bounds: Vec::new(),
is_placeholder: false,
kind: ast::GenericParamKind::Lifetime,
colon_span: None,
})
.collect::<Vec<GenericParam>>();
lifetime_defs.append(&mut generic_params);

let generic_args_span = generic_args.span();
let mut err =
self.struct_span_err(generic_args_span, "`Fn` traits cannot take lifetime parameters");
let snippet = format!(
"for<{}> ",
lifetimes.iter().map(|lt| lt.ident.as_str()).intersperse(", ").collect::<String>(),
);
let before_fn_path = fn_path.span.shrink_to_lo();
err.multipart_suggestion(
"consider using a higher-ranked trait bound instead",
vec![(generic_args_span, "".to_owned()), (before_fn_path, snippet)],
Applicability::MaybeIncorrect,
)
.emit();
Ok(())
}

pub(super) fn check_lifetime(&mut self) -> bool {
self.expected_tokens.push(TokenType::Lifetime);
self.token.is_lifetime()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Test that Fn-family traits with lifetime parameters shouldn't compile and
// we suggest the usage of higher-rank trait bounds instead.

fn fa(_: impl Fn<'a>(&'a str) -> bool) {}
//~^ ERROR `Fn` traits cannot take lifetime parameters

fn fb(_: impl FnMut<'a, 'b>(&'a str, &'b str) -> bool) {}
//~^ ERROR `Fn` traits cannot take lifetime parameters

fn fc(_: impl std::fmt::Display + FnOnce<'a>(&'a str) -> bool + std::fmt::Debug) {}
//~^ ERROR `Fn` traits cannot take lifetime parameters

use std::ops::Fn as AliasedFn;
fn fd(_: impl AliasedFn<'a>(&'a str) -> bool) {}
//~^ ERROR `Fn` traits cannot take lifetime parameters

fn fe<F>(_: F) where F: Fn<'a>(&'a str) -> bool {}
//~^ ERROR `Fn` traits cannot take lifetime parameters

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
error: `Fn` traits cannot take lifetime parameters
--> $DIR/hrtb-malformed-lifetime-generics.rs:4:17
|
LL | fn fa(_: impl Fn<'a>(&'a str) -> bool) {}
| ^^^^
|
help: consider using a higher-ranked trait bound instead
|
LL - fn fa(_: impl Fn<'a>(&'a str) -> bool) {}
LL + fn fa(_: impl for<'a> Fn(&'a str) -> bool) {}
|

error: `Fn` traits cannot take lifetime parameters
--> $DIR/hrtb-malformed-lifetime-generics.rs:7:20
|
LL | fn fb(_: impl FnMut<'a, 'b>(&'a str, &'b str) -> bool) {}
| ^^^^^^^^
|
help: consider using a higher-ranked trait bound instead
|
LL - fn fb(_: impl FnMut<'a, 'b>(&'a str, &'b str) -> bool) {}
LL + fn fb(_: impl for<'a, 'b> FnMut(&'a str, &'b str) -> bool) {}
|

error: `Fn` traits cannot take lifetime parameters
--> $DIR/hrtb-malformed-lifetime-generics.rs:10:41
|
LL | fn fc(_: impl std::fmt::Display + FnOnce<'a>(&'a str) -> bool + std::fmt::Debug) {}
| ^^^^
|
help: consider using a higher-ranked trait bound instead
|
LL - fn fc(_: impl std::fmt::Display + FnOnce<'a>(&'a str) -> bool + std::fmt::Debug) {}
LL + fn fc(_: impl std::fmt::Display + for<'a> FnOnce(&'a str) -> bool + std::fmt::Debug) {}
|

error: `Fn` traits cannot take lifetime parameters
--> $DIR/hrtb-malformed-lifetime-generics.rs:14:24
|
LL | fn fd(_: impl AliasedFn<'a>(&'a str) -> bool) {}
| ^^^^
|
help: consider using a higher-ranked trait bound instead
|
LL - fn fd(_: impl AliasedFn<'a>(&'a str) -> bool) {}
LL + fn fd(_: impl for<'a> AliasedFn(&'a str) -> bool) {}
|

error: `Fn` traits cannot take lifetime parameters
--> $DIR/hrtb-malformed-lifetime-generics.rs:17:27
|
LL | fn fe<F>(_: F) where F: Fn<'a>(&'a str) -> bool {}
| ^^^^
|
help: consider using a higher-ranked trait bound instead
|
LL - fn fe<F>(_: F) where F: Fn<'a>(&'a str) -> bool {}
LL + fn fe<F>(_: F) where F: for<'a> Fn(&'a str) -> bool {}
|

error: aborting due to 5 previous errors