From e5281c389d5bffabaa0a9762314808fb6c2803a3 Mon Sep 17 00:00:00 2001 From: Yutaro Ohno Date: Thu, 17 Nov 2022 20:38:40 +0900 Subject: [PATCH] Provide a better error for `Fn` traits with lifetime params Currently, given `Fn`-family traits with lifetime params like `Fn<'a>(&'a str) -> bool`, many unhelpful errors show up. These are a bit confusing. This commit allows these situations to suggest simply using higher-ranked trait bounds like `for<'a> Fn(&'a str) -> bool`. --- compiler/rustc_parse/src/parser/item.rs | 2 +- compiler/rustc_parse/src/parser/ty.rs | 95 ++++++++++++++++++- .../hrtb-malformed-lifetime-generics.rs | 20 ++++ .../hrtb-malformed-lifetime-generics.stderr | 62 ++++++++++++ 4 files changed, 176 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.rs create mode 100644 src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.stderr diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index e1ced2eb9656c..265c03e40b7f0 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -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> { + pub(super) fn parse_fn_params(&mut self, req_name: ReqName) -> PResult<'a, Vec> { let mut first_param = true; // Parse the arguments, starting out with `self` being allowed... let (mut params, _) = self.parse_paren_comma_seq(|p| { diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 8661e9ca16b8d..fb5dea457e1d1 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -933,8 +933,8 @@ 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() { @@ -942,6 +942,11 @@ impl<'a> Parser<'a> { } 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 @@ -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, + ) -> 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 { + // 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::>(); + 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::(), + ); + 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() diff --git a/src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.rs b/src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.rs new file mode 100644 index 0000000000000..4b096be591a04 --- /dev/null +++ b/src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.rs @@ -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) where F: Fn<'a>(&'a str) -> bool {} +//~^ ERROR `Fn` traits cannot take lifetime parameters + +fn main() {} diff --git a/src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.stderr b/src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.stderr new file mode 100644 index 0000000000000..e8f6d63b5ab8a --- /dev/null +++ b/src/test/ui/higher-rank-trait-bounds/hrtb-malformed-lifetime-generics.stderr @@ -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) where F: Fn<'a>(&'a str) -> bool {} + | ^^^^ + | +help: consider using a higher-ranked trait bound instead + | +LL - fn fe(_: F) where F: Fn<'a>(&'a str) -> bool {} +LL + fn fe(_: F) where F: for<'a> Fn(&'a str) -> bool {} + | + +error: aborting due to 5 previous errors +