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: Associated type bounds of form MyTrait<AssociatedType: Bounds> #2289

Merged
merged 9 commits into from
Jul 24, 2018

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jan 13, 2018

RENDERED

TRACKING ISSUE


Introduce the bound form MyTrait<AssociatedType: Bounds>, permitted anywhere a bound of the form MyTrait<AssociatedType = T> would be allowed. The bound T: Trait<AssociatedType: Bounds> desugars to the bounds T: Trait and <T as Trait>::AssociatedType: Bounds.

An example:

fn print_all<T: Iterator<Item: Display>>(printables: T) {
    for p in printables {
        println!("{}", p);
    }
}

This RFC was co-authored with @joshtriplett whom it was my pleasure to work with.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jan 13, 2018
@Centril Centril changed the title RFC: Associated type bounds, RFC: Associated type bounds of form MyTrait<AssociatedType: Bounds> Jan 13, 2018
# Unresolved questions
[unresolved]: #unresolved-questions

- Does this introduce any parsing ambiguities?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were pretty sure that there wouldn't be any - but it is nice to get that confirmed.
I'll remove this question in a bit then =)

[reference-level-explanation]: #reference-level-explanation

The surface syntax `Trait<AssociatedType: Bounds>` should desugar to
`Trait<AssociatedType = impl Bounds>` anywhere it appears. This syntax
Copy link
Contributor

@petrochenkov petrochenkov Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure literally desugaring like this will be legal and will mean the same thing in all contexts.
Maybe it's better to use the desugaring from the examples above (T: Trait<AssocTy: Bounds> -> T: Trait + <T as Trait>::AssocTy: Bounds) everywhere?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice sugar, overall.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in other words: T: Trait<AssocTy: Bounds> desugars into adding <T as Trait>::AssocTy: Bounds to the list of where clauses? How do we deal with that in the case of the following?

fn printables() -> impl Iterator<Item: Display> {
    // ..
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Centril -> impl Iterator<Item = T> is already desugared to two bounds, X: Iterator and <X as Iterator>::Item == T, where X is the impl Iterator<Item = T> nominal type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrochenkov That's great! It means we can implement this today with very little effort, since we have all the needed infrastructure from Trait<AssocTy = T> (assuming we don't even need to handle trait objects).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrochenkov You are more familiar with the inner workings of the compiler, so I think you can better specify the proper desugaring. Could you PR against our PR perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch in: Centril#2

@aturon aturon self-assigned this Jan 25, 2018
@Centril
Copy link
Contributor Author

Centril commented Mar 5, 2018

Status update: We (me and @joshtriplett) have patched the reference and have made consequence-changes in other places + some housekeeping. Hopefully everything should be up to snuff now.

@nikomatsakis
Copy link
Contributor

Personally, I'm not a big fan of this proposal. I just don't find it very obvious that a new name is being bound in the syntax as given, though I realize it is ambiguous, and also this is not a scenario that I think I've ever really noticed arising a lot. I'd be curious to see a set of examples from stdlib or other real crates to get some idea of how often it arises and how it feels in practice.

@Centril
Copy link
Contributor Author

Centril commented Apr 26, 2018

@nikomatsakis I'll work on providing some more examples :)

@fstirlitz
Copy link

This is an extension of the T: Trait<U=V> syntax, which I've always found quite lousy. I'd rather have it deprecated altogether than built upon.

@joshtriplett
Copy link
Member

joshtriplett commented Apr 27, 2018

@fstirlitz I find T: Iterator<Item=U> far clearer than <T as Iterator>::Item = U. The former more directly translates what I want to say: "T is an Iterator whose Item is U".

@joshtriplett
Copy link
Member

@nikomatsakis I run into this scenario quite often. I tend to write any function using the most general type I can, so I often want to say "an iterator over types that have this trait", for instance.

@fstirlitz
Copy link

@joshtriplett: I don't think grammars of programming languages should be designed based on how things 'directly translate' to natural languages, especially English. Natural languages are full of syntactic ambiguities and irregularities that are detrimental to reasoning about code. And I'd argue this particular form is one of the latter: Iterator<Item=U> looks like keyword arguments in Python, i.e. something that should be very much like Iterator<U> except that the type parameter is specified by name instead of by position. If T: Trait<X> and T: Trait<Y> can be met simultaneously, why not T: Trait<U=X> and T: Trait<U=Y>?

@burdges
Copy link

burdges commented Apr 27, 2018

Is this reasonable to do with ATCs ala PointerFamily<Pointer: Snyc> or should it use quantification ala for <T> PointerFamily<Pointer<T>: Snyc>.

I suppose the MyTrait<AssociatedType = impl Bounds> form does not create ambiguities, meaning only MyTrait<AssociatedType = some Bounds> makes sense, not MyTrait<AssociatedType = any Bounds>.

I'm sympathetic to not rushing to optimize syntax that expresses complex relationships, and initially reacted negatively to this proposal, but.. We're stretching impl Trait hard here too, meaning some Trait requires less thought, and the form proposed here already exists in struct declarations.

I suppose MyTrait<AssociatedType = Bounds> would be equivalent to MyTrait<AssociatedType = dyn Bounds> until the old trait object syntax gets deprecated.

@Centril
Copy link
Contributor Author

Centril commented Apr 28, 2018

@fstirlitz

This is an extension of the T: Trait<U=V> syntax, which I've always found quite lousy. I'd rather have it deprecated altogether than built upon.

To me, deprecation of Trait<Assoc = Type> sounds wildly undoable. By now, I think this syntax is firmly established in Rust. This is line of reasoning is further enhanced by the fact that you can't even write
<T as Iterator>::Item = U. Requiring the introduction of the latter may lead to other surprises for users such as "But what about T = U?", where T is not a projection, given that we might not want to introduce equality constraints of the form T = U.

@joshtriplett: I don't think grammars of programming languages should be designed based on how things 'directly translate' to natural languages, especially English.

What's wrong with English with regard to ambiguity? French has its irregular verbs and "la", "le"; Swedish has its unintuitive (for a non-native speaker) "en" and "ett"...

Natural languages are full of syntactic ambiguities and irregularities that are detrimental to reasoning about code.

They are full of syntactic ambiguities, but converting a subset which is unambiguous makes reasoning easier IMO. Being able to read code out loud in a way not too far from your mental model enhances understanding (and thus readability). This partly explains python's popularity due "executable pseudocode".

And I'd argue this particular form is one of the latter: Iterator<Item=U> looks like keyword arguments in Python, i.e. something that should be very much like Iterator<U> except that the type parameter is specified by name instead of by position. If T: Trait<X> and T: Trait<Y> can be met simultaneously, why not T: Trait<U=X> and T: Trait<U=Y>?

I think that the possible confusion here is approximates a one-time problem in learning Rust. Once you realize that type parameters correspond to relations (in mathematics) and that associated types correspond to functions (and crucially that x = y -> f(x) = f(y)) and that parameters are positional (as with arguments to value level fns) I think it makes perfect sense and it is consistent.
Once you learn that F<X = A> is an equality constraint on an associated type of F, I think the confusion fades quickly.

Given that we already have F<X = A> and that, as I argue, deprecation of this syntactic form is untenable given the establishment of this and a lack of a clear alternative, I think we should stick with it and build upon it. The natural extension is F<X: C>.

@Centril
Copy link
Contributor Author

Centril commented Apr 28, 2018

@burdges It depends on what you wish to communicate.

Consider: where X: Family<Assoc<T>: Bound>. I would translate this to the bounds X: Family and X::Assoc<T>: Bound.

Consider: where X: for<T> Family<Assoc<T>: Bound>. I would translate that to the bounds: X: Family and for<T> X::Assoc<T>: Bound.

These two bounds are clearly different (even if the latter builds upon the combination of for<T> and Family<Assoc<T>: Bound>). The former talks about a specific T applied to Assoc satisfying Bound while the latter says that for any T you pick, T applied to Assoc, Assoc<T> satisfies Bound.

At this point; I think it is too early to introduce Assoc<T>: Bound given that GATs are not even in the nightly compiler. Once we gain more experience we can see if this extension is worthwhile;

Regarding Family<Assoc: Bound> as sugar for for<T> Family<Assoc<T>: Bound>, I think that this would be surprising if we don't have HKTs. If however we do introduce HKTs, this would translate to Functor (AssocFunctor MyFamily) (as a concrete example) in Haskell. Then I do think it makes sense. But this addition is premature.

EDIT: Actually, no... that was wrong! I don't see Family<Assoc: Bound>, where Assoc is higher kinded, meaning for<T> Family<Assoc<T>: Bound> because in the case of HKTs, it is the functor that has the bound, and not the functor applied to some type argument.

Regarding Trait<Assoc = impl Bounds>, the meaning depends on if it is in argument position or return position. If it is in return position, that is: -> impl T<A = impl B>, then the type of A is existential as seen in this example:

fn foo() -> impl Iterator<Item = impl Clone> {
    ::std::iter::empty::<u8>()
}

Therefore, the callee picks the type of Item.

If however you have impl T<A = impl B> in argument position, you get:

fn bar(mut iter: impl Iterator<Item = impl Clone>) {
    let x: Option<()> = iter.next();
}

However; this snippet does not compile because the caller, and not the callee, gets to pick the type, and so we get:

10 |     let x: Option<()> = iter.next();
   |                         ^^^^^^^^^^^ expected (), found type parameter

@Centril
Copy link
Contributor Author

Centril commented Apr 28, 2018

@nikomatsakis Here are some examples of where it applies:

Proptest does things like:

<<S as Strategy>::Value as ValueTree>::Value: Foo

which we can rewrite as:

S: Strategy<Value: ValueTree<Value: Foo>>

which is more readable to me (less tracking of sigils).

@joshtriplett
Copy link
Member

@Centril Thank you for the extensive work putting together that list of examples!

(Was https://doc.rust-lang.org/nightly/src/alloc/vec.rs.html#2562 intended to link somewhere else? I don't see any instances in that source file to which this would apply.)

@Centril
Copy link
Contributor Author

Centril commented Apr 28, 2018

@joshtriplett Oops; that one was derived :)

@joshtriplett
Copy link
Member

@Centril That raises the question: How did you find these?

@Centril
Copy link
Contributor Author

Centril commented Apr 28, 2018

@joshtriplett I checked the standard library for impls manually searching for >:: :) I mainly went for the traits in libstd.

@joshtriplett
Copy link
Member

joshtriplett commented Apr 28, 2018

@Centril Ah, I see.

I suspect that other examples exist of the form T::AssocType: Trait that don't directly write <T as Something>::AssocType: Trait. Some searching turned up quite a few of that style, too. A few representative examples:

https://github.com/rust-lang/rust/blob/1e01e22509df395fb42885235d694909b4309398/src/liballoc/tests/str.rs#L1385
https://github.com/rust-lang/rust/blob/1e01e22509df395fb42885235d694909b4309398/src/librustc_data_structures/small_vec.rs#L34
https://github.com/rust-lang/rust/blob/1e01e22509df395fb42885235d694909b4309398/src/librustc_data_structures/owning_ref/mod.rs#L845
https://github.com/rust-lang/rust/blob/1e01e22509df395fb42885235d694909b4309398/src/librustc_mir/dataflow/mod.rs#L520

Also, for future reference: git grep -Pn '(?<!Self)::[A-Z][A-Za-z]+: ' (That'll also find many of the examples @Centril found, too.)

@Nemo157
Copy link
Member

Nemo157 commented May 8, 2018

This seems like it requires introducing a new feature to existential types that RFC 2071 doesn't specify, given

fn foo() -> impl Iterator<Item: Display> { ... }

this RFC appears to specify the desugaring as

existential type _0: Iterator where <_0 as Iterator>::Item: Display;
fn foo() -> _0 { ... }

EDIT: or is it

existential type _0: Iterator;
fn foo() -> _0 where <_0 as Iterator>::Item: Display { ... }

either way I'm not sure what the intended semantics of these statements would be.

@Nemo157
Copy link
Member

Nemo157 commented May 8, 2018

I would like to note that I'm +1 on this syntax for generic type parameters, it's just the barely mentioned extension to existential types that seems underspecified. Also, looking through to #1093 since that was linked, the desugaring for associated types in traits mentioned there doesn't seem to be specified. Given

trait Foo {
    type Bar: Iterator<Item: Display>;
}

what does this desugar to, and how does that differ in practice/why would you choose that over the current

trait Foo {
    type BarItem: Display;
    type Bar: Iterator<Item = Self::BarItem>;
}

EDIT: or the other possibility in current rust, which I don't recall seeing used in the wild:

trait Foo where <Self::Bar as Iterator>::Item: Display {
    type Bar: Iterator;
}

@Centril
Copy link
Contributor Author

Centril commented May 8, 2018

@Nemo157 While me and @joshtriplett discuss how to make this RFC clearer, let me talk a bit about the semantics in the cases you brought up for now.

In the first case of fn foo() -> impl Iterator<Item: Display> { ... }, the end result is the same semantics as fn foo() -> impl Iterator<Item = impl Display> { ... }. In this context, it is existential quantification all the way.

The latter case can be desugared directly to:

use std::fmt::Display;

trait Foo {
    type Bar: Iterator where
      <Self::Bar as Iterator>::Item: Display;
}

impl Foo for () {
    type Bar = ::std::iter::Empty<u8>;
}

Operationally speaking, it should have the same affect as having the two associated types (but where one is uninteresting wrt. public interface, which is also why you'd want to do this).

@aturon
Copy link
Member

aturon commented Jun 22, 2018

The lang team spent a while discussing this RFC in our most recent meeting:

  • There was general agreement that this feature is well-motivated, especially for generics-heavy libraries (like Diesel and Tower).

  • @nikomatsakis expressed concern about continuing to accrete new bits of syntax.

    • I argued that there are two important factors at play here. First, as pure syntax sugar, there is very little risk in terms of adding fundamental complexity. Second, in general we've had good success in adding shorthands around central features, like lifetimes and generics, that are heavily used (and hence where the payoff is high). And in particular, this feature would extend the reach of the impl Trait in argument position shorthand
    • @joshtriplett further argued that expansions like this often make the language simpler by making it more orthogonal -- i.e. having : bounds be usable in more places, rather than having to remember exactly where they are usable.
  • In general, the lang team is not opposed to having multiple ways to express something -- some being more general, others more concise -- as long as the technique is applied only to cases that are common or particularly painful. Lifetime elision, the ability to write bounds directly (rather than in where clauses), and impl Trait in argument position are all examples.

  • @cramertj had previously expressed concern about this RFC compared to writingIterator<Item = implSomeTrait> to express a similar idea. He wasn't present at the meeting, but others felt happy to have both forms available, again in the name of orthgonality and convenience of expression.

All told, we felt ready to move toward final review:

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Jun 22, 2018

Team member @aturon has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Jun 22, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Jul 12, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Jul 12, 2018
@LukasKalbertodt
Copy link
Member

I just talked to @Centril about one thing that haven't been brought up yet: associated types in HRTBs.

trait Foo<'a> {
    type Out;
}

fn bar<T>()
where
    T: for<'a> Foo<'a>,
    for<'a> <T as Foo<'a>>::Out: Clone,
{}

With this RFC, the bounds on bar() can be rewritten like this:

where
    T: for<'a> Foo<'a, Out: Clone>,

Much better!

@rfcbot
Copy link
Collaborator

rfcbot commented Jul 22, 2018

The final comment period, with a disposition to merge, as per the review above, is now complete.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Jul 22, 2018
@Centril Centril merged commit 2d5218a into rust-lang:master Jul 24, 2018
@Centril Centril deleted the rfc/associated-type-bounds branch July 24, 2018 11:47
@Centril
Copy link
Contributor Author

Centril commented Jul 24, 2018

Huzzah! This RFC is merged!

Tracking issue: rust-lang/rust#52662

@burdges
Copy link

burdges commented Jul 31, 2018

We're going to find some compiler bugs thanks to this.

I wanted to abstract over collection types by using &mut T: IntoIterator bounds

fn batch_normalization<II: ?Sized>(v: &mut II)  where
for<'a> &'a mut II: IntoIterator<Item = &'a mut Self, IntoIter: DoubleEndedIterator+ExactSizeIterator>,

so I wrote it out long hand like

fn batch_normalization<II: ?Sized>(v: &mut II)
where
    for<'a> &'a mut II: IntoIterator<Item = &'a mut Self>,
    for<'a> <&'a mut II as IntoIterator>::IntoIter: DoubleEndedIterator+ExactSizeIterator

It passes cargo check fine if not called, but if called like in cargo test then rustc never finds DoubleEndedIterator or ExactSizeIterator, even when the call site looks like batch_normalization(v.as_mut_slice()).

error[E0277]: the trait bound `for<'a> <&'a mut _ as std::iter::IntoIterator>::IntoIter: std::iter::DoubleEndedIterator` is not satisfied
   --> src/tests/curve.rs:378:9
    |
    |         G::batch_normalization(v.as_mut_slice());
    |         ^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'a> std::iter::DoubleEndedIterator` is not implemented for `<&'a mut _ as std::iter::IntoIterator>::IntoIter`
    |
    = help: the following implementations were found:
              <&'a mut I as std::iter::DoubleEndedIterator>

In this case rustc is clearly wrong because <&mut [T] as IntoIterator>::IntoIter = std::slice::IterMut which satisfies both. And the help message notes this.

See commit: burdges/pairing@6b53813

@eddyb
Copy link
Member

eddyb commented Jul 31, 2018

@nikomatsakis ^^ (lack of) lazy normalization strikes again?

@dtolnay dtolnay mentioned this pull request Sep 2, 2018
@burdges burdges mentioned this pull request Sep 17, 2018
@Centril Centril added A-syntax Syntax related proposals & ideas A-traits Trait system related proposals & ideas A-typesystem Type system related proposals & ideas labels Nov 23, 2018
@Centril Centril mentioned this pull request Mar 7, 2019
@burdges burdges mentioned this pull request Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-syntax Syntax related proposals & ideas A-traits Trait system related proposals & ideas A-typesystem Type system related proposals & ideas disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.