regression: conflicting trait impls because `Box<dyn Fn*>` now implements `Fn*`.

[adrian17]

Most likely related to #59500 ?

Simplified reproduction:

pub type FuncWrapper = Box<dyn Fn() + 'static>;

pub trait IntoFuncWrapper {
    fn into_func(self) -> FuncWrapper;
}

impl<F> IntoFuncWrapper for F where F: Fn() + 'static {
    fn into_func(self) -> FuncWrapper { Box::new(self) }
}

impl IntoFuncWrapper for FuncWrapper {
    fn into_func(self) -> FuncWrapper { self }
}

fn main() { }

Before: compiles on stable, beta, previous nightly
On latest nightly:

error[E0119]: conflicting implementations of trait `IntoFuncWrapper` for type `std::boxed::Box<(dyn std::ops::Fn() + 'static)>`:
  --> src/main.rs:11:1
   |
7  | impl<F> IntoFuncWrapper for F where F: Fn() + 'static {
   | ----------------------------------------------------- first implementation here
...
11 | impl IntoFuncWrapper for FuncWrapper {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `std::boxed::Box<(dyn std::ops::Fn() + 'static)>`

error: aborting due to previous error

I do not know whether that was expected and should be fixed on my side, or is it an unintended breakage.

[Centril]

Minimized reproduction:

trait B {}

impl<C: FnOnce()> B for C {}

impl B for Box<dyn FnOnce()> {}

The issue here is that Box<dyn FnOnce()>: FnOnce() and so if you set C = Box<dyn FnOnce()> you have impl B for Box<dyn FnOnce()> {} twice => overlap.

[pnkfelix]

Is this a T-libs issue or a T-compiler one?

Was this was expected fallout from another change?

Tagging as P-high for now, due its nature as a regression.

[cramertj]

This is an expected regression due to introducing a new implementation of a #[fundamental] trait (the Fn traits are all #[fundamental]). Adding an implementation of a #[fundamental] trait is a breaking change. However, the missing impls of Fn traits for Box are a long-standing issue that we've been wanting to resolve. The addition of these impls has significant practical and ecosystem impact, in that FnOnce is now object-safe.

If there are key ecosystem crates that relied upon these impls not existing, then we should hold off on the introduction of these impls and give them time to migrate. However, I believe that it is important that we achieve our long-standing goal of object-safe FnOnce and continue to prioritize landing these impls on stable.

cc @rust-lang/lang @rust-lang/libs to call attention to the introduction of new trait impls causing breakage.

[pnkfelix]

so is the next step to attempt some sort of evaluation to figure out if there exist "key ecosystem crates" relying on the absence of these impls?

I assume our main method for doing this (beyond the crater runs that have already been done) is to just wait and see:

• what other bugs get filed that look like this one, and
• what comments are posted with indications of which particular crates were broken

right?

[Centril]

I assume our main method for doing this (beyond the crater runs that have already been done) is to just wait and see:

What you could do is to open a PR that is effectively a no-op and crater run that one. Otherwise we'll find out in the usual beta crater runs and stuff.

[pnkfelix]

In any case I don't think there is anything actually actionable here on our side until we see evidence that this change broke "key ecosystem crates" in the wild.

With that in mind, I'm going to downgrade this to P-medium.

[pnkfelix]

I assume our main method for doing this (beyond the crater runs that have already been done) is to just wait and see:

What you could do is to open a PR that is effectively a no-op and crater run that one. Otherwise we'll find out in the usual beta crater runs and stuff.

Ah true; I assumed there had already been crater runs, but I don't know why I thought that.