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: impl specialization #1210

Merged
merged 15 commits into from Feb 23, 2016
Merged

RFC: impl specialization #1210

merged 15 commits into from Feb 23, 2016

Conversation

@aturon
Copy link
Member

aturon commented Jul 13, 2015

This RFC proposes a design for specialization, which permits multiple impl
blocks to apply to the same type/trait, so long as one of the blocks is clearly
"more specific" than the other. The more specific impl block is used in a case
of overlap. The design proposed here also supports refining default trait
implementations based on specifics about the types involved.

Altogether, this relatively small extension to the trait system yields benefits
for performance and code reuse, and it lays the groundwork for an "efficient
inheritance" scheme that is largely based on the trait system (described in a
forthcoming companion RFC).

Rendered

@aturon aturon changed the title RFC: Impl specialization RFC: impl specialization Jul 13, 2015
@aturon aturon added the T-lang label Jul 13, 2015
@aturon aturon self-assigned this Jul 13, 2015
}
partial impl<T: Clone, Rhs> Add<Rhs> for T {
fn add_assign(&mut self, rhs: R) {

This comment has been minimized.

Copy link
@sfackler

sfackler Jul 13, 2015

Member

This should have a default, I think.

The solution proposed in this RFC is instead to treat specialization of items in
a trait as a per-item *opt in*, described in the next section.

## The `default` keyword

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Jul 13, 2015

Member

This mentions that default will be a keyword, but we currently have modules like std::default and methods like Default::default, so I think adding this as a keyword may be a breaking change (same with partial below). Could this RFC perhaps clarify that they'll be contextual keywords? I believe that should be backwards compatible, right?

This comment has been minimized.

Copy link
@eddyb

eddyb Jul 14, 2015

Member

Are we going to do that, instead of using attributes?

This comment has been minimized.

Copy link
@bstrie

bstrie Aug 1, 2015

Contributor

Making them contextual keywords would be BC, but contextual keywords themselves are unprecedented in the language. Unfortunate, but probably inevitable.

@Diggsey
Copy link
Contributor

Diggsey commented Jul 13, 2015

How does the compiler handle code such as this:

struct Foo<T>;

impl<T> Foo<T> {
    default fn test(self) { ... }
}

fn bar(Foo<u32> p) {
    p.test();
}

It has no way of knowing whether a downstream crate will provide a more specialized implementation of test for Foo<u32>. At the moment problems like this are avoided because it can only happen in generic code, which can be monomorphised by the downstream create, whereas in this case it can happen in code with no type parameters. It's as though bar() has a hidden type parameter.

edit:
Maybe this specific example would be prevented by the coherence rules? However, it's not at all obvious that the coherence rules will prevent all such problems.

@sfackler
Copy link
Member

sfackler commented Jul 13, 2015

I'd imagine downstream crates can't specialize inherent methods of a type, just as they can't add inherent methods to a type today.

@jroesch
Copy link
Member

jroesch commented Jul 14, 2015

I'm probably gonna come back to this a few times over the next couple days because I feel like there is a lot to chew on here, and even now reading it for the third time I have some things I want to think about more deeply.

Overall I like this approach and think that the specified algorithm is a good point in the design space.

As I mentioned on IRC I think we should also follow up with a proposal for a detailed implementation strategy, that we (the compilers team, core team, me, any other relevant parties) talk about for a period of time. From my perspective (and I hope others too) it is important that we evaluate our implementation strategy and ensure that it isn't going to cause problems down the road in terms of stability, ICEs, our future compiler design (incremental, parallel, etc).

@Stebalien
Copy link
Contributor

Stebalien commented Jul 14, 2015

This isn't relevant to the current RFC but an alternative explicit ordering mechanism would be a match like syntax:

impl<'a, T: ?Sized, U: ?Sized> AsRef<U> for {
    &'a T where T: AsRef<U> => {
        fn as_ref(&self) -> &T {
            <T as AsRef<U>>::as_ref(*self)
        }
    },
    T where T == U => {
        fn as_ref(&self) -> &T {
            self
        }
    },
}
@stevenblenkinsop
Copy link

stevenblenkinsop commented Jul 14, 2015

Another alternative [edit] to explicit ordering [/edit] would be to break the overlap using negative bounds. Obviously this is contingent on negative bounds being accepted. This seems like a better approach, since it preserves the intuitive rule used in this proposal along with its various properties. Also, allowing I < J when each of apply(I) and apply(J) contains elements the other does not makes adding super down the road more awkward, since it'll reference something different depending on whether the particular types ∈ apply(I) ∪ apply(J) also belongs to the intersection or not.

edit: Clarified that I'm talking about an alternative to explicit ordering, not to specialization.

```

This partial impl does *not* mean that `Add` is implemented for all `Clone`
data, but jut that when you do impl `Add` and `Self: Clone`, you can leave off

This comment has been minimized.

Copy link
@mdinger

mdinger Jul 14, 2015

Contributor

s/jut/just/

@withoutboats
Copy link
Contributor

withoutboats commented Jul 14, 2015

I'm not very comfortable with the idea of introducing a mechanism which can create very tall inheritance trees; if the number of possible default impls is unbounded, it could be a major pain to determine which implementation is actually being run. Inheritance-oriented code structuring is pretty widely thought to be bad design, and an advantage of Rust is that so far it has strongly discouraged that kind of code.

Would it be possible therefore to limit the number of default impls to one? That is, any two default implementations would conflict. This would make it very easy to determine which impl is being executed in all cases. Is this a restriction that would be very limiting in practice? Can someone familiar with servo's inheritance use case say whether it needs n-deep inheritance? Would enforcing this rule require additional orphan rules making special determinations about where default impls could be declared (I'm fairly sure the answer is 'no')?


I don't think negative bounds and specialization are alternatives to one another at all. This RFC doesn't mention PR #1148 which enables negative bounds without the backcompat hazard that troubled PR #586. I think that these would be complementary changes to the coherence rules which address one another's limitations. Not trying to trumpet my own RFC -- just pointing out that is a related proposal.


Haskell has extensions which implement some form of specialization (e.g. OverlappingInstances). It would be a good idea probably to ask in the Haskell community about the pitfalls that implementations of type class specialization have run into. I think this is a situation in which Rust's more OO-influenced heritage makes specialization more useful for us than it was for Haskell though.


We'll probably need a new name for impl Trait for .. { } impls if this RFC is accepted.


Regardless of these, this is an awesome and impressively exhaustive RFC!


```rust
impl<T> Debug for T where T: Display {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {

This comment has been minimized.

Copy link
@llogiq

llogiq Jul 14, 2015

Contributor

Shouldn't that be a default fn fmt(…)?

This comment has been minimized.

Copy link
@stevenblenkinsop

stevenblenkinsop Jul 14, 2015

The text hasn't motivated default at this point. The example is of what the proposal is trying to allow "at the simplest level"—i.e. before the complexity of needing a default keyword is added—not of what the proposed syntax will ultimately look like once this complexity is taken into account. I thought this was clear, but maybe it could be clarified.

It might be a good idea to limit the appearance of motivating examples which don't follow the proposed syntax. One way would just be to add text saying "ignore the default keyword for now, it'll be motivated later". This would be unfortunate though, since I liked the style of exposition used here, and adding these caveats would diminish it somewhat. Perhaps a better option is just to add a comment in the example itself saying:

// Note: This example will not work as written under this proposal.

This comment has been minimized.

Copy link
@pnkfelix

pnkfelix Jul 16, 2015

Member

At the very least, any example that does not actually follow the expected end-syntax could have an explicit annotation saying so, perhaps with a pointer to an end-appendix that shows each such example in the final expected form.

(Also, the "Motivation" section did at least say default in one example -- so there is at least precedent for using it here as well, if one does not want to go the route of adding an appendix with all of the examples according to their final formulation)

- You have to lift out trait parameters to enable specialization, as in the
`Extend` example above. The RFC mentions a few ways of dealing with this
limitation -- either by employing inherent item specialization, or by
eventually generalizing HRTBs.

This comment has been minimized.

Copy link
@llogiq

llogiq Jul 14, 2015

Contributor

Please write out Higher-Ranked trait bounds.

@llogiq
Copy link
Contributor

llogiq commented Jul 14, 2015

Overall, I'm quite happy with the proposal; I'm a bit worried about corner cases (especially regarding dropck), but I think we should be able to sort them out.

Of course, once this RFC is accepted, Rust will cease to even be a language of medium complexity. People will misuse this feature to create a maze of twisty little fns, all alike, then complain when they can no longer reason about who calls whom (Case in point: I have a very evil example using 3 very small java classes fitting on half a page in 10-point that I used in an exam once. Of more than 600 CS students, only 1 got it right) (Edit: This blog post shows a reduced, less evil example).

Therefore the only thing I don't like is the use of the default keyword (despite having it in Java interfaces). I want the keyword to be long, outlandish and hard to remember, so folks will have to think twice before writing it. Something like: iknowwhatidosoletmeoverridelater (only partially tongue-in-cheek).

@bill-myers
Copy link

bill-myers commented Jul 14, 2015

Requiring that apply(I) and apply(J) are either disjoint or one contained in the other seems excessively restrictive.

A less restrictive rule could be this set of equivalent rules:

  1. The set of impls I_j that apply to any given type T and trait R has a minimum element relative to the I <= J ordering in the RFC (which is the impl that would be chosen)
  2. The set of apply(I_j) that contain any given type-tuple T has a minimum element relative to the set inclusion ordering
  3. The intersection of the sets of apply(I_j) that contain any given type-tuple T is equal to apply(I) for some I
  4. For any two impls I and J, the intersection of apply(I) and apply(J) is equal to the union of apply(I_k) for all apply(I_k) that are subsets of both apply(I) and apply(J)

This allows sets of impls like this:

impl<T, U> Foo for (T, U) // A
impl<T> Foo for (T, int) // B
impl<U> Foo for (int, U) // C
impl Foo for (int, int) // D

This would be allowed since the intersection of apply(B) and apply(C) is equal to apply(D), the only apply-set contained in both, and all other apply-set pairs are contained in each other.

Not sure about the algorithmic complexity of checking for this though. It appears to be equivalent to SAT solving, but this also applies to checking the exhaustiveness of match patterns, so it's not necessarily an issue.

A possible middle ground is to require that the intersection of apply(I) and apply(J) is equal to apply(K) for just one K, which should eliminate the SAT equivalency, and might still be expressive enough.

@tbu-
Copy link
Contributor

tbu- commented Jul 14, 2015

Another motivating example is the ToString trait which should really be specialized for &str.

@Kimundi
Copy link
Member

Kimundi commented Jul 14, 2015

I like it! My only worry is the that the language is getting more complex, but arguable thats not avoidable in this case.

Am I right in thinking that this RFC would enable backwards-compatibly solving the Copy => Clone situation like this?

partial impl<T> Clone for T where T: Copy {
    default fn clone(&self) -> Self {
        *self
    }
}
@arielb1
Copy link
Contributor

arielb1 commented Jul 14, 2015

Wow, this basically destroys dropck - we may have to go back to a (maybe compiler-guaranteed) form of #[unsafe_destructor].

@Kimundi

No. That requires "lattice impls", which is explicitly not part of this RFC.

@llogiq
Copy link
Contributor

llogiq commented Jul 14, 2015

@arielb1 please elaborate how this 'destroys dropck'.

@arielb1
Copy link
Contributor

arielb1 commented Jul 14, 2015

@llogiq

It completely destroys parametricity, and therefore dropck. I think it may even be a better option to make specialization unsafe for that reason.

To see how it destroys parametricity:

Suppose you have a completely innocent blanket impl for a #[fundamental] type:

impl<'a, T> Clone for &'a T { fn clone(&self) -> Self { *self } }

It can be trivially called by a destructor:

struct Zook<T>(T);
fn innocent<T>(t: &T) { <&T as Clone>::clone(&t) /* completely innocent, isn't it? */; }
impl<T> Drop for Zook<T> { fn drop(&mut self) { innocent(&self.0); } }

However, this can be abused:

struct Evil<'a>(&'b OwnsResources /* anything that is invalid after being dropped */);
impl<'a,'b> Clone for &'a Evil<'b> {
    fn clone(&self) -> Self {
        println!("I can access {} even after it was freed! muhahahaha", self.0);
        loop {}
    }
}

fn main() {
    let (zook, owns_resources);
    owns_resources = OwnsResources::allocate();
    zook = Zook(Evil(&owns_resources));
}
@Diggsey
Copy link
Contributor

Diggsey commented Jul 14, 2015

It completely destroys parametricity

Isn't this problem what the Reflect trait was intended to solve? ie. you're guaranteed parametricity so long as you don't have a Reflect bound. Default items in an impl block could be allowed only if the trait inherits from Reflect?

@aturon
Copy link
Member Author

aturon commented Jul 14, 2015

@arielb1

Wow, this basically destroys dropck

@nikomatsakis, @pnkfelix and I had discussed the dropck issue a while back (note the brief discussion in Unresolved Questions). There are a few avenues for preventing the interaction you're describing -- note, for example, that in the RFC proposal you need a default qualifier to allow overriding on such a blanket impl. We have been considering rules that would disallow use of default (and therefore specialization) in circumstances where it's possible to apply the relevant impl starting with a type T with no bounds -- basically, the other side of the parametricity requirement for dropck. I left it as an Unresolved Question in the RFC mainly because I'd like to prototype first before proposing a firm rule. But this question must be resolved before we could move forward with this RFC.

Am I right in thinking that this RFC would enable backwards-compatibly solving the Copy => Clone situation like this?

partial impl<T> Clone for T where T: Copy {
    default fn clone(&self) -> Self {
        *self
    }
}

@Kimundi
No. That requires "lattice impls", which is explicitly not part of this RFC.

The RFC talks a bit about how we could handle this case -- in particular, the overlap/specialization requirements for partial impl need not be as stringent as for full impls. I'm not sure whether it's worth pushing a full design through as part of this RFC, or loosening the rules later after we've gained some experience.

@aturon
Copy link
Member Author

aturon commented Jul 14, 2015

Note that ruling out bad dropck interaction is nontrivial because of examples like the following:

trait Marker {}

trait Bad {
    fn foo(&self);
}

impl<T: Marker> Bad for T {
    default fn foo(&self) {}
}

impl<'a, T> Marker for &'a T {}

Here, given an arbitrary T you cannot deduce that T: Bad, but if you have an &T you can do so. Thus, the "parametricity check" for use of default would have to consider ways you could build a type around a type parameter that results in an applicable blanket impl. (Basically, we want to say that for there to be a specializable impl, you need to have a "nontrivial" bound on T, which then means that the parametricity dropck relies on still holds good.)

UPDATE: the main questions here are: can we convince ourselves that such a restriction retains the needed parametricity for dropck? And does such a restriction still support the main use cases for specialization? This is why I wanted to experiment a bit more before laying out a detailed proposal for the restriction.

@arielb1
Copy link
Contributor

arielb1 commented Jul 14, 2015

@aturon

Wouldn't partial impl<T:Copy> Clone for T be useless? It would also still conflict with the likes of impl<U:Clone,V:Clone> Clone for (U,V).

Also, "nontrivial bound": the absence of #[fundamental] basically saves us if we restrict to the single-constructor case, but I wouldn't want to have a rule that relies on that.

@arielb1
Copy link
Contributor

arielb1 commented Jul 14, 2015

Even more fun from Unsound Labs:

this useful-ish and rather innocent code:

use std::fmt;

pub struct Zook<T>(T);
impl<T> Drop for Zook<T> { fn drop(&mut self) { log(&self.0); } }
fn log<T>(t: &T) {
    let obj = Object(Box::new(t));
    println!("dropped object is {:?}", obj);
}

struct Object<T>(Box<T>);
impl<T> fmt::Debug for Object<T> {
    default fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
        write!(f, "<instance at {:?}>", (&*self.0) as *const T)
    }
}
impl<T: fmt::Debug> fmt::Debug for Object<T> {
    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
        self.0.fmt(f)
    }
}

can be exploited by this wolf in sheep's clothing (not even a single evil structure in sight!):

fn main() {
    let (zook, data);
    data = vec![1,2,3];
    zook = Zook(Object(Box::new(&data)));
}

enjoy!

@bluss
Copy link
Member

bluss commented Jul 14, 2015

@arielb1 Do you have any idea for how we can provide simple specialization? An example is PartialEq<&[U]> for &[T]. For the special case of PartialEq<&[u8]> for &[u8], we'd like to call memcmp. It's infeasible to use T: Reflect for this.

@tbu-
Copy link
Contributor

tbu- commented Jul 14, 2015

@bluss That sounds like one of the things the optimizer should be able to do reliably.

@bluss
Copy link
Member

bluss commented Jul 14, 2015

@tbu-: it does not (LLVM has no loop idiom recognize for memcmp in this case), and it's one of the example motivations for specialization.

@tbu-
Copy link
Contributor

tbu- commented Jul 14, 2015

That should probably also be filed for LLVM, then.

@TitanThinktank
Copy link

TitanThinktank commented Aug 19, 2019

not a single useful example out there to help us get going with using impl Trait, very sad

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 19, 2019

@TitanThinktank this RFC is about specialization. You can find an introduction to Traits (and impl Trait) in this chapter of the book: https://doc.rust-lang.org/book/ch10-02-traits.html

@Sydre
Copy link

Sydre commented May 18, 2020

How close from implementation is "lattice" specialization nowadays ?

@RustyYato
Copy link

RustyYato commented May 19, 2020

I don't think the lattice specialization was ever accepted. So we would need a separate RFC to extend specialization to handle it. But I think that will have to wait until we figure how to deal with the soundness hole in the current implementation of specialization.

@Sydre
Copy link

Sydre commented May 19, 2020

Weren’t the soundness concerns dropped alongside associated type specialization ?

That feature is so direly needed one could argue implementing lattice specialization on top or the current work is good enough for now.

@RustyYato
Copy link

RustyYato commented May 19, 2020

@Sydre

Spoiler alert: we have not fully solved them yet. But we see a viable way to ship a sound, useful subset of specialization in the meantime.

No, the soundness concetns aren't dropped. The minimal version of specialization referred to here is min_specialization, and that only allows specializing on types, and only in some limited cases. You can't specialize with traits. I don't think that the lattice rule is neccessary because there are ways around it

trait Foo { ... }
trait Bar { ... }

impl<T> Foo for T { ... } // defers to Bar
impl<T: Eq> Foo for T { ... }

impl<T> Bar for T { ... }
impl<T, U> Bar for (T, U) { ... }

Note: if we get a tuple (u8,u8) it will take the second Foo impl, and (f32, u8) will take the second Bar impl. This is a general pattern that can be applied in any situation where you need to make a choice, it's similar to if else chains. If Eq is implemented do this, else if type = (T, U) do this, else do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.