Fix the grammar of function pointer types#2240
Fix the grammar of function pointer types#2240fmease wants to merge 4 commits intorust-lang:masterfrom
Conversation
rustbot
added
the
S-waiting-on-review
| MaybeNamedFunctionParametersVariadic -> | ||
| ( MaybeNamedParam `,` )* MaybeNamedParam `,` OuterAttribute* `...` | ||
| MaybeNamedPattern -> | ||
| `mut`? IDENTIFIER | ( `&` | `&&` )? ( `_` | `false` | `true` ) |
There was a problem hiding this comment.
Yes, indeed, that's the grammar! It's a tiny subset of patterns.
Note that this subgrammar is also used for associated functions in traits in Rust 2015. I didn't know how I am meant to branch on editions, so I didn't update the corresponding function item grammar that currently only has FunctionParam -> … Type[^fn-param-2015] … which is not sufficient strictly speaking.
I guess I could do sth. akin to "ModernGrammar[^1] | LegacyGrammar[^2] // [^1]: Rust 2018–2024 // [^2]: Rust 2015 only" (pseudo). Let me know what you think.
|
r? ehuss or reassign |
| MaybeNamedFunctionParametersVariadic -> | ||
| ( MaybeNamedParam `,` )* MaybeNamedParam `,` OuterAttribute* `...` | ||
| MaybeNamedPattern -> | ||
| `mut`? IDENTIFIER | ( `&` | `&&` )? ( `_` | `false` | `true` ) |
There was a problem hiding this comment.
TODO: Note somewhere (maybe in a footnote that semantically only identifiers and underscores are valid)
There was a problem hiding this comment.
Thanks!
This is just an initial review. I think we'll want to add rules that express each of the semantic restrictions. Whether it's conditional (like items.fn.safety-qualifiers) or unconditional (like items.traits.associated-visibility).
| MaybeNamedFunctionParametersVariadic -> | ||
| ( MaybeNamedParam `,` )* MaybeNamedParam `,` OuterAttribute* `...` | ||
| MaybeNamedPattern -> | ||
| `mut`? IDENTIFIER | ( `&` | `&&` )? ( `_` | `false` | `true` ) |
There was a problem hiding this comment.
This doesn't look quite right. My understanding is that this logic comes from is_named_param.
I think it would look something closer to:
| `mut`? IDENTIFIER | ( `&` | `&&` )? ( `_` | `false` | `true` ) | |
| ((`mut`? | `&` | `&&`)? IDENTIFIER) | ((`&` | `&&`)? (`_` | `false` | `true`)) |
because ident can be preceded by & or &&.
However, this seems to be getting close to the territory of unintentional behavior. I'm wondering if this really is a bug in the compiler, and the grammar should be something else.
There was a problem hiding this comment.
Good catch, I messed that up when copying over & adjusting #t-lang > Naming arguments in Fn traits @ 💬 (which had its own problems).
Yeah, so the gnarly grammar stems from INTERSECT( PatternNoTopAlt , ( `mut` | `&` | `&&` ) IDENTIFIER_OR_KEYWORD ).
Right, the & and && are ancient remnants of argument modes (~2012/2013) and should be banished. The rest should be removed, too. I'm going to spin up a crater run soon and add more context with regards to "argument modes" etc. Slightly more context: rust-lang/rfcs#3955 (comment).
|
I did some research to see how these got missed. Some of them just weren't communicated during implementation/stabilization. Some were just missed when reverse-engineering the compiler. Some were missed when trying to handle too many things at once.
trailing
function pointer parameters have limited patterns -- Missed in initial introduction in #433. I don't remember what happened there. The current behavior does not look intentional to me, though. |
rustbot
removed
the
S-waiting-on-review
* `safe` is a syntactically valid function pointer type qualifier next to `unsafe` but it wasn't listed prior. * renamed `ItemSafety` to `Safety` since it's now also used in the grammar of function pointer types * footnote on non-terminal `Safety` in rule `StaticItem` called `safe` & `unsafe` *function* modifiers when they're evidentally more general and can be placed on *static* items, too * made the footnotes related to `Safety` more precise
The grammar was incorrect since * C variadics `...` no longer need to be preceded by a normal parameter * that's the case since <rust-lang/rust#124048> * C variadics don't need to be the final element, `fn(..., i32)` is perfectly fine * C variadics can indeed be named (e.g., `fn f(_: ...)` or `fn(v: ...)`) * C variadics can be followed by a comma (e.g., `fn(...,)`)
`fn(&self)`, `fn(&'static mut self)` etc. are syntactically valid function pointer types
It's not just (common) identifiers or underscores, it's more complex.
fmease
force-pushed
the
fix-grammar-fn-ptr-tys-etc
branch
from
400cb4b to
01e9ac6
Compare
3 participants
The grammar of function pointer types on master drastically diverges from reality.
Please read the individual commit messages for details (changes, motivation).
For example, according to the grammar on master, the following snippets are syntactically illegal but that is in conflict with rustc which deems them syntactically well-formed:
safe fn()fn(...),fn(...,),fn(_: ...),fn(v: ...),fn(..., ())fn(&self),fn(&'static mut self, ())fn(mut x: ()),fn(false: bool),fn(&_: ()),fn(&&true: ())Context: #t-lang > Naming arguments in Fn traits and rust-lang/rfcs#3955 where this was quite relevant.