Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: extension trait conventions #445

Merged
merged 2 commits into from
Nov 25, 2014

Conversation

aturon
Copy link
Member

@aturon aturon commented Nov 5, 2014

This is a conventions RFC establishing a definition and naming
convention for extension traits: FooExt.

Rendered

@Gankra
Copy link
Contributor

Gankra commented Nov 8, 2014

Should we be conflating all the different kinds of extension here? There's extension traits for:

  • supporting object-safety
  • "special cases" of a base type (this one was distinguished in the proposal)
  • implementing methods on "special" types like int or [T]
  • extending a type someone else defined (similar to the previous)
  • arguably the operator traits are a kind of extension trait, as it doesn't seem reasonable to generically accept anything that just impl's Index or whatever.

Obviously, these lines are a bit blurry.

Overall I like the recommendation (distinguish traits for generic programming from other traits with a simple suffix), I just want to be sure we aren't conflating unrelated concepts.

@aturon
Copy link
Member Author

aturon commented Nov 10, 2014

@gankro Thanks for the feedback! Here are some thoughts broken down by your categories:

  • "special cases" of a base type (this one was distinguished in the proposal)

Eventually, this will be handled by where clauses at the method level, which will no longer require breaking things out into extra crates. It's not clear whether this will happen before 1.0, though, so we may have to freeze some extension traits as more granular than we'd otherwise like.

  • implementing methods on "special" types like int or [T]

I've long wished we could do this another way, but there hasn't been a strong enough justification for the work, yet.

That said, there's a separate -Prelude suffixed for prelude extension traits. (Since they're in the prelude and not meant for generic programming, picking a long and very marked name like that is OK.)

  • supporting object-safety
  • extending a type someone else defined

These two are very closely related, since in general the blanket impl you provide when breaking out an object safe trait is actually adding methods to types defined elsewhere.

  • arguably the operator traits are a kind of extension trait, as it doesn't seem reasonable to generically accept anything that just impl's Index or whatever.

I've sometimes thought the same thing, but actually many of these traits do end up used for generic programming (for better or worse).

The other thing is, these operator traits are "generic" in the sense that they apply to a large (unbounded) number of new types, whereas most extension traits are designed to apply to a single, or small family, of known types. So the "generic" aspect doesn't just mean generics in the technical sense, but also in a more fuzzy design sense.

In short, I think having just the Prelude and Ext suffixes should cover our needs, but I'd love to hear concrete alternatives if people have other ideas!

@liigo
Copy link
Contributor

liigo commented Nov 15, 2014

FooExtension maybe another alternative.

@alexcrichton alexcrichton merged commit 97ecd04 into rust-lang:master Nov 25, 2014
@alexcrichton
Copy link
Member

Discussion

Tracking

aturon added a commit to aturon/rust that referenced this pull request Nov 26, 2014
This is an initial pass at stabilizing the `iter` module. The module is
fairly large, but is also pretty polished, so most of the stabilization
leaves things as they are.

Some changes:

* Due to the new object safety rules, various traits needs to be split
  into object-safe traits and extension traits. This includes `Iterator`
  itself. While splitting up the traits adds some complexity, it will
  also increase flexbility: once we have automatic impls of `Trait` for
  trait objects over `Trait`, then things like the iterator adapters
  will all work with trait objects.

* Iterator adapters that use up the entire iterator now take it by
  value, which makes the semantics more clear and helps catch bugs. Due
  to the splitting of Iterator, this does not affect trait objects. If
  the underlying iterator is still desired for some reason, `by_ref` can
  be used. (Note: this change had no fallout in the Rust distro except
  for the useless mut lint.)

* In general, extension traits new and old are following an [in-progress
  convention](rust-lang/rfcs#445). As such, they
  are marked `unstable`.

* As usual, anything involving closures is `unstable` pending unboxed
  closures.

* A few of the more esoteric/underdeveloped iterator forms (like
  `RandomAccessIterator` and `MutableDoubleEndedIterator`, along with
  various unfolds) are left experimental for now.

* The `order` submodule is left `experimental` because it will hopefully
  be replaced by generalized comparison traits.

* "Leaf" iterators (like `Repeat` and `Counter`) are uniformly
  constructed by free fns at the module level. That's because the types
  are not otherwise of any significance (if we had `impl Trait`, you
  wouldn't want to define a type at all).

Closes rust-lang#17701

Due to renamings and splitting of traits, this is a:

[breaking-change]
bors added a commit to rust-lang/rust that referenced this pull request Nov 26, 2014
This is an initial pass at stabilizing the `iter` module. The module is
fairly large, but is also pretty polished, so most of the stabilization
leaves things as they are.

Some changes:

* Due to the new object safety rules, various traits needs to be split
  into object-safe traits and extension traits. This includes `Iterator`
  itself. While splitting up the traits adds some complexity, it will
  also increase flexbility: once we have automatic impls of `Trait` for
  trait objects over `Trait`, then things like the iterator adapters
  will all work with trait objects.

* Iterator adapters that use up the entire iterator now take it by
  value, which makes the semantics more clear and helps catch bugs. Due
  to the splitting of Iterator, this does not affect trait objects. If
  the underlying iterator is still desired for some reason, `by_ref` can
  be used. (Note: this change had no fallout in the Rust distro except
  for the useless mut lint.)

* In general, extension traits new and old are following an [in-progress
  convention](rust-lang/rfcs#445). As such, they
  are marked `unstable`.

* As usual, anything involving closures is `unstable` pending unboxed
  closures.

* A few of the more esoteric/underdeveloped iterator forms (like
  `RandomAccessIterator` and `MutableDoubleEndedIterator`, along with
  various unfolds) are left experimental for now.

* The `order` submodule is left `experimental` because it will hopefully
  be replaced by generalized comparison traits.

* "Leaf" iterators (like `Repeat` and `Counter`) are uniformly
  constructed by free fns at the module level. That's because the types
  are not otherwise of any significance (if we had `impl Trait`, you
  wouldn't want to define a type at all).

Closes #17701

Due to renamings and splitting of traits, this is a:

[breaking-change]
@Thiez
Copy link

Thiez commented Dec 3, 2014

People are running into problems because the extension traits tend to have a blanket implementation (e.g. here, impl<A, I> IteratorExt<A> for I where I: Iterator<A> {}) which prevents overriding methods when a better implementation is possible. Perhaps the extension traits should never have a blanket implemention?

It is easy to come up with better implementations that the default, for example last() on a DoubleEndedIterator would be next_back(). Calling skip(100) on a RandomAccessIterator should not call next() a hundred times.

@Thiez
Copy link

Thiez commented Dec 3, 2014

When we get opt-out traits (#19) it might be possible to define something like IteratorExtDefault and then impl<A, I> IteratorExt<A> for I where I: IteratorExtDefault + Iterator<A> {}. Specific iterators can then opt-out of IteratorExtDefault and implement IteratorExt manually. But perhaps that's not worth the extra mess and we should simply never have blanket implementations for Ext traits at all.

@skade
Copy link
Contributor

skade commented Dec 3, 2014

I see the problem that IteratorExt (and FooExt traits in general) are basically utils junkdrawers for Iterator with many useful default implementations. Reimplementing the whole thing just for having one method replaced seems a bit excessive, especially as those things tend to grow and not shrink. It squats a whole namespace.

reem added a commit to reem/rust-void that referenced this pull request Mar 24, 2015
@Centril Centril added the A-convention Proposals relating to documentation conventions. label Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-convention Proposals relating to documentation conventions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants