Open Veetaha opened 1 year ago
@rustbot label +T-libs-api
I may be wrong, but I think this is a known issue that can't be fixed because it would break backwards compatibility.
But if I'm wrong will other people please chime in. (It's much quicker to get an answer on the internet by posting something incorrect than by asking a question 😛)
@derekdreery I came across this comment while working on #113464 which supports your backwards compatibility claim: https://github.com/rust-lang/rust/blob/master/library/core/src/error.rs#L409
But I'll admit I don't understand why removing the Sized
bound via ?Sized
is backwards incompatible and am interested to have that explained more clearly...
Like if you had bounds of MyTrait
and YourTrait
on a given generic type parameter and a concrete type MyConcreteType
that implements both traits but remove the MyTrait
bound on the type parameter in question, MyConcreteType
should still be valid because it still implements YourTrait
-- right? Intuitively, it seems to me that by removing the implicit Sized
bound for T
in Box<T>
via ?Sized
any concrete type loitering out there in the wild that is Sized
should still be valid for impl<T: core::error::Error + ?Sized> core::error::Error for Box<T>
.
I think I just realized the issue here -- it's the downstream usages of Box
I believe this is because Box
is #[fundamental]
, which has implications for trait impls: it allows crates to implement traits on Box<MyType>
directly, which is not normally allowed for a crate that doesn't own the Box
type.
It might still be acceptable to relax the bounds though, if a crater run shows no breakage.
Okay I just tried this out:
zsh/5 10140 (git)-[relax-the-sized-requirement-for-box-dyn-error]-% git diff
diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs
index 8697a77db3b..8ad94ad1eff 100644
--- a/library/alloc/src/boxed.rs
+++ b/library/alloc/src/boxed.rs
@@ -2430,7 +2430,7 @@ fn from(err: Cow<'a, str>) -> Box<dyn Error> {
}
#[stable(feature = "box_error", since = "1.8.0")]
-impl<T: core::error::Error> core::error::Error for Box<T> {
+impl<T: core::error::Error + ?Sized> core::error::Error for Box<T> {
#[allow(deprecated, deprecated_in_future)]
fn description(&self) -> &str {
core::error::Error::description(&**self)
But when I try compiling I get
error[E0119]: conflicting implementations of trait `From<Box<dyn core::error::Error>>` for type `Box<dyn core::error::Error>`
--> library/alloc/src/boxed.rs:2207:1
|
2207 | impl<'a, E: Error + 'a> From<E> for Box<dyn Error + 'a> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: conflicting implementation in crate `core`:
- impl<T> From<T> for T;
error[E0119]: conflicting implementations of trait `From<Box<dyn core::error::Error + Send + Sync>>` for type `Box<dyn core::error::Error + Send + Sync>`
--> library/alloc/src/boxed.rs:2240:1
|
2240 | impl<'a, E: Error + Send + Sync + 'a> From<E> for Box<dyn Error + Send + Sync + 'a> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: conflicting implementation in crate `core`:
- impl<T> From<T> for T;
So impl<T: core::error::Error + ?Sized> core::error::Error for Box<T>
now means that Box<dyn Error>
is an Error
implementation which means that impl<T> From<T> for T
overlaps with the impls at line 2240 and 2207 in library/alloc/src/boxed.rs
since they are basically now converting possibly from a Box<dyn Error>
to a Box<dyn Error>
. I find this a little bit confusing.
Is this happening because of monomorphization? At compile time because one possible known implementation of Error
is Box<Error + ?Sized>
aka Box<dyn Error>
which monomorphs (is that a word? is there such thing as monomorphin' time? (bad Power Rangers joke, can't help myself)) to (taking the impl at line 2207 of library/alloc/src/boxed.rs
shown in the compiler error as an example) to something like:
impl From<Box<dyn Error +'a>> for Box<dyn Error + 'a> { ... }
And this would also be monomorphed from impl<T> From<T> for T
, hence the conflict.
Is there a good way to work around this? Add a Sized
bound to lines 2207 and 2240 in library/alloc/src/boxed.rs
? When I try that, I get essentially the same error. :thinking:
As for today the following blanket implementation exists: https://github.com/rust-lang/rust/blob/e702534763599db252f2ca308739ec340d0933de/library/alloc/src/boxed.rs#L2443-L2458
The problem with this is that
T
is required to beSized
, so, for example,Box<dyn Error>
doesn't implementError
. It looks like a straightforward and intuitive assumption thatBox<dyn Error>
should implementError
trait, but it doesn't.Going even further, there is an inconsistency between the blanket impls for
Arc
as well, because there exists an analogous impl forArc
that has relaxedSized?
requirement:https://github.com/rust-lang/rust/blob/e702534763599db252f2ca308739ec340d0933de/library/alloc/src/sync.rs#L2769-L2788
Use Case
The use case where I stumbled with this problem is where I wanted to put
Box<dyn Error>
as asource
error inthiserror
error enum, but I couldn't because that type doesn't implement Error trait.Workarounds
The workaround for this problem is to use
Arc<T>
instead ofBox<T>
when an Error impl is required forSized?
type.