Language inconsistency when implementing a trait

language design
1

I'm escalating this from users.rust-lang.org because the weirdness just keeps rising.

Basically the issue is that this is accepted:

impl<T> TryFrom<RcBar<T>> for Rc<T> { 
    // code here
}

impl<T> TryFrom<Rc<T>> for RcBar<T> {
    // code here
}

whereas this is not:

impl<T> TryFrom<BoxBar<T>> for Box<T> { 
    // code here
}

impl<T> TryFrom<Box<T>> for BoxBar<T> {
    // code here
}

In the above snippets, RcBar<T> and BoxBar<T> are local types, yet the impls for RcBar<T> and for BoxBar<T> are rejected regardless. So the RFC doesn't even make any sense there.

Urlo user @trentj pointed me to the #[fundamental] attribute as well as RFC 1023, but that doesn't really help me forward. In fact all it does is tell me that #[fundamental] makes Box<T> behave very differently from Rc<T> and Arc<T> in a very important respect, but fails to explain why I should care as a user other than some hand-wavey mentions of safety and coherence. Which I'd be willing to believe in principle, except that it's:

  1. apparently not needed for Rc<T> and Arc<T> so it's not consistent (instead a choice was made to make it consistent with &T and &mut T for whatever reason) and
  2. actively harming my ability to get useful work done because of the unexpected inconsistency.

From the RFC:

This RFC proposes a shift that attempts to strike a balance between the needs of downstream and upstream crates. In particular, we wish to preserve the ability of upstream crates to add impls to traits that they define, while still allowing downstream creates to define the sorts of impls they need.

I don't know if I can call this "mission failed", but it's definitely no success because so far I haven't been able to write the impl blocks I need to for Box<T>, and it seems that that is directly due to the implementation of this RFC.

Can anyone tell me what I need to do to write the impl blocks I need to and get around the #[fundamental] stumbling block?

(I'm silently hoping that the answer isn't the forever-nightly-feature called impl specialization...)

2 Likes
2

I think re-rebalancing coherence (RFC#2451) is partially to blame here for the inconsistency. Going back to 1.34.0, when TryFrom was stabilized and re-rebalancing coherence was not yet in, I get a different set of errors that also disallows the impl on Rc:

use std::rc::Rc;
use std::convert::TryFrom;

struct MyRc<T>(Rc<T>);
struct MyBox<T>(Box<T>);

impl<T> TryFrom<MyRc<T>> for Rc<T> {}
impl<T> TryFrom<Rc<T>> for MyRc<T> {}

impl<T> TryFrom<MyBox<T>> for Box<T> {}
impl<T> TryFrom<Box<T>> for MyBox<T> {}

1.41.0 (re-rebalancing coherence)

error[E0119]: conflicting implementations of trait `std::convert::TryFrom<MyBox<_>>` for type `std::boxed::Box<_>`:
  --> src/lib.rs:10:1
   |
10 | impl<T> TryFrom<MyBox<T>> for Box<T> {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: conflicting implementation in crate `core`:
           - impl<T, U> std::convert::TryFrom<U> for T
             where U: std::convert::Into<T>;
   = note: downstream crates may implement trait `std::convert::From<MyBox<_>>` for type `std::boxed::Box<_>`

error[E0119]: conflicting implementations of trait `std::convert::TryFrom<std::boxed::Box<_>>` for type `MyBox<_>`:
  --> src/lib.rs:11:1
   |
11 | impl<T> TryFrom<Box<T>> for MyBox<T> {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: conflicting implementation in crate `core`:
           - impl<T, U> std::convert::TryFrom<U> for T
             where U: std::convert::Into<T>;
   = note: downstream crates may implement trait `std::convert::From<std::boxed::Box<_>>` for type `MyBox<_>`

error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`MyBox<T>`)
  --> src/lib.rs:10:6
   |
10 | impl<T> TryFrom<MyBox<T>> for Box<T> {}
   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`MyBox<T>`)
   |
   = note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

1.34.0 (pre-re-rebalancing coherence)

error: conflicting implementations of trait `std::convert::TryFrom<MyBox<_>>` for type `std::boxed::Box<_>`: (E0119)
  --> src\main.rs:10:1
   |
10 | impl<T> TryFrom<MyBox<T>> for Box<T> {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: #[deny(incoherent_fundamental_impls)] on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #46205 <https://github.com/rust-lang/rust/issues/46205>
   = note: conflicting implementation in crate `core`:
           - impl<T, U> std::convert::TryFrom<U> for T
             where U: std::convert::Into<T>;
   = note: downstream crates may implement trait `std::convert::From<MyBox<_>>` for type `std::boxed::Box<_>`

error: conflicting implementations of trait `std::convert::TryFrom<std::boxed::Box<_>>` for type `MyBox<_>`: (E0119)
  --> src\main.rs:11:1
   |
11 | impl<T> TryFrom<Box<T>> for MyBox<T> {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #46205 <https://github.com/rust-lang/rust/issues/46205>
   = note: conflicting implementation in crate `core`:
           - impl<T, U> std::convert::TryFrom<U> for T
             where U: std::convert::Into<T>;
   = note: downstream crates may implement trait `std::convert::From<std::boxed::Box<_>>` for type `MyBox<_>`

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
 --> src\main.rs:7:1
  |
7 | impl<T> TryFrom<MyRc<T>> for Rc<T> {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
  |
  = note: only traits defined in the current crate can be implemented for a type parameter

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
  --> src\main.rs:10:1
   |
10 | impl<T> TryFrom<MyBox<T>> for Box<T> {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
   |
   = note: only traits defined in the current crate can be implemented for a type parameter

Ooh, further tidbit: the Box impl used to be allowed, and was deprecated/removed with #46205. That issue contains some further information about why this is incoherent.

So two takeaway points: under rebalancing coherence, both of these cases are disallowed; re-rebalancing coherence allows it for non-fundamental (covering) types. And specialization doesn't even help you at all.

#[fundamental] is inherently about negative reasoning. A #[fundamental] trait can be assumed to not be implemented (e.g. Fn* and Sized) if it is not currently implemented. A #[fundamental] type is slightly different; per my reading of re-rebalancing coherence, a fundamental type's only quality is that its locality is always that of its (singular) type argument, and as such cannot "cover" a type variable.

Without knowing the exact use case of your BoxBar, it's not really possible to suggest other paths. My only suggestion would really be to, if at all possible, use Box<Bar<T>> (where Bar is a repr(transparent) wrapper) rather than a custom box type.

1 Like
3

I initially didn't understand what #[fundamental] meant and like many things in Rust, grasping the concept and its implications (and justifications) was... difficult (Rust has so much jargon, some of which is newly invented and some of which is academic PLT stuff that's way over my head). Anyway, I found this Q&A on StackOverflow to be enlightening and it might help you understand, @jjpe, why certain impls involving Box<T> aren't permitted.

I think the compiler's error messages could be clarified with a link to something that explains fundamental types and why/how they're special (in a beginner-friendly way).

2 Likes
4

I think part of the issue is that, if I'm reading between the lines correctly, #[fundamental] was originally introduced as a sort of tentative "we need to circle back to this later" kind of solution, in the hopes something better would come later:

This attribute is unstable because it is not clear whether it will prove to be adequate or need to be generalized; this part of the design can be considered somewhat incomplete, and we expect to finalize it based on what we observe after the 1.0 release.

RFC 1023 Rebalancing Coherence

But at this point it seems unlikely we'll get rid of it, and extremely unlikely we'll ever end up on a radically different mechanism, so we probably should commit to doing a proper bikeshed on this terminology and improving some of the error messages and documentation (e.g. the Box docs don't even mention that it's #[fundamental]).

Might as well start here. I can't think of anything I actually feel good about, but just as a strawman to get the ball rolling: how does #[no_new_blanket_impls] for types and #[no_new_impls_for_existing_types] for traits sound?

2 Likes
5

Honestly, fundamental is a good descriptor of why the traits/types are like they are, rather than what the actual impact is. We just need to actually better document and expose the fundamental nature and what impact that has on coherence.

And perhaps, if we embrace the fundamental terminology, we should move towards marking the other primitive types (*[const|mut] _, tuples, [_], [_; N], and fn pointers) as fundamental. After all, these are fundamental builtin types to the language, not "just" another struct definition in core/std. This would, of course, require finalizing and committing to the exact set of core/std traits that any fundamental types implement, so is more of a "making fundamental consistent" long term goal than a PR to make soon.

Referencing the re-rebalancing coherence RFC is probably the better RFC to reference than rebalancing coherence, as it redefines and respecifies the entirety of coherence to be more consistent, especially where #1023 and #1105 contradict each other.

6

I disagree that "fundamental is a good descriptor". I had exactly zero clue what "fundamental type" meant or why it had any impact on traits/impls. Even after reading that StackOverflow Q&A I previously linked to, I still don't really see why "fundamental" is a good descriptor.

Naming things is hard. I don't have any better suggestions. But I would like to better understand why you think fundamental is a good descriptor, because I think you have a perspective or insight that I lack.

3 Likes
7

Just to circle back for a second to the issue that got the ball rolling. How do I implement TryFrom for Box<T>? The blanket impl defined in the stdlib is not acceptable to rustc, so I need something better.

Here's some context, as I plan to open source the crate once it's actually usable anyway: I'm writing a derive macro that computes deltas between instances of a certain type. These deltas are de/serializable using Serde. The deltas are computed and applied to types by means of a trait called DeltaOps, and that trait has an associated type representing the Delta for the implementing type. Here's the thing, I need TryFrom to convert the Delta types each way ie one TryFrom impl to go from implementing type to the corresponding Delta type, and one to convert back. The conversion from Delta to the implementing type can fail and I need to conversions to be symmetric, so both directions implement TryFrom rather than From.

This seems to become an issue with Box<T> because of the fundamental attribute. So. How do I kick these intentional but undesirable side-effects out of the equation?

1 Like
8

The answer seems to be you don't. It's in violation of re-rebalancing coherence, because Box<T> is an uncovered type parameter; it's not an upstream type, it's not a local type: it's potentially a downstream type, local to a downstream crate. You need to somehow cover the type parameter (if it needs to exist) with a local type.

Just to sketch out the APIs you're trying to provide, to understand what you're trying to do,

trait DeltaOps {
    type Delta: Serialize + Deserialize + TryFrom<Self> + TryInto<Self>;
    fn delta(left: &Self, right: &Self) -> Self::Delta;
}

...and then your example over on urlo has struct BoxDelta<T>(<T as DeltaOps>::Delta) for... some reason?

Given that your example impls for Box/Rc are just "clone out the value, use it's TryInto", I'd just leave it to the user to clone out the value, honestly.

The one reason I can see to want a Delta type is to make naming an otherwise unnamable type more easily namable. But a type alias handles this fine:

trait DeltaOps {
    type Delta;
    fn delta(left: &Self, right: &Self) -> Self::Delta;
}

type Delta<T: DeltaOps> = <T as DeltaOps>::Delta;

impl DeltaOps for usize {
    type Delta = isize;
    fn delta(left: &Self, right: &Self) -> isize {
        right.wrapping_sub(*left) as isize
    }
}

fn main() {
    let x: Delta<usize> = DeltaOps::delta(&0, &10);
    dbg!(x);
}
3 Likes
9

The idea is that if MyType is the type for which a DeltaOps impl is being written / generated, then<MyType as DeltaOps>::Delta is the type representing the Delta for MyType. Thus, instances of <MyType as DeltaOps>::Delta represent individual delta's between instances of MyType. This delta type is generated by the derive macro, and thus both its name and structure are known at that time, but it is useful to be able to talk generically about delta types (e.g. in the return type of the DeltaOps::delta() method, or in the reverse operation not shown here, fn apply_delta(&self, &Self::Delta) -> Self), hence the associated type. The associated type is also not the issue here, it's working as it's supposed to. So I'm not sure why you're focusing on that. The issue is the TryFrom<T> impls, and specifically for Box<T> although now I should probably stretch that up to "types marked as #[fundamental]".

The entire point of the derive is for the declaration to consist of a one-liner on top of a type: #[derive(Delta)]. Keep in mind that if a user need to do manual work, then for complex types (i.e. many layers of composition deep) it can be that in order to use this derive macro they'd suddenly need to implement a bunch of traits for a bunch of types. So no, asking the user to do a lot of the work to support deltas is not a workable solution, just like it wouldn't be okay to ask a user to manually write a Deserialize impl for a dumb data object (i.e. no special behavior of any kind - just pure data representation).

The TryFrom<T> impls should either be present with the rest of the infrastructure (i.e. where the DeltaOps trait is defined - this is mainly for types defined in stdlib, e.g. Rc<T> or Box<T>) or generated by the derive macro (i.e. to convert between a type that is generated, which is a delta type, and the original type that the delta type is based on). In either case the user shouldn't have to write anything except their type and the 1-liner containing the call to the derive macro.

This #[fundamental] business has taken away a valuable piece of functionality, and TBH I want it back rather than resort to nasty kludges.

10

What I'm missing is why you need to implement TryFrom in the first place, or rather, why you have your "BoxBar" type that you want/need to be TryFrom/Into Box genetically.

The reason I ask about the associated type is that the only use you've given for "BoxBar" is "struct BoxBar<T: FooOps>(T::Bar);".

On top of that, a conversion that would be as well sold to be from T or &T as Box<T>.

11

The TryFrom impls exist because the delta types exist, and it must be possible to interconvert (in a controlled way) between a value of a type and a value of its corresponding delta type. It's a very useful building block in generating deltas, while making sure those delta's are de/serializable.

The de/serialization is one reason that delta's need to have their own actual type.

That's easy, that code only exists to enable types that internally use Box<T> to use #[derive(Delta)]. The data flow can go both ways so it must be possible to create a new Box from a BoxDelta.

References aren't supported since it's fundamentally impossible to instantiate a completely new borrow without first having something to borrow from. Nobody that understands deltas as well as view structs would even try this, because it makes about as much sense as trying to add the string "blah" to the number 1.

12

Can you not just impl<D: DeltaOps> DeltaOps for Box<D> in the crate defining DeltaOps and use Box everywhere?

13

The impl you refer to exists as well. It's required but not sufficient.

But in the meantime I think I have a workable solution: Ditch TryFrom in favor of a trait that effectively does the same thing, let's call it Convert, that is not part of stdlib. The idea is that since Convert isn't tainted by #[fundamental] semantics, I can do whatever I want with it.

14

Notwithstanding #[fundamental], this would be a good use case for negative trait impls. The error message correctly notes that a downstream crate could add an impl like

impl From<Box<Foo>> for MyBox<Foo>

where Foo is a type defined by that crate. That would then trigger the standard library's impl<T, U> Into<U> for T where U: From<T>, which would in turn trigger the standard library's impl<U, T> TryFrom<T> for U where T: Into<U>, conflicting with your impl.

If you could prevent downstream crates from doing that by writing:

impl<T> !From<Box<A>> for MyBox<B> {}

then in theory the compiler could allow your TryFrom impl.

As it happens, there's an active rustc PR to allow negative impls for arbitrary traits. The PR as it stands does not affect coherence – the coherence check would simply ignore negative impls – but @nikomatsakis has said that's a future goal.

Note that even after this feature is implemented, it would have to go through the RFC process and then the stabilization process before it could be stabilized. Don't expect it anytime soon, especially if you're not using nightly. But it does affect the conversation about whether to propose other language changes to solve your problem.

1 Like
15

@comex I had expected TryFrom and TryInto to have a similar relationship to From and Into have. I did not expect interdependencies between TryFrom/TryInto on the one hand and From/Into on the other. No wonder my attempt failed. Very unintuitive I must say, I still don't understand why a fallible conversion should necessarily trigger an infallible one, and if you happen to know why I'm all ears.

That also means that ditching TryFrom and TryInto in favor of my own conversion traits is more likely to work.

It effectively also means that the TryFrom/TryInto traits are defined in a way that, under certain conditions (which are met here), makes them defeat their own purpose.

16

It's the other way around, Into imples TryFrom, with the error case being std::convert::Infallible. The reason it goes from Into to TryFrom is detailed in the PR here:

1 Like
17

I see.

All the same, assuming that my guess works out*, I think I made the right choice in duplicating (at least in spirit) TryFrom and TryInto as it also allows me to change the trait signatures while I'm at it.

*which it seems to, but I'm not done yet

18

I'm still not certain why you need a custom Box variant in the first place. The rough expansion I'm imagining is

trait Diff {
    type Diff;
    fn diff(left: &Self, right: &Self) -> Self::Diff;
    fn apply(to: &mut Self, diff: Self::Diff);
}

struct [<#type Diff>] {
  #(
    #field: <#field_type as Diff>::Diff,
  )*
}

impl Diff for #type {
    type Diff = [<#type Diff>];
    fn diff(left: &Self, right: &Self) -> [<#type Diff>] {
        [<#type Diff>] {
          #(
            #field: Diff::diff(&left.#field, &right.#field),
          )*
        }
    }
    fn apply(to: &mut Self, diff: [<#type Diff>]) {
      #(
        Diff::apply(&mut to.#field, diff.#field);
      )*
    }
}

I fail to see the real requirement of custom container types here, though it is, of course, possible that I missed something.

19

I neither need nor want a custom Box variant. That would be an ugly hack, which I would be forced to use if using my own trait definitions turn out not to work.

What I need is to be able to derive any necessary impl blocks for some traits for the types that are annotated with a #[derive(Delta)], as well as for the types that are generated in the process. But #[fundamental] mucks that up by preventing impl blocks from being written for certain trait/type combos, one of which is TryFrom / Box<T>.

20

Ok, straight to the point:

What is the BoxBar<T> type used for. (Why does this need to be a distinct type from Box.)