Skip to content

refactor(parse): separate self parameter parsing from general parameter parsing #145088

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
30 changes: 14 additions & 16 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2948,8 +2948,6 @@ impl<'a> Parser<'a> {

/// Parses the parameter list of a function, including the `(` and `)` delimiters.
pub(super) fn parse_fn_params(&mut self, req_name: ReqName) -> PResult<'a, ThinVec<Param>> {
let mut first_param = true;
// Parse the arguments, starting out with `self` being allowed...
if self.token != TokenKind::OpenParen
// might be typo'd trait impl, handled elsewhere
&& !self.token.is_keyword(kw::For)
Expand All @@ -2959,11 +2957,20 @@ impl<'a> Parser<'a> {
.emit_err(errors::MissingFnParams { span: self.prev_token.span.shrink_to_hi() });
return Ok(ThinVec::new());
}

let mut params = ThinVec::new();

let (mut params, _) = self.parse_paren_comma_seq(|p| {
//Parse the self parameter as first parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//Parse the self parameter as first parameter
// Parse the self parameter as first parameter

if let Some(mut self_param) = self.parse_self_param()?{
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if let Some(mut self_param) = self.parse_self_param()?{
if let Some(mut self_param) = self.parse_self_param()? {

let self_attrs = self.parse_outer_attributes()?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Having the parse_outer_attributes after the parse_self_param is suspicious.

self_param.attrs = self.attrs;
params.push(self_param);
}

let (mut remaining_params, _) = self.parse_paren_comma_seq(|p| {
p.recover_vcs_conflict_marker();
let snapshot = p.create_snapshot_for_diagnostic();
let param = p.parse_param_general(req_name, first_param, true).or_else(|e| {
let param = p.parse_param_general(req_name, true).or_else(|e| {
let guar = e.emit();
// When parsing a param failed, we should check to make the span of the param
// not contain '(' before it.
Expand All @@ -2979,35 +2986,26 @@ impl<'a> Parser<'a> {
// Create a placeholder argument for proper arg count (issue #34264).
Ok(dummy_arg(Ident::new(sym::dummy, lo.to(p.prev_token.span)), guar))
});
// ...now that we've parsed the first argument, `self` is no longer allowed.
first_param = false;
param
})?;
// Combine self parameter (if any) with remaining parameters
params.extend(remaining_params);
// Replace duplicated recovered params with `_` pattern to avoid unnecessary errors.
self.deduplicate_recovered_params_names(&mut params);
Ok(params)
}

/// Parses a single function parameter.
///
/// - `self` is syntactically allowed when `first_param` holds.
/// - `recover_arg_parse` is used to recover from a failed argument parse.
pub(super) fn parse_param_general(
&mut self,
req_name: ReqName,
first_param: bool,
recover_arg_parse: bool,
) -> PResult<'a, Param> {
let lo = self.token.span;
let attrs = self.parse_outer_attributes()?;
self.collect_tokens(None, attrs, ForceCollect::No, |this, attrs| {
// Possibly parse `self`. Recover if we parsed it and it wasn't allowed here.
if let Some(mut param) = this.parse_self_param()? {
param.attrs = attrs;
let res = if first_param { Ok(param) } else { this.recover_bad_self_param(param) };
return Ok((res?, Trailing::No, UsePreAttrPos::No));
}

let is_name_required = match this.token.kind {
token::DotDotDot => false,
_ => req_name(this.token.span.with_neighbor(this.prev_token.span).edition()),
Expand All @@ -3018,7 +3016,7 @@ impl<'a> Parser<'a> {
if !colon {
let mut err = this.unexpected().unwrap_err();
return if let Some(ident) =
this.parameter_without_type(&mut err, pat, is_name_required, first_param)
this.parameter_without_type(&mut err, pat, is_name_required, false)
{
let guar = err.emit();
Ok((dummy_arg(ident, guar), Trailing::No, UsePreAttrPos::No))
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ impl<'a> Parser<'a> {

let dcx = self.dcx();
let parse_params_result = self.parse_paren_comma_seq(|p| {
let param = p.parse_param_general(|_| false, false, false);
let param = p.parse_param_general(|_| false, false);
param.map(move |param| {
if !matches!(param.pat.kind, PatKind::Missing) {
dcx.emit_err(FnPathFoundNamedParams {
Expand Down
Loading