Distinguish closures from callables

[SoniEx2]

Rendered

This proposal introduces a struct core::ops::Closure, which represents a "concrete" form of closures and allows trait impls to distinguish closures from other kinds of callables. This distinction brings restrictions Rust applies to closures, to the type system, like how closures strictly support only a single Fn* signature; having this restriction visible at the type level allows the same trait to be implemented for multiple closure signatures.

In particular, this proposal aims to solve a historical std API design problem where some char methods, particularly the char::is_ascii* family of functions, cannot be used as std::str::pattern::Patterns, due to requiring &self as a receiver, while Pattern is only implemented for FnMut(char) -> bool, i.e.:

let s: &str = todo!();

s.starts_with(char::is_ascii); // will not compile, `is_ascii` accepts `&char`, not `char`
s.starts_with(char::is_whitespace); // compiles

---

We're also attempting to write this RFC using an unauthorized experimental collaborative process. We're hoping this helps things along, tho we still have no idea if it works. We also couldn't figure out how to git merge --allow-unrelated-histories so if we end up with other contributors we'll just have to manually make the merge commit object we guess.

[shepmaster]

I do not see such a struct. Can you please link to it? Or are you proposing that this become what happens?

[SoniEx2]

The RFC template says to write here "as if [the proposal] was already included in the language". This is that.

Tho we certainly may have taken that too literally.

[lebensterben]

currently closures acts like anonymous structs that captures its environment. But very importantly there's no identical closure.

How does the proposed core::ops:: Closures reflect that?

[SoniEx2]

How does the proposed core::ops:: Closures reflect that?

That's what the internal callable (F) is for! Closure would be just a special compiler-blessed wrapper (we even call it a wrapper) around a callable which allows distinguishing closures from other, non-compiler-blessed callables. This is because closures can't impl multiple Fns, while other callables can.

// this is not a closure, because a closure can't have both impls.
struct CustomCallable;

impl Fn(char) -> bool for CustomCallable {
...
}

impl Fn(&char) -> bool for CustomCallable {
...
}

In other words, closures explicitly cannot do this (and, if we get this RFC, then we can guarantee this will never become accepted):

fn foo<F: Fn(char) -> bool + Fn(&char) -> bool>(f: F) {
    todo!();
}

fn main() {
    foo(|x| true);
}

(Interestingly, rustc attempts to infer the closure signature to whatever comes first here. In this case, that's fn(char) -> _.)

[ChayimFriedman2]

I believe the solution is to disallow implementing Fn* multiple times, not introducing a new wrapper struct.

[SoniEx2]

Introducing a wrapper struct is free. Turning Fn* into a second-class trait is not. Besides, who knows what sorts of interactions that would have with chalk and whatnot.

[ErichDonGubler]

It took some significant reading to collapse my mental model of a problem statement into the following summary:

This RFC attempts to resolve an API design problem where char's inherent methods for classification cannot be used directly with the string pattern API, which used in stable Rust with many important str search APIs like str::find. This is because these methods take self as an argument by reference (&self), while std::str::pattern::Pattern is implemented only for FnMut(char) -> bool (which implies self by value, instead of by reference). For example:

let s: &str = todo!();

s.starts_with(char::is_ascii); // will not compile, `is_ascii` accepts `&char`, not `char`
s.starts_with(|c| c.is_ascii()); // compiles

Does the above seem correct, @SoniEx2? Assuming that it is, I see several issues¹ with this proposal that have a common thread: it proposes a language-level solution to an under-specified problem.

1. There is no coherent problem statement in this PR. The PR OP currently only provides a “Rendered” link and a process note. Quoting the Motivation section in full:

   This proposal exists entirely out of spite for the char::is_ascii* family of functions.

   I interpret “spite” to be an expression of @SoniEx2's annoyance associated with using the mentioned APIs. Obviously not an ideal Rust user experience! By itself, however, this is insufficient as a problem statement; there's no relation of that spite to a concrete problem. I suggest adding descriptions of:

   1. the summary of the issue motivating this RFC,
   2. the technical impact of the issue, and
   3. the desired end state (not the solution).

   Commented code samples, like the one I provided above, will probably be an effective medium for communicating these. Feel free to steal my summary and snippet above for the PR OP, and maybe even the Motivation section.

2. I see some significant gaps in the Rationale and alternatives section.

   1. A typical “workaround” for the motivating issue in user code would be to use a closure (i.e., |c| c.is_ascii() in the motivating code snippet provided above). What is the impact of this workaround? How does that impact compare to the proposed solution?
   2. Library crates with this sort of API design issue could use the deprecated attribute and/or SemVer-breaking API changes to migrate users to a new API that does not have the issues. How might these ideas be useful in solving this problem², compared to the proposed solution?
   3. Could we add FnMut(&char) -> bool to the set of traits implementing Pattern?

Footnotes

1. Admittedly, they might benefit from being in-diff conversations...😅 ↩

2. Backwards compatibility is axiomatic in std, so we probably can't do that. Calling it out, however, seems like important context: it drives home that the motivating issue is a std API design problem, not a general Rust language problem, seemingly. ↩

[SoniEx2]

Yes, that is the entire motivation/issue, reason it needs to be baked into the language, and desired end state.

Spite is still a pretty strong motivator and absolutely belongs in the motivation section of this proposal.

[ErichDonGubler]

Spite is still a pretty strong motivator and absolutely belongs in the motivation section of this proposal.

In case it wasn't clear, I have made no suggestion that you remove mention of spite, or that it doesn't belong. On the contrary, emotional impact can be important context, and I don't see anything wrong with the way you've expressed it. Let's just make sure that people who haven't had first-hand experience feeling spite for this API can relate too. 😉

[Aloso]

The "Summary" section is not a summary. It should be AT LEAST one sentence summarizing the feature, and not just 3 words that tell you nothing unless you are already familiar with it.

Likewise, the "Motivation" section needs to be expanded. It should explain what is the current situation and why it is bad.

[shepmaster]

s.starts_with(char::is_ascii); // will not compile, `is_ascii` accepts `&char`, not `char`

The alternate way I wanted to solve this was to allow auto-(de)ref to apply just as it does for explicit function and method calls:

fn x(_: &str) {}
fn y(_: impl Fn(&String)) {}

fn main() {
    y(|v| x(v));  // Works
    y(x);         // Does not work
}

[SoniEx2]

There are certainly a lot of alternatives to pick from, the only question is which one is most appropriate.

We do wonder if Closure could bring about any additional benefits to the language... Tho we guess future potential isn't usually a deciding factor for RFCs.

For another, definitely more verbose, but zero-cost alternative: could we impl Pattern for char::is_ascii? (It's not actually that verbose due to the use of macros. It does however require exposing function/type-duality to impls, at least for std internals.) Unlike impl Pattern for fn(&char) -> bool, this avoids the fn pointer cost, and applies strictly to the is_ascii* family of functions (i.e. doesn't have any concerns about how the language might expand in the future).

[SoniEx2]

(any feedback on the updated version/etc? we kinda put some of the motivation in the summary and we're not sure how to split it back out but it's probably okay??? at least for now?)

also hmm, future possibility...? the ability to convert a ZST closure into an fn (which might be cheaper than a dyn Fn in some cases)?

[lebensterben]

I'm concerned about the "unauthorized experimental collaborative process" you mentioned...

What does that even mean? I clicked the link and then the page shows me something called MMO RFC???

[SoniEx2]

It means we welcome anyone to edit the proposed RFC almost like it's a wiki. Because the "GAnarchy" thing we linked? It's basically a "wiki of git repos".

[shepmaster]

I'm concerned about the "unauthorized experimental collaborative process" you mentioned...

It literally doesn’t matter because there is no “authorized” way of creating an RFC.

[SoniEx2]

Sure, but it's not like "making a wiki for it" is the expected way of creating RFCs. 😅

We hope that's fine tho.

[SoniEx2]

Hmm, why did this get such an overwhelmingly negative response?

---

Btw, future work could've included giving &Closure<dyn Fn, fn> an TryInto<fn> or so. Tho we find it unlikely that something like that would ever be possible with the current ABI (after all, there's supposed to be an &self there). Or at the very least &Closure<F, fn> could have it, as that's monomorphized - in which case some algorithms which use &dyn Fn could switch to using plain fn instead if there's a significant performance benefit. It would avoid a vtable lookup at least.

[SoniEx2]

apparently rust-lang/rust#84366 means we can't have this. :(

[SoniEx2]

it's unfortunate that fundamental traits are such a huge hack that even doing it properly (like with this RFC) wouldn't allow their removal...