[pre-pre-RFC] `impl trait` inside `impl`, treating trait impls as sort-of inherent items

Idea

Instead of

impl Foo {
  /// docs go here
  fn foo() {}
}
impl Trait for Foo {
}

One would be able to write:

impl Foo {
  /// docs go here
  fn foo() {}
  /// more docs here
  impl trait Trait {
  }
}

There are a few benefits to this:

  1. Less generic boilerplate (see below).
  2. Encouraging more docs.
  3. Eh that's about it really.

There are a few drawbacks to this, tho:

  1. Extra indentation.

There are also some tradeoffs that can be had:

  1. It could be just "impl Trait", but the choice of using "impl trait Trait" makes it clearer that this is acting as an item of the outer "impl". This makes it easier for humans to read. There's no technical reason for it tho.

Motivation

We have these RAII/transaction things:

struct SubtreeHelper<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> where Self: 'r {
    root: &'r mut Parser<'s, P, O, T>,
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> SubtreeHelper<'r, 's, P, O, T> where Self: 'r {
    fn start(value: &'r mut Parser<'s, P, O, T>) -> Self {
        value.consts.protos.push(Default::default());
        Self {
            root: value,
        }
    }

    fn commit(self) -> usize {
        let mut self_ = ManuallyDrop::new(self);
        let proto = self_.root.consts.protos.pop().unwrap();
        let id = self_.root.closed_subtrees.next().unwrap();
        self_.root.consts.protos.insert(id, proto);
        id
    }
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> std::ops::Deref for SubtreeHelper<'r, 's, P, O, T> where Self: 'r {
    type Target = Parser<'s, P, O, T>;

    fn deref(&self) -> &Self::Target {
        &*self.root
    }
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> std::ops::DerefMut for SubtreeHelper<'r, 's, P, O, T> where Self: 'r {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.root
    }
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> Drop for SubtreeHelper<'r, 's, P, O, T> where Self: 'r {
    fn drop(&mut self) {
        // remove "partial" proto
        self.root.consts.protos.pop().expect("SubtreeHelper");
    }
}

struct TagHelper<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> where Self: 'r {
    root: &'r mut Parser<'s, P, O, T>,
    len: usize,
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> TagHelper<'r, 's, P, O, T> where Self: 'r {
    fn start(value: &'r mut Parser<'s, P, O, T>) -> Self {
        let len = value.consts.protos.last().unwrap().len();
        Self {
            root: value,
            len : len,
        }
    }

    fn commit(self) {
        std::mem::forget(self)
    }
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> std::ops::Deref for TagHelper<'r, 's, P, O, T> where Self: 'r {
    type Target = Parser<'s, P, O, T>;

    fn deref(&self) -> &Self::Target {
        &*self.root
    }
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> std::ops::DerefMut for TagHelper<'r, 's, P, O, T> where Self: 'r {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.root
    }
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> Drop for TagHelper<'r, 's, P, O, T> where Self: 'r {
    fn drop(&mut self) {
        let proto = self.root.consts.protos.last_mut().unwrap();
        assert!(proto.len() >= self.len);
        while proto.len() > self.len {
            let _ = proto.pop();
        }
    }
}

They come with a lot of generics. This is less than ideal. Since Drop can't be specialized there's really no good way to get rid of the pointless generics for Deref and DerefMut, so it has to be written this way.

This would look like this with the proposal:

struct SubtreeHelper<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> where Self: 'r {
    root: &'r mut Parser<'s, P, O, T>,
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> SubtreeHelper<'r, 's, P, O, T> where Self: 'r {
    fn start(value: &'r mut Parser<'s, P, O, T>) -> Self {
        value.consts.protos.push(Default::default());
        Self {
            root: value,
        }
    }

    fn commit(self) -> usize {
        let mut self_ = ManuallyDrop::new(self);
        let proto = self_.root.consts.protos.pop().unwrap();
        let id = self_.root.closed_subtrees.next().unwrap();
        self_.root.consts.protos.insert(id, proto);
        id
    }

    impl trait std::ops::Deref {
        type Target = Parser<'s, P, O, T>;

        fn deref(&self) -> &Self::Target {
            &*self.root
        }
    }

    impl trait std::ops::DerefMut {
        fn deref_mut(&mut self) -> &mut Self::Target {
            self.root
        }
    }

    impl trait Drop {
        fn drop(&mut self) {
            // remove "partial" proto
            self.root.consts.protos.pop().expect("SubtreeHelper");
        }
    }
}

struct TagHelper<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> where Self: 'r {
    root: &'r mut Parser<'s, P, O, T>,
    len: usize,
}

impl<'r, 's, P: Borrow<str> + Ord, O: Borrow<str> + Ord, T: PatternTypes> TagHelper<'r, 's, P, O, T> where Self: 'r {
    fn start(value: &'r mut Parser<'s, P, O, T>) -> Self {
        let len = value.consts.protos.last().unwrap().len();
        Self {
            root: value,
            len : len,
        }
    }

    fn commit(self) {
        std::mem::forget(self)
    }

    impl trait std::ops::Deref {
        type Target = Parser<'s, P, O, T>;

        fn deref(&self) -> &Self::Target {
            &*self.root
        }
    }

    impl trait std::ops::DerefMut {
        fn deref_mut(&mut self) -> &mut Self::Target {
            self.root
        }
    }

    impl trait Drop {
        fn drop(&mut self) {
            let proto = self.root.consts.protos.last_mut().unwrap();
            assert!(proto.len() >= self.len);
            while proto.len() > self.len {
                let _ = proto.pop();
            }
        }
    }
}

Reference-level guide

The syntax:

impl<'a, T, const N: usize> Foo<'a, T, N> where T: Bar {
  // some items omitted
  impl<U> trait Trait<U> where T: Trait<U> {
  }
}

is sugar for:

impl<'a, T, const N: usize> Foo<'a, T, N> where T: Bar {
  // some items omitted
}

impl<'a, T, U, const N: usize> Trait<U> for Foo<'a, T, N> where T: Bar + Trait<U> {
}

Note that both the generics and the bounds simply combine. No special provisions are made for overriding the inherent impl's bounds at this time.

Rustdoc changes

The impl trait items should appear inside the inherent impl. They should show the item-level docs by default, but not the subitem docs. That is,

impl Foo {
  /// docs here
  trait impl Bar {
    /// more docs here
    fn baz() {}
  }
}

should, by default, show as

[-] impl Foo

[-] [+] trait impl Bar

docs here

and can be expanded to

[-] impl Foo

[-] [-] trait impl Bar

docs here

[-] fn baz()

more docs here

or swallowed to

[-] impl Foo

[+] trait impl Bar

This is consistent with how other items are displayed.

Previous discussion

There have been discussions about this on the community Discord, mainly with regards to the documentation side of things. We don't know of any similar discussions on here, however.

2 Likes

The problem of verbose impl generics should be addressed by implied bounds

3 Likes

hm... thanks for pointing out implied bounds. it seems it requires chalk?

we'd argue impl trait in impl struct would have more use-cases tho, and doesn't interact badly with inference (as it's just sugar, mostly). and now that referring to a trait in type position requires an explicit dyn, we can have this without ambiguity and stuff (altho having impl trait Foo seems less ambiguous, at least to the user).

(it would also, potentially, make better docs, by grouping trait impls with the relevant impl blocks. not sure how useful this would be tho.)

Decided to make a proc macro for this: https://crates.io/crates/impl_trait

(doesn't solve the rustdoc problem tho)

I think it would be better for the syntax to be impl Foo for Self. This reuses existing syntax instead of making something new.

Sure.

But then, should there be impl Foo<Self> for Bar? (e.g. impl From<Self> for Bar?) This feels like it could be confusing, precisely because of existing syntax - impl From<Self> for Bar already exists today, and it maps the Self to Bar!

I think impl Foo<Self> for Self isnt that confusing looking. Or maybe I'm misunderstanding your point.

Allowing trait impls only for Self within the inherent impl block seems reasonable. In that case there's no ambiguity on what a Self-parameter in a generic trait refers to, because the two possibilities (the type it's for and the type for the containing impl block) are the same type by definition.

Sure, impl Trait<Foo> for Bar is related to the type Foo, but it's not a trait implemented by that type, so shouldn't be contained in an inherent impl block for that type.

3 Likes

Yes, but also the for Self feels redundant if it's the only thing that's allowed...

I don't know how to deal with the ambiguous Self binding within the inner impl, but it would be nice to be able to define traits on objects related to Self without having to repeat the generic parameters. For example:

impl<'a> IntoIterator for &'a Self { ... }