Add missing impl Fn for &mut F where F: Fn

[Kivooeo]

Currently, there are blanket implementations for &F and &mut F for FnMut and FnOnce, but not for Fn, as a result, &mut F implements fewer of the Fn traits than &F, which is inconsistent and this PR fills that coherence hole

Technically, this is a breaking change because Fn is a fundamental trait, however, practical breakage is expected to be minimal, as such overlapping impl patterns are extremely rare

pub trait Trait {}
impl<F: Fn()> Trait for F {}
impl Trait for &fn() {}     // not allowed
impl Trait for &mut fn() {} // allowed

Because of this, second impl for &mut fn() works fine but shouldn't

Fixes #147931

[rustbot]

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Noratrieb]

@bors try

[Noratrieb]

r? libs-api

[rust-bors]

☀️ Try build successful (CI)
Build commit: ddcf048 (ddcf048784667508cd676fef4667f88ae15628a3, parent: 292be5c7c05138d753bbd4b30db7a3f1a5c914f7)

[theemathas]

Is this "since" line correct?

[Kivooeo]

This is going to be insta stable so I think so and it should be available on all versions

[theemathas]

What do you mean by "should be available on all versions"?

[Kivooeo]

Ah, you mean that if I make it since = 1.91 it will work not only in 1.91+? because in my opinion this should be available in all versions since 1.0

[Noratrieb]

This attribute is only describing when it was added for docs. We cannot add things to old versions.

[Kivooeo]

oh, ok then, I will change it after crater

[Kivooeo]

huh, seems like I can't?

error[E0711]: feature `async_closure` is declared stable since 1.93.0, but was previously declared stable since 1.85.0
   --> library/core/src/ops/async_function.rs:130:5
    |
130 |     #[stable(feature = "async_closure", since = "1.93.0")]
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0711]: feature `rust1` is declared stable since 1.93.0, but was previously declared stable since 1.0.0
   --> library/core/src/ops/function.rs:314:5
    |
314 |     #[stable(feature = "rust1", since = "1.93.0")]
    |    

[Kivooeo]

it don't feel correct to change stable since for all impls

[teor2345]

What you've done might be fine, I've seen other PRs that just re-use an existing stable feature name and version for small gaps like this.

But for other PRs, if you want to declare them stable in the version where the PR merges, use:

Suggested change

	#[stable(feature = "rust1", since = "1.0.0")]
	#[stable(feature = "new_feature_name", since = "CURRENT_RUSTC_VERSION")]

This does two things:

• makes the "consistent feature stable version" check pass (because it's a new feature)
• when the code is promoted to beta, CURRENT_RUSTC_VERSION will automatically get replaced with the current version

https://std-dev-guide.rust-lang.org/development/stabilization.html#stabilization-pr-checklist

[saethlin]

@craterbot run mode=check-only

[craterbot]

👌 Experiment pr-148271 created and queued.
🤖 Automatically detected try build ddcf048
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-148271 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-148271 is completed!
📊 6 regressed and 4 fixed (726473 total)
📊 1786 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-148271/retry-regressed-list.txt ↩

[Noratrieb]

@craterbot run mode=check-only crates=https://crater-reports.s3.amazonaws.com/pr-148271/retry-regressed-list.txt p=1

[craterbot]

👌 Experiment pr-148271-1 created and queued.
🤖 Automatically detected try build ddcf048
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-148271-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-148271-1 is completed!
📊 1 regressed and 0 fixed (1777 total)
📊 102 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-148271-1/retry-regressed-list.txt ↩

[theemathas]

The only non-spurious failure was from the currying crate. I've minimized the code from that crate to below:

#![feature(unboxed_closures)]
#![feature(fn_traits)]

#[allow(dead_code)]
pub struct Curried<F> {
    func: F,
}

impl<F: FnOnce()> FnOnce<()> for Curried<F> {
    type Output = ();
    extern "rust-call" fn call_once(self, _: ()) {}
}
impl<F: FnMut()> FnMut<()> for Curried<F> {
    extern "rust-call" fn call_mut(&mut self, _: ()) {}
}
impl<F: Fn()> Fn<()> for Curried<F> {
    extern "rust-call" fn call(&self, _: ()) {}
}

pub fn test_mut() {
    let mut i = 0;
    let mut f = || {
        i += 1;
    };
    let mut fj = Curried { func: &mut f };
    fj();
}

Error after this PR:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnMut`
  --> src\lib.rs:22:17
   |
22 |     let mut f = || {
   |                 ^^ this closure implements `FnMut`, not `Fn`
23 |         i += 1;
   |         - closure is `FnMut` because it mutates the variable `i` here
...
26 |     fj();
   |     ---- the requirement to implement `Fn` derives from here
   |
   = note: required for `&mut {closure@src\lib.rs:22:17: 22:19}` to implement `Fn()`
   = note: 1 redundant requirement hidden
   = note: required for `Curried<&mut {closure@src\lib.rs:22:17: 22:19}>` to implement `Fn()`

For more information about this error, try `rustc --explain E0525`.
error: could not compile `currying` (lib) due to 1 previous error

I don't understand why it's erroring though...

[BoxyUwU]

gonna nominate this for T-types. I am not sure to what extent we actually depend on the exact set of Fn* impls in core/std from within the compiler. It would seem plausible to me that method lookup or the trait solver could be making assumptions about the kind of impls that exist.

[programmerjake]

can you add AsyncFn for &mut impl AsyncFn too?

[Kivooeo]

@programmerjake, I have zero understanding of what is happening in async code, is this correct impl (I just copied from AsyncFn for &F and changed it to &mut self

    impl<'a, A: Tuple, F: ?Sized> AsyncFn<A> for &'a mut F
    where
        F: AsyncFn<A>,
    {
        extern "rust-call" fn async_call(&self, args: A) -> Self::CallRefFuture<'_> {
            F::async_call(*self, args)
        }
    }

For context: https://doc.rust-lang.org/src/core/ops/async_function.rs.html#60

[programmerjake]

@programmerjake, I have zero understanding of what is happening in async code, is this correct impl (I just copied from AsyncFn for &F and changed it to &mut self

it looks correct to me

[viliml]

@theemathas I don't understand why it's erroring though...

@BoxyUwU gonna nominate this for T-types. I am not sure to what extent we actually depend on the exact set of Fn* impls in core/std from within the compiler. It would seem plausible to me that method lookup or the trait solver could be making assumptions about the kind of impls that exist.

Desugaring of call expressions into Fn* method invocations happens before closure restrictiveness inference. So the compiler sees that the type may implement Fn if the closure does, and optimistically desugars it into a Fn::call invocation. Later on, it is discovered that the closure does not in fact implement Fn, and therefore Curried doesn't either, and type checking fails.

There's a special case baked in to handle direct closure invocations, but not for invocations of types that contain closures.

This is tracked at #38305.

So basically the lack of this impl allowed you to bypass this bug in some cases, and adding it makes it resurface.

[rust-bors]

☔ The latest upstream changes (presumably #146923) made this pull request unmergeable. Please resolve the merge conflicts.

[apiraino]

hi, checking progress. Is this still waiting on a T-types discussion? thanks for an update :)

[BoxyUwU]

Boxy: I think we decided yes we want to be involved when new impls of Fn are added?
Boxy: which means someone should review that part of this PR, dont know who though :)

from the types nomination