Named Fn trait parameters

[JonathanBrouwer]

View all comments

Allow (optional) named function parameters in parenthesized generic argument lists, such as those of Fn, FnMut, FnOnce, AsyncFn, AsyncFnMut, and AsyncFnOnce.
For example:

fn parse_my_data(
    data: &str,
    log: impl Fn(msg: String, priority: usize)
) { }

Similar to named function pointer parameters, these names don't affect rust's semantics.

Important

Since RFCs involve many conversations at once that can be difficult to follow, please use review comment threads on the text changes instead of direct comments on the RFC.

If you don't have a particular section of the RFC to comment on, you can click on the "Comment on this file" button on the top-right corner of the diff, to the right of the "Viewed" checkbox. This will create a separate thread even if others have commented on the file too.

Rendered

[clarfonthey]

Mostly just adding this comment here from Zulip to keep track of some of the discussion.

I mentioned that we should probably try to unify the syntax for Fn traits, fn pointers, and functions without bodies. It appears that similarly, the reference is not fully clear on these either.

Particularly, the example you gave that did not compile:

trait Test {
    fn thing((x, y): (usize, usize));
}

seems to directly contradict what the reference says: https://doc.rust-lang.org/stable/reference/items/functions.html

In particular, it appears that only name: Type is supported, not arbitrary patterns.

View changes since the review

[fmease]

We've already briefly discussed this on Zulip but just to get others up to speed: The Reference is correct in stating that the "parameter names" of even bodiless functions are patterns (PatNoTopAlt to be clear).

The grammar rules describe what's syntactically allowed and doesn't dictate what's semantically allowed or forbidden.

Patterns except CommonIdent | "_" in bodiless functions are semantically ill-formed but syntactically allowed.

[fmease]

To further clarify this (since the Reference doesn't document this nuance which I'll rectify in a moment):

The parameters of function pointer types basically share the same grammar as parameters of trait associated functions in Rust 2015 where the "parameter name" is optional and where the parameter pattern is restricted. As I've mentioned on Zulip it obeys the following rules (excerpt):

RestrictedPat = RestrictedIdent | "&" RestrictedIdent | "&&" RestrictedIdent | "mut" CommonIdent;
RestrictedIdent = CommonIdent | "_" | "false" | "true";

In Rust >2015, however, the grammar of trait associated functions is the same as all other kinds of functions (free, foreign, impl associated).

[petrochenkov]

In the past RestrictedPat was semantically accepted as well.
I thought we eliminated it many years ago through a deprecation lint, but apparently the lint only worked at semantic level and wasn't reported at parsing time. I suspect we just didn't have the parse-time linting infra at the time (lint buffering, etc).

[fmease]

I'm currently working on yeeting that. I've seen the historical RFCs, future-incompat issues and relevant PRs.

Fun fact, the & and the && are the remnants of argument modes (2012/2013-ish era?). Only by happenstance do they get parsed as borrow patterns nowadays.

Back in the day there were 5 argument modes, - (by move), & (by mut ref), && (by ref), + (by copy), ++ (by value). That used to look something like fn f(-a: A, &b: B, &&c: C, +d: D, ++e: E) {}

That's literally the reason why, it's beyond insane. Crater run coming soon btw :)

[clarfonthey]

This feels like something that, even if we allow, we should lint against. It feels reasonable to not propose as part of the RFC, but I feel like what makes the most sense is to replicate what traits/functions do but allow for a deny-by-default lint in the future.

View changes since the review

[petrochenkov]

Old issue for this - rust-lang/rust#33995.

[ChayimFriedman2]

A major reason for this RFC is better documentation and better LSP hints. As a member of T-rust-analyzer, I unfortunately have to inform you that rust-analyzer cannot support this feature, at least not without significant changes (it does not show hints for named fn pointers parameters, either).

It might be possible, though, to support a few special cases.

View changes since the review

[JonathanBrouwer]

Ah, sucks :c

I still believe this RFC is valuable even without this benefit though, as mentioned the primary benefit is "Better documentation"

[Veykril]

Yea I think the main issue is that even if the type IR encodes this info, we might lose this information after inference depending on how the types are unified, trait bounds are proven etc

[clarfonthey]

That said, even though R-A won't necessarily be able to provide inlay hints, the hover-rustdoc could show this information, meaning that it technically still could have some usefulness in the LSP implementation, if only indirectly.

[programmerjake]

Yea I think the main issue is that even if the type IR encodes this info, we might lose this information after inference depending on how the types are unified, trait bounds are proven etc

when unifying two types, couldn't rust-analyzer try to propagate argument names if one type has them and the other doesn't?

[ChayimFriedman2]

@programmerjake We probably could, but that depends on the solver allowing us to encode this info (which is a rustc crate and perf-sensitive, and also a major change).

[fmease]

The syntax Fn, FnMut and FnOnce traits is currently not documented in the reference.

It is documented. See e.g., https://doc.rust-lang.org/nightly/reference/paths.html#grammar-TypePathFnInputs. More specifically TypePathFn.

View changes since the review

[JonathanBrouwer]

Ahhh that is where it is, I will change the reference level explanation in terms of those rules

[fmease]

Addendum: Type paths are not the only paths featuring parenthesized generic argument lists. Expression and pattern paths have the same behavior. For example the following is legal:

#[cfg(false)]
fn scope() {
    let F::() -> () = F::() -> ()::Y;
}

[JonathanBrouwer]

I've rewritten the reference-level explanation in terms of TypePathFn. I will process your addendums later

[fmease]

Final addendum: This syntax is obviously not tied to Fn, FnMut and FnOnce; the path can be anything. Moreover, from a semantic standpoint, not only are Fn{,Mut,Once} legal but also AsyncFn, AsyncFnMut and AsyncFnOnce.

[JonathanBrouwer]

Type paths are not the only paths featuring parenthesized generic argument lists. Expression and pattern paths have the same behavior. For example the following is legal:

Is this behaviour documented in the reference anywhere?

[JonathanBrouwer]

This syntax is obviously not tied to Fn, FnMut and FnOnce; the path can be anything. Moreover, from a semantic standpoint, not only are Fn{,Mut,Once} legal but also AsyncFn, AsyncFnMut and AsyncFnOnce.

I've made this more explicit in the RFC