-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Description
Summary
The function parse_param_general currently handles two responsibilities:
- Parsing the general function parameter syntax (like
x: T,_x: T, etc.) - Optionally parsing the
selfreceiver (e.g.self,&mut self,self: Box<Self>, etc.)
This makes the function harder to follow and less composable.
Observation
This function is only used in one place: Parser::parse_fn_params.
Because of this, it is safe and straightforward to remove the first_param flag and instead handle the optional self parameter directly in parse_fn_params.
Suggested Refactor
Move the self parsing logic out of parse_param_general and into parse_fn_params.
This means:
- Add a
try_parse_self_param()function to explicitly attempt parsingselfas the first parameter. - If successful, push the resulting param into the
ThinVecbefore looping over regular parameters. - Remove the
first_paramboolean fromparse_param_general. - Keep
req_namelogic intact (used to require named params).
Benefits
- Simpler and more maintainable code.
- Clearer separation of responsibilities.
- Easier to test general parameter logic.
- More idiomatic parser design (handle special cases separately).
Challenge
A key complication is that self parsing must occur after the ( token has been consumed — it is not syntactically valid before that point.
As a result, while extracting self-param parsing into a separate function (like try_parse_self_param) seems clean in theory, it doesn't buy us much in practice, because:
- It cannot be called before
parse_fn_paramssees the opening(. parse_param_generalis only called after the paren is parsed anyway.- Therefore, the decision logic for "is this the first parameter?" and "can it be
self?" may remain right there.
Notes
This was previously attempted in PR #144198 by myself, but I closed due to mentioned challange.
This issue tracks reintroducing that refactor cleanly.
Related Files
Parser::parse_fn_paramsParser::parse_param_general