Skip to content

Commit e9fe9f3

Browse files
ZalatharZalathar
authored
Rollup merge of #145378 - xizheyin:144968, r=davidtwco
Add `FnContext` in parser for diagnostic Fixes #144968 Inspired by #144968 (comment), I implemented `FnContext` to indicate whether a function should have a self parameter, for example, whether the function is a trait method, whether it is in an impl block. And I removed the outdated note. I made two commits to show the difference. cc `@estebank` `@djc` r? compiler
2 parents 5f0612e + 3ce555f commit e9fe9f3

17 files changed

+176
-74
lines changed

‎compiler/rustc_parse/src/parser/attr.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use tracing::debug;
1111
use super::{
1212
AttrWrapper, Capturing, FnParseMode, ForceCollect, Parser, PathStyle, Trailing, UsePreAttrPos,
1313
};
14+
use crate::parser::FnContext;
1415
use crate::{errors, exp, fluent_generated as fluent};
1516

1617
// Public for rustfmt usage
@@ -200,7 +201,7 @@ impl<'a> Parser<'a> {
200201
AttrWrapper::empty(),
201202
true,
202203
false,
203-
FnParseMode { req_name: |_| true, req_body: true },
204+
FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true },
204205
ForceCollect::No,
205206
) {
206207
Ok(Some(item)) => {

‎compiler/rustc_parse/src/parser/diagnostics.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use crate::errors::{
4444
UnexpectedConstParamDeclaration, UnexpectedConstParamDeclarationSugg, UnmatchedAngleBrackets,
4545
UseEqInstead, WrapType,
4646
};
47+
use crate::parser::FnContext;
4748
use crate::parser::attr::InnerAttrPolicy;
4849
use crate::{exp, fluent_generated as fluent};
4950

@@ -2246,6 +2247,7 @@ impl<'a> Parser<'a> {
22462247
pat: Box<ast::Pat>,
22472248
require_name: bool,
22482249
first_param: bool,
2250+
fn_parse_mode: &crate::parser::item::FnParseMode,
22492251
) -> Option<Ident> {
22502252
// If we find a pattern followed by an identifier, it could be an (incorrect)
22512253
// C-style parameter declaration.
@@ -2268,7 +2270,14 @@ impl<'a> Parser<'a> {
22682270
|| self.token == token::Lt
22692271
|| self.token == token::CloseParen)
22702272
{
2271-
let rfc_note = "anonymous parameters are removed in the 2018 edition (see RFC 1685)";
2273+
let maybe_emit_anon_params_note = |this: &mut Self, err: &mut Diag<'_>| {
2274+
let ed = this.token.span.with_neighbor(this.prev_token.span).edition();
2275+
if matches!(fn_parse_mode.context, crate::parser::item::FnContext::Trait)
2276+
&& (fn_parse_mode.req_name)(ed)
2277+
{
2278+
err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)");
2279+
}
2280+
};
22722281

22732282
let (ident, self_sugg, param_sugg, type_sugg, self_span, param_span, type_span) =
22742283
match pat.kind {
@@ -2305,15 +2314,21 @@ impl<'a> Parser<'a> {
23052314
"_: ".to_string(),
23062315
Applicability::MachineApplicable,
23072316
);
2308-
err.note(rfc_note);
2317+
maybe_emit_anon_params_note(self, err);
23092318
}
23102319

23112320
return None;
23122321
}
23132322
};
23142323

23152324
// `fn foo(a, b) {}`, `fn foo(a<x>, b<y>) {}` or `fn foo(usize, usize) {}`
2316-
if first_param {
2325+
if first_param
2326+
// Only when the fn is a method, we emit this suggestion.
2327+
&& matches!(
2328+
fn_parse_mode.context,
2329+
FnContext::Trait | FnContext::Impl
2330+
)
2331+
{
23172332
err.span_suggestion_verbose(
23182333
self_span,
23192334
"if this is a `self` type, give it a parameter name",
@@ -2337,7 +2352,7 @@ impl<'a> Parser<'a> {
23372352
type_sugg,
23382353
Applicability::MachineApplicable,
23392354
);
2340-
err.note(rfc_note);
2355+
maybe_emit_anon_params_note(self, err);
23412356

23422357
// Don't attempt to recover by using the `X` in `X<Y>` as the parameter name.
23432358
return if self.token == token::Lt { None } else { Some(ident) };

‎compiler/rustc_parse/src/parser/item.rs

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ impl<'a> Parser<'a> {
116116

117117
impl<'a> Parser<'a> {
118118
pub fn parse_item(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<Box<Item>>> {
119-
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
119+
let fn_parse_mode =
120+
FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true };
120121
self.parse_item_(fn_parse_mode, force_collect).map(|i| i.map(Box::new))
121122
}
122123

@@ -975,16 +976,20 @@ impl<'a> Parser<'a> {
975976
&mut self,
976977
force_collect: ForceCollect,
977978
) -> PResult<'a, Option<Option<Box<AssocItem>>>> {
978-
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
979+
let fn_parse_mode =
980+
FnParseMode { req_name: |_| true, context: FnContext::Impl, req_body: true };
979981
self.parse_assoc_item(fn_parse_mode, force_collect)
980982
}
981983

982984
pub fn parse_trait_item(
983985
&mut self,
984986
force_collect: ForceCollect,
985987
) -> PResult<'a, Option<Option<Box<AssocItem>>>> {
986-
let fn_parse_mode =
987-
FnParseMode { req_name: |edition| edition >= Edition::Edition2018, req_body: false };
988+
let fn_parse_mode = FnParseMode {
989+
req_name: |edition| edition >= Edition::Edition2018,
990+
context: FnContext::Trait,
991+
req_body: false,
992+
};
988993
self.parse_assoc_item(fn_parse_mode, force_collect)
989994
}
990995

@@ -1261,7 +1266,8 @@ impl<'a> Parser<'a> {
12611266
&mut self,
12621267
force_collect: ForceCollect,
12631268
) -> PResult<'a, Option<Option<Box<ForeignItem>>>> {
1264-
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: false };
1269+
let fn_parse_mode =
1270+
FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: false };
12651271
Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
12661272
|Item { attrs, id, span, vis, kind, tokens }| {
12671273
let kind = match ForeignItemKind::try_from(kind) {
@@ -2135,7 +2141,8 @@ impl<'a> Parser<'a> {
21352141
let inherited_vis =
21362142
Visibility { span: DUMMY_SP, kind: VisibilityKind::Inherited, tokens: None };
21372143
// We use `parse_fn` to get a span for the function
2138-
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
2144+
let fn_parse_mode =
2145+
FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true };
21392146
match self.parse_fn(
21402147
&mut AttrVec::new(),
21412148
fn_parse_mode,
@@ -2403,6 +2410,9 @@ pub(crate) struct FnParseMode {
24032410
/// * The span is from Edition 2015. In particular, you can get a
24042411
/// 2015 span inside a 2021 crate using macros.
24052412
pub(super) req_name: ReqName,
2413+
/// The context in which this function is parsed, used for diagnostics.
2414+
/// This indicates the fn is a free function or method and so on.
2415+
pub(super) context: FnContext,
24062416
/// If this flag is set to `true`, then plain, semicolon-terminated function
24072417
/// prototypes are not allowed here.
24082418
///
@@ -2424,6 +2434,18 @@ pub(crate) struct FnParseMode {
24242434
pub(super) req_body: bool,
24252435
}
24262436

2437+
/// The context in which a function is parsed.
2438+
/// FIXME(estebank, xizheyin): Use more variants.
2439+
#[derive(Clone, Copy, PartialEq, Eq)]
2440+
pub(crate) enum FnContext {
2441+
/// Free context.
2442+
Free,
2443+
/// A Trait context.
2444+
Trait,
2445+
/// An Impl block.
2446+
Impl,
2447+
}
2448+
24272449
/// Parsing of functions and methods.
24282450
impl<'a> Parser<'a> {
24292451
/// Parse a function starting from the front matter (`const ...`) to the body `{ ... }` or `;`.
@@ -2439,11 +2461,8 @@ impl<'a> Parser<'a> {
24392461
let header = self.parse_fn_front_matter(vis, case, FrontMatterParsingMode::Function)?; // `const ... fn`
24402462
let ident = self.parse_ident()?; // `foo`
24412463
let mut generics = self.parse_generics()?; // `<'a, T, ...>`
2442-
let decl = match self.parse_fn_decl(
2443-
fn_parse_mode.req_name,
2444-
AllowPlus::Yes,
2445-
RecoverReturnSign::Yes,
2446-
) {
2464+
let decl = match self.parse_fn_decl(&fn_parse_mode, AllowPlus::Yes, RecoverReturnSign::Yes)
2465+
{
24472466
Ok(decl) => decl,
24482467
Err(old_err) => {
24492468
// If we see `for Ty ...` then user probably meant `impl` item.
@@ -2961,18 +2980,21 @@ impl<'a> Parser<'a> {
29612980
/// Parses the parameter list and result type of a function declaration.
29622981
pub(super) fn parse_fn_decl(
29632982
&mut self,
2964-
req_name: ReqName,
2983+
fn_parse_mode: &FnParseMode,
29652984
ret_allow_plus: AllowPlus,
29662985
recover_return_sign: RecoverReturnSign,
29672986
) -> PResult<'a, Box<FnDecl>> {
29682987
Ok(Box::new(FnDecl {
2969-
inputs: self.parse_fn_params(req_name)?,
2988+
inputs: self.parse_fn_params(fn_parse_mode)?,
29702989
output: self.parse_ret_ty(ret_allow_plus, RecoverQPath::Yes, recover_return_sign)?,
29712990
}))
29722991
}
29732992

29742993
/// Parses the parameter list of a function, including the `(` and `)` delimiters.
2975-
pub(super) fn parse_fn_params(&mut self, req_name: ReqName) -> PResult<'a, ThinVec<Param>> {
2994+
pub(super) fn parse_fn_params(
2995+
&mut self,
2996+
fn_parse_mode: &FnParseMode,
2997+
) -> PResult<'a, ThinVec<Param>> {
29762998
let mut first_param = true;
29772999
// Parse the arguments, starting out with `self` being allowed...
29783000
if self.token != TokenKind::OpenParen
@@ -2988,7 +3010,7 @@ impl<'a> Parser<'a> {
29883010
let (mut params, _) = self.parse_paren_comma_seq(|p| {
29893011
p.recover_vcs_conflict_marker();
29903012
let snapshot = p.create_snapshot_for_diagnostic();
2991-
let param = p.parse_param_general(req_name, first_param, true).or_else(|e| {
3013+
let param = p.parse_param_general(fn_parse_mode, first_param, true).or_else(|e| {
29923014
let guar = e.emit();
29933015
// When parsing a param failed, we should check to make the span of the param
29943016
// not contain '(' before it.
@@ -3019,7 +3041,7 @@ impl<'a> Parser<'a> {
30193041
/// - `recover_arg_parse` is used to recover from a failed argument parse.
30203042
pub(super) fn parse_param_general(
30213043
&mut self,
3022-
req_name: ReqName,
3044+
fn_parse_mode: &FnParseMode,
30233045
first_param: bool,
30243046
recover_arg_parse: bool,
30253047
) -> PResult<'a, Param> {
@@ -3035,16 +3057,22 @@ impl<'a> Parser<'a> {
30353057

30363058
let is_name_required = match this.token.kind {
30373059
token::DotDotDot => false,
3038-
_ => req_name(this.token.span.with_neighbor(this.prev_token.span).edition()),
3060+
_ => (fn_parse_mode.req_name)(
3061+
this.token.span.with_neighbor(this.prev_token.span).edition(),
3062+
),
30393063
};
30403064
let (pat, ty) = if is_name_required || this.is_named_param() {
30413065
debug!("parse_param_general parse_pat (is_name_required:{})", is_name_required);
30423066
let (pat, colon) = this.parse_fn_param_pat_colon()?;
30433067
if !colon {
30443068
let mut err = this.unexpected().unwrap_err();
3045-
return if let Some(ident) =
3046-
this.parameter_without_type(&mut err, pat, is_name_required, first_param)
3047-
{
3069+
return if let Some(ident) = this.parameter_without_type(
3070+
&mut err,
3071+
pat,
3072+
is_name_required,
3073+
first_param,
3074+
fn_parse_mode,
3075+
) {
30483076
let guar = err.emit();
30493077
Ok((dummy_arg(ident, guar), Trailing::No, UsePreAttrPos::No))
30503078
} else {

‎compiler/rustc_parse/src/parser/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use std::{fmt, mem, slice};
2222
use attr_wrapper::{AttrWrapper, UsePreAttrPos};
2323
pub use diagnostics::AttemptLocalParseRecovery;
2424
pub(crate) use expr::ForbiddenLetReason;
25-
pub(crate) use item::FnParseMode;
25+
pub(crate) use item::{FnContext, FnParseMode};
2626
pub use pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
2727
use path::PathStyle;
2828
use rustc_ast::token::{

‎compiler/rustc_parse/src/parser/path.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ use crate::errors::{
2020
PathFoundAttributeInParams, PathFoundCVariadicParams, PathSingleColon, PathTripleColon,
2121
};
2222
use crate::exp;
23-
use crate::parser::{CommaRecoveryMode, ExprKind, RecoverColon, RecoverComma};
23+
use crate::parser::{
24+
CommaRecoveryMode, ExprKind, FnContext, FnParseMode, RecoverColon, RecoverComma,
25+
};
2426

2527
/// Specifies how to parse a path.
2628
#[derive(Copy, Clone, PartialEq)]
@@ -399,7 +401,13 @@ impl<'a> Parser<'a> {
399401

400402
let dcx = self.dcx();
401403
let parse_params_result = self.parse_paren_comma_seq(|p| {
402-
let param = p.parse_param_general(|_| false, false, false);
404+
// Inside parenthesized type arguments, we want types only, not names.
405+
let mode = FnParseMode {
406+
context: FnContext::Free,
407+
req_name: |_| false,
408+
req_body: false,
409+
};
410+
let param = p.parse_param_general(&mode, false, false);
403411
param.map(move |param| {
404412
if !matches!(param.pat.kind, PatKind::Missing) {
405413
dcx.emit_err(FnPathFoundNamedParams {

‎compiler/rustc_parse/src/parser/stmt.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use super::diagnostics::AttemptLocalParseRecovery;
1919
use super::pat::{PatternLocation, RecoverComma};
2020
use super::path::PathStyle;
2121
use super::{
22-
AttrWrapper, BlockMode, FnParseMode, ForceCollect, Parser, Restrictions, SemiColonMode,
23-
Trailing, UsePreAttrPos,
22+
AttrWrapper, BlockMode, FnContext, FnParseMode, ForceCollect, Parser, Restrictions,
23+
SemiColonMode, Trailing, UsePreAttrPos,
2424
};
2525
use crate::errors::{self, MalformedLoopLabel};
2626
use crate::exp;
@@ -153,7 +153,7 @@ impl<'a> Parser<'a> {
153153
attrs.clone(), // FIXME: unwanted clone of attrs
154154
false,
155155
true,
156-
FnParseMode { req_name: |_| true, req_body: true },
156+
FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true },
157157
force_collect,
158158
)? {
159159
self.mk_stmt(lo.to(item.span), StmtKind::Item(Box::new(item)))

‎compiler/rustc_parse/src/parser/ty.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use crate::errors::{
1919
NestedCVariadicType, ReturnTypesUseThinArrow,
2020
};
2121
use crate::parser::item::FrontMatterParsingMode;
22+
use crate::parser::{FnContext, FnParseMode};
2223
use crate::{exp, maybe_recover_from_interpolated_ty_qpath};
2324

2425
/// Signals whether parsing a type should allow `+`.
@@ -769,7 +770,12 @@ impl<'a> Parser<'a> {
769770
if self.may_recover() && self.token == TokenKind::Lt {
770771
self.recover_fn_ptr_with_generics(lo, &mut params, param_insertion_point)?;
771772
}
772-
let decl = self.parse_fn_decl(|_| false, AllowPlus::No, recover_return_sign)?;
773+
let mode = crate::parser::item::FnParseMode {
774+
req_name: |_| false,
775+
context: FnContext::Free,
776+
req_body: false,
777+
};
778+
let decl = self.parse_fn_decl(&mode, AllowPlus::No, recover_return_sign)?;
773779

774780
let decl_span = span_start.to(self.prev_token.span);
775781
Ok(TyKind::FnPtr(Box::new(FnPtrTy {
@@ -1314,7 +1320,8 @@ impl<'a> Parser<'a> {
13141320
self.bump();
13151321
let args_lo = self.token.span;
13161322
let snapshot = self.create_snapshot_for_diagnostic();
1317-
match self.parse_fn_decl(|_| false, AllowPlus::No, RecoverReturnSign::OnlyFatArrow) {
1323+
let mode = FnParseMode { req_name: |_| false, context: FnContext::Free, req_body: false };
1324+
match self.parse_fn_decl(&mode, AllowPlus::No, RecoverReturnSign::OnlyFatArrow) {
13181325
Ok(decl) => {
13191326
self.dcx().emit_err(ExpectedFnPathFoundFnKeyword { fn_token_span });
13201327
Some(ast::Path {
@@ -1400,8 +1407,9 @@ impl<'a> Parser<'a> {
14001407

14011408
// Parse `(T, U) -> R`.
14021409
let inputs_lo = self.token.span;
1410+
let mode = FnParseMode { req_name: |_| false, context: FnContext::Free, req_body: false };
14031411
let inputs: ThinVec<_> =
1404-
self.parse_fn_params(|_| false)?.into_iter().map(|input| input.ty).collect();
1412+
self.parse_fn_params(&mode)?.into_iter().map(|input| input.ty).collect();
14051413
let inputs_span = inputs_lo.to(self.prev_token.span);
14061414
let output = self.parse_ret_ty(AllowPlus::No, RecoverQPath::No, RecoverReturnSign::No)?;
14071415
let args = ast::ParenthesizedArgs {

‎tests/ui/parser/inverted-parameters.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ fn pattern((i32, i32) (a, b)) {}
2323
fn fizz(i32) {}
2424
//~^ ERROR expected one of `:`, `@`
2525
//~| HELP if this is a parameter name, give it a type
26-
//~| HELP if this is a `self` type, give it a parameter name
2726
//~| HELP if this is a type, explicitly ignore the parameter name
2827

2928
fn missing_colon(quux S) {}

‎tests/ui/parser/inverted-parameters.stderr

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ error: expected one of `:`, `@`, or `|`, found `)`
3434
LL | fn fizz(i32) {}
3535
| ^ expected one of `:`, `@`, or `|`
3636
|
37-
= note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
38-
help: if this is a `self` type, give it a parameter name
39-
|
40-
LL | fn fizz(self: i32) {}
41-
| +++++
4237
help: if this is a parameter name, give it a type
4338
|
4439
LL | fn fizz(i32: TypeName) {}
@@ -49,7 +44,7 @@ LL | fn fizz(_: i32) {}
4944
| ++
5045

5146
error: expected one of `:`, `@`, or `|`, found `S`
52-
--> $DIR/inverted-parameters.rs:29:23
47+
--> $DIR/inverted-parameters.rs:28:23
5348
|
5449
LL | fn missing_colon(quux S) {}
5550
| -----^

‎tests/ui/parser/lifetime-in-pattern.stderr

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@ error: expected one of `:`, `@`, or `|`, found `)`
1616
LL | fn test(&'a str) {
1717
| ^ expected one of `:`, `@`, or `|`
1818
|
19-
= note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
20-
help: if this is a `self` type, give it a parameter name
21-
|
22-
LL | fn test(self: &'a str) {
23-
| +++++
2419
help: if this is a parameter name, give it a type
2520
|
2621
LL - fn test(&'a str) {

0 commit comments

Comments
 (0)