Crates that use the "inherent trait method" pattern

1

My firm belief is that it is nonsense for a crate to require you to use a trait in order to call methods on types defined by that crate, when those methods feel like they should be part of the type.

I can think of plenty of examples of what I consider to be bad:

  • You can do many things out of the box with a stack::ArrayVec, but if you want to push to it, you need to import stack::Vector.
  • mpi::collective::Root. Not only is this highly important functionality hidden under some unusually-named trait in some arbitrarily-named submodule, but it's only implemented for one type. Why aren't these simply inherent methods on that type!?
  • You must import error_chain::ChainedError to call the .chain_err method on types generated by error_chain. This is a particularly nasty example because you usually want to call this on error types generated in other crates, and those other crates might depend on different versions of error_chain!

These have a number of issues:

  • Having to import extra traits to use core functionality of a type is silly. If you later refactor the code that calls the method and move it to another module, you have to move that use.
  • There is a disconnect between the name you must import versus the method name you are trying to call. That's more things for the user to remember. (the same issue with publicly exposing an overly-nested hierarchy)
  • Most of these traits are things I wouldn't want to import even if I wanted to use them generically. (I'd rather use them as crate-qualified names). But if they are to be used for their methods, they absolutely must be imported. This leads to a messy namespace.
  • If there's static methods and blanket impls on uncovered T, then it can cause ambiguities that are impossible to resolve.

In my opinion, many of these examples (with the exception of mpi::Root which ought not exist) should have inherent methods identical to the trait methods. I.e. ArrayVec should have fn push in addition to Vector::push:

Why both? Easy:

  • ArrayVec::push is for people who want to use ArrayVec.
  • Vector::push is for people who want to be generic over V: Vector.

Now, I was asked to find examples of crates that actually do things the way I suggest. The trouble is, this is the sort of thing that, when a crate does it right, you don't notice.

Are there popular crates that provide inherent versions of trait methods for ergonomics? Or does nobody agree with these ideals?

(note: you can't say frunk; I added the inherent methods to that crate)

3 Likes
2

As the num maintainer, I'll confess that it's a bit annoying to import traits from num-traits and num-integer just to use them on other concrete num types like BigInt or Complex. Maybe I should mirror all of those to inherent methods too.

1 Like
3

Perhaps not identical to, but delegating to (and with #[inline] or even #[inline(always)], ofc), so that the code isn't duplicated? Or even the other way around - everything is implemented directly at the struct, and traits are delegating their work to this implementation?

4

Oh, but of course! I simply meant identical signatures.

I would say this is usually the way to do it: (trait delegating to method)

impl<T> ArrayVec<T> {
    pub fn push(&mut self, value: T) {
        // implementation here
    }
}

impl<T> Vector for ArrayVec<T> {
    type Item = T;

    #[inline(always)]
    fn push(&mut self, value: T) { self.push(value) }
}

In crates where the trait is required as an implementation detail, you can have it go the other way around, with inherent methods that delegate to the trait. That's what I did in Frunk:

impl<Head, Tail> IntoReverse for HCons<Head, Tail>
where
    Tail: IntoReverse,
    <Tail as IntoReverse>::Output: Add<HCons<H, HNil>>,
{
    type Output = ...
    fn into_reverse(self) -> Self::Output { ... }
}

impl<Head, Tail> HCons<Head, Tail> {
    #[inline(always)]
    pub fn into_reverse(self) -> <Self as IntoReverse>::Output
    where
        Self: IntoReverse,
    { IntoReverse::into_reverse(self) }
}
5

I believe ideally we need language support for this, see this RFC:
https://github.com/rust-lang/rfcs/pull/2375
Duplicating methods explicitly will result in code and documentation bloat.

4 Likes
6

This wouldn't just be a problem in 3rd party crates. I know people have been wanting collection traits (kinda like C#'s IList<T>) in std, so imagine needing to import the std::collections::Dictionary trait if you wanted to insert an item into a dictionary...

You could almost say that most crates promoting use some_crate::prelude::* would benefit from this inherent trait method pattern.

Would the failure crate's Error::context() method count? The failure::Fail trait provides a context() method to attach extra context to an error, but the Error type has context() as an inherent method (although Error doesn't implement Fail).

I guess another reason why it's hard to find examples like this is the vast majority of traits are used for some form of dependency injection or to make a function/type accept a larger range of inputs. As such, the trait tends to be used as a function argument and not for its methods.

7

Possibly. There are some legitimate uses of a prelude, such as for crates like rayon that provide extension traits for standard library types.

I would also say that it's inclusion of ParallelIterator is fair, despite not being an extension trait. I mean, technically these methods could be inherent on all of the ParallelIterators defined in the crate... but there are far too many, and people don't usually think about those concrete types anyways. When people use these methods, they're thinking about working with ParallelIterators in general.

Would the failure crate’s Error::context() method count? The failure::Fail trait provides a context() method to attach extra context to an error, but the Error type has context() as an inherent method (although Error doesn’t implement Fail) .

It's close (insofar as it provides an inherent method to make things "just work" according to user expectation), but I would not count it. As you mention, Error does not implement Fail (it would lead to overlap), so this method has to exist in both places.

(though I guess you could argue that failure chose not to have a trait dedicated to the context method...)

1 Like
8

Another way to phrase this is what are the disadvantages to mirroring the methods?

So far I have seen:

  • documentation bloat and confusion: the user might wonder why a method is implemented twice, although this is probably fairly obvious.
  • code bloat/duplication: this kind of code can be fairly annoying to write, read and maintain.
  • backwards compatibility: there might be minimal backwards compatibility concerns, such as if someone's type was previously using a different trait, and now will be using the inherent method.

I think the first two issues could be solved with a macro that uses #doc(hidden). The last one is probably not a major issue.

Are there any other down sides? @ExpHP suggested I do this in this bug, but I think I'd rather prefer a macro that does it for me :smiley:

1 Like
9

That's not such a bad idea. I imagine you could write a #[generate_inherent_methods] attribute which wraps your impl Trait for Foo block and creates inherent methods for you. Attribute macros have next to no maintenance cost, so that's a massive bonus.

1 Like
10

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.