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

[DRAFT] RFC: Associated type defaults and default groups #11

Closed
wants to merge 13 commits into from

Conversation

Centril
Copy link
Owner

@Centril Centril commented Aug 23, 2018

This is a draft version of an RFC for you to review, before a formal proposal is made for consideration.

Summary

  1. Resolve the design of associated type defaults, first introduced in RFC 192, such that provided methods and other items may not assume type defaults. This applies equally to default with respect to specialization.

  2. Introduce the concept of default { .. } groups in traits and their implementations which may be used to introduce atomic units of specialization (if anything in the group is specialized, everything must be).


As Rust traits are a form of type classes,
we naturally look for prior art from were they first were introduced.
That language, being Haskell, permits a user to specify associated type defaults.
Copy link

Choose a reason for hiding this comment

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

While traits are type classes, it is also comparable to OOP interfaces; specialization in particular is entirely inspired from C++.

So I'd suggest adding the following comparison to prior arts as well:

  • C++ (all diagnostic is done post-monomorphization so probably not an interesting example, but still...)
  • Swift (Rust trait ≈ Swift protocols. Protocols can be inherited and can have associated types. But protocol constants and methods cannot be defaulted.)
  • Perhaps also check what Scala is doing 🙃


fn arbitrary() -> Self::Strategy {
Self::arbitrary_with(Default::default())
}

This comment was marked as off-topic.

This comment was marked as off-topic.


[proptest]: https://altsysrq.github.io/rustdoc/proptest/latest/proptest/arbitrary/trait.Arbitrary.html

For example, we may provide an API (due to [proptest]):
Copy link

Choose a reason for hiding this comment

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

It might read better to write this line as:

For example, we could change [proptest]'s API to be:

just like before, that `x: u8`. The reason why is much the same as
we have previously discussed in the [background].

Once place where this proposal does diverge from what is currently implemented
Copy link

Choose a reason for hiding this comment

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

Once -> One; does diverge -> diverges

what do I do then?"*. Don't worry; We've got you covered.

To be able to assume that `Self::Bar` is truly `u8` in snippets (2) and (5),
you may henceforth use `default { .. }` to group items into atomic units of
Copy link

Choose a reason for hiding this comment

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

Might want to explicitly state associated items here.

}
}

struct Alan;
Copy link

Choose a reason for hiding this comment

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

At RustConf, misdreavious's slides included descriptions of what was being referred to. It was a great thing to include. I feel like we should do the same for RFCs, and link to these peoples' Wikipedia pages.

}
}

struct Alan;
Copy link

Choose a reason for hiding this comment

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

Can we change two of these to be women who've contributed to computer science, to maintain a gender balance?


In the current implementation, (6) is rejected because the compiler will not
let you assume that `x` is of type `usize`. But in this proposal, you would be
allowed to assume this. To permit this is not a problem because `Foo for ()`
Copy link

Choose a reason for hiding this comment

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

() => Vec<T>


Idris has a concept it calls [`interface`s][idris_interface].
These resemble type classes in Haskell, and by extension traits in Rust.
However, unlike Haskell and Rust, these `interface`s are incoherent and will
Copy link

Choose a reason for hiding this comment

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

Incoherent is a strongly negative word. Would be good to rephrase this to be less negative?

```rust
trait ComputerScientist {
default {
type Bar = u8;
Copy link

Choose a reason for hiding this comment

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

"Bar, QUUX, wibble" keep confusing me. Could these be changed to terms in the domain of being a Computer Scientist?

Copy link

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

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

I like the base RFC, mostly editorial stuff, plus some corrections on the C++.

protocol Foo {
associatedtype Bar = Int

func append() -> Bar { return 0 }

Choose a reason for hiding this comment

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

The syntax highlighter seems to dislike this, maybe split it?

Copy link

Choose a reason for hiding this comment

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

The syntax highlighter dislikes it because it's literally not allowed.

Choose a reason for hiding this comment

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

Ohhh, okay

```

This shows that templates in C++ are fundamentally different from the type of
parametric polymorphism (generics) that Rust employs.

Choose a reason for hiding this comment

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

I would cut out this part, from the "do note" on, and rewrite the above paragraph.

One thing to note here is that C++ allows us to assume in both the unspecialized
variant of `foo` as well as the specialized version that `bar` is the underlying
type we said it was in the `typedef`. This is unlike the default in this RFC
but the same as when we use `default { .. }`.

Choose a reason for hiding this comment

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

I'd rewrite this comment quite heavily, to something along the following lines:

You will note that C++ allows us to assume in both the base template class, as well as the specialization, that bar is equal to the underlying type. This is because one cannot specialize any part of a class without specializing the whole of it. It's equivalent to one atomic default { ... } block.

};

template<typename T> struct foo<wrap<T>> { // Partial specialization.
typedef std::string bar;

Choose a reason for hiding this comment

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

You should use using here instead - using bar = std::string;. It's just more readable.

template<typename T> struct wrap {};

template<typename T> struct foo { // Unspecialized.
typedef int bar;
Copy link

@strega-nil strega-nil Aug 26, 2018

Choose a reason for hiding this comment

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

See above comment on using.

}
```

However, please note that for use of `default { .. }` inside implementations,
Copy link

Choose a reason for hiding this comment

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

This is worth having at the top of the section, not the bottom.

use by specialization and for `default impl`.
Therefore, the syntax is only partially new.

2. secondarily, if you have implementations where it is commonly needed
Copy link

Choose a reason for hiding this comment

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

"secondarily" is already said by the number "2".

@Havvy
Copy link

Havvy commented Aug 26, 2018

I have an alternative in mind.

So, a caveat here. I have not thought about how this interacts with specialization. So everything here talks from the perspective of writing defaults in traits.

The short of it is that in this alternative, default blocks let you specialize default implementations when the implementing type's type aliases match the specified type aliases in the default block. And furthermore, the default block does not allow declaration of new associated items. It can only provide new defaults that can make use of the specified type.

For an example of this alternative, we'll use this trait and this impl:

trait Trait {
  type Type;
  const Const: Type;
}

impl Trait for UStruct {
  type Type = u8;
  const Const: Type = 0u8;
}

Without using a default block, we cannot give a default to Const for the same reasons the RFC already spells out. But we want to have it so that when Type is u8, then Const is 0u8. So to do so, we add a default block to the trait.

trait Trait {
  type Type;
  const Const: Type;

  default {
    type Type = u8;
    const Const: u8 = 0u8;
  }
}

impl Trait for UStruct {
  type Type = u8;
  // No need to specify `Const` as it uses the default given.
}

But then, we decide that if the implementer chooses i8 for their type, we should also have a different default constant for them. To fulfill this, we just add a new default block.

trait Trait {
  type Type;
  const Const: Type;

  default {
    type Type = u8;
    const Const: u8 = 0u8;
  }

  default {
    type Type = i8;
    const Const: i8 = 1i8;
  }
}

impl Trait for IStruct {
  type Type = i8;
  // `Const` defaults to 1i8
}

To pick out which default applies, we look at what the types are in the default block compared to what they are in the implementation. And since this is about allowing default on type aliases outside of the default blocks, if it's there, and the implementation doesn't override, then that's the type. And if there's a default block for that, it also uses those as defaults. So, let's give Trait a default Type of u8.

trait Trait {
  type Type = u8;
  const Const: Type;

  default {
    type Type = u8;
    const Const: u8 = 0u8;
  }

  default {
    type Type = i8;
    const Const: i8 = 1i8;
  }
}

impl Trait for UStruct {
  // type defaults to `u8`.
  // `Const` defaults to 0u8 because `Type` is `u8`.
}

And this all works great in simple cases. It gets complex though, if there's more than one type. You could have...

trait Trait {
  type TypeA;
  type TypeB;
  const Const: u8;

  default {
    type TypeA = ();
    const Const: u8= 0u8;
  }

  default {
    type TypeB = ();
    const Const: u8= 1u8;
  }
}

...and then you're not sure which default applies. That can be fixed by requiring defaulted items use all type aliases in the default block. Similarly, you can have specialization issues if you allow defaults where the types are generic.

@kennytm
Copy link

kennytm commented Aug 26, 2018

In RFC rust-lang#2500 I've got this trait:

trait Needle<H: Haystack>: Sized {
    type Searcher: Searcher<H::Target>;
    type Consumer: Consumer<H::Target>;
    fn into_searcher(self) -> Self::Searcher;
    fn into_consumer(self) -> Self::Consumer;
}

usually the associated types Consumer and Searcher are equivalent, so I'd like to default it:

trait Needle<H: Haystack>: Sized {
    type Searcher: Searcher<H::Target>;
    fn into_searcher(self) -> Self::Searcher;

    default {
        type Consumer: Consumer<H::Target> = Self::Searcher;
        fn into_consumer(self) -> Self::Consumer { self.into_searcher() }
    }
}

However, a Searcher may not implement Consume<H::Target>, and the default should not be applied in this case (should cause error). Could this RFC handle this?

@varkor
Copy link

varkor commented Aug 26, 2018

I haven't looked at others' comments, but this seems like a nice, straightforward approach to defaults to me.
I would like to see impl Trait mentioned here, but it's not passed an RFC yet so it's not technically relevant now. But specifically my question is that if an associated type default is provided:

type Foo = impl Bar;

is the opaque type that implements Bar here shared between all implementors of the trait, or re-inferred for all implementers of the trait? Or is this construction entirely banned for now?

This could be left unaddressed unless the trait alias impl Trait is accepted, but I think it's worth bearing in mind.

Copy link

@alexreg alexreg left a comment

Choose a reason for hiding this comment

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

Nice work. Overall, definitely in favour of this RFC. I must say, I didn't fully appreciate the motivation until I read the Guide section (not so much the Motivation section, hah). I was weighing up the pros and cons of the behaviour of associated type defaults in provided methods in this RFC vs. existing behaviour, and came to essentially the same conclusions as you. The granularity of this approach offers a big advantage, and the way methods interact with default associated types just seems more intuitive this way.

```

By allowing defaults, we can transition to this more flexible API without
breaking any consumers by simply saying:

This comment was marked as resolved.


[specialization]: https://github.com/rust-lang/rfcs/pull/1210

With respect to [specialization], the behaviour is the same.

This comment was marked as resolved.

is with respect to the following example (6):

```rust
#![feature(associated_type_defaults)]
Copy link

Choose a reason for hiding this comment

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

Slightly confusing feature name, given we already have "associated type defaults"? Maybe "associated_type_defaults_v2" or such?


the underlying type of `Assoc` stays the same for all implementations which
do not change the default of `Assoc`. The same applies to specializations.
With respect to type visibility, it is the same as that of `existential type`.
Copy link

Choose a reason for hiding this comment

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

"Type opacity" (rather than "visibility"), you mean? I think that's clearer.


There applies no restriction on the nesting of groups.
This means that you may nest them arbitrarily.
When nesting does occur, the atomicity applies as if the nesting was flattened.
Copy link

Choose a reason for hiding this comment

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

s/was flattened/were flattened/.

Wouldn't it be better just to disallow nesting? I thought of this before I read this section, and came to the conclusion that such variations just add needless complexity and potentially confusion.


Note: Everything in this section assumes actual support for [specialization].

Now, you might be thinking: - *"Well, what if I __do__ need to assume that

This comment was marked as resolved.

@Centril
Copy link
Owner Author

Centril commented Aug 27, 2018

Thanks all for chipping in!
Time to publish :)

@Centril Centril closed this Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants