Skip to content

Fix the grammar of function pointer types#2240

Open
fmease wants to merge 4 commits intorust-lang:masterfrom
fmease:fix-grammar-fn-ptr-tys-etc
Open

Fix the grammar of function pointer types#2240
fmease wants to merge 4 commits intorust-lang:masterfrom
fmease:fix-grammar-fn-ptr-tys-etc

Conversation

@fmease
Copy link
Copy Markdown
Member

@fmease fmease commented Apr 24, 2026

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:

  1. safe fn()
  2. fn(...), fn(...,), fn(_: ...), fn(v: ...), fn(..., ())
  3. fn(&self), fn(&'static mut self, ())
  4. 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.

fmease added 4 commits April 24, 2026 19:48
* `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
* 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.
@rustbot rustbot added the S-waiting-on-review Status: The marked PR is awaiting review from a maintainer label Apr 24, 2026
MaybeNamedFunctionParametersVariadic ->
( MaybeNamedParam `,` )* MaybeNamedParam `,` OuterAttribute* `...`
MaybeNamedPattern ->
`mut`? IDENTIFIER | ( `&` | `&&` )? ( `_` | `false` | `true` )
Copy link
Copy Markdown
Member Author

@fmease fmease Apr 24, 2026

Choose a reason for hiding this comment

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

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.

View changes since the review

@fmease
Copy link
Copy Markdown
Member Author

fmease commented Apr 24, 2026

r? ehuss or reassign

Comment thread src/items/functions.md

[^fn-param-2015]: Function parameters with only a type are only allowed in an associated function of a [trait item] in the 2015 edition.

[^safe-semantics]: The `safe` qualifier is only allowed semantically on functions and function pointer types within `extern` blocks.
Copy link
Copy Markdown
Member Author

@fmease fmease Apr 24, 2026

Choose a reason for hiding this comment

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

Suggested change
[^safe-semantics]: The `safe` qualifier is only allowed semantically on functions and function pointer types within `extern` blocks.
[^safe-semantics]: The `safe` qualifier is only allowed semantically on functions within `extern` blocks.

oops

View changes since the review

MaybeNamedFunctionParametersVariadic ->
( MaybeNamedParam `,` )* MaybeNamedParam `,` OuterAttribute* `...`
MaybeNamedPattern ->
`mut`? IDENTIFIER | ( `&` | `&&` )? ( `_` | `false` | `true` )
Copy link
Copy Markdown
Member Author

@fmease fmease Apr 24, 2026

Choose a reason for hiding this comment

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

TODO: Note somewhere (maybe in a footnote that semantically only identifiers and underscores are valid)

View changes since the review

Copy link
Copy Markdown
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

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).

View changes since this review

MaybeNamedFunctionParametersVariadic ->
( MaybeNamedParam `,` )* MaybeNamedParam `,` OuterAttribute* `...`
MaybeNamedPattern ->
`mut`? IDENTIFIER | ( `&` | `&&` )? ( `_` | `false` | `true` )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
`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.

@ehuss
Copy link
Copy Markdown
Contributor

ehuss commented Apr 26, 2026

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.

safe syntactically allowed on a function pointer -- Missed in #1536 (not communicated, missed in verification)

... allowed as a standalone argument -- Missed in #1494 (not communicated, missed in verification)

... as last argument switch to a semantic restriction -- Missed in #927 (oversight, overwhelmed in large number of changes)

trailing ... can be followed by a comma -- Missed in #927 (oversight, overwhelmed in large number of changes)

self allowed in function pointer -- Missed in #927 (from rust-lang/rust#68764) (oversight, overwhelmed in large number of changes)

... allows named argument -- Added in rust-lang/rust#57760 which should have been unstable. No notification of this change.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: The marked PR is awaiting review from a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants