Support variance for type parameters by nikomatsakis · Pull Request #738 · rust-lang/rfcs (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation38 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
A revised proposal for properly supporting variance in Rust's type system.
👍 I like PhantomData, this will make writing FFI code significantly easier.
Variance for type parameters: This is one of the things that users don't notice if it works properly, and is utterly confusing when types are unnecessarily invariant. This alone is a good reason to infer variance instead of requiring explicit variance annotations.
| which can't do any harm. Here is an example: |
|---|
| ```rust |
| fn approx_context<'long,'short>(t: &Context<'long>, data: &'short Data) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this meant to take a reference for the first parameter? As it is, the example is a compile error because do_something expects a Context, but gets passed a &Context in approx_context.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Another alternative could be to have inference as this RFC suggests, but then emit an error if a variance annotation does not match the inferred variance, telling the user exactly what change needs to be made:
type parameter `T` is covariant, so you should declare `Foo` as `struct Foo<+T>`.
If you didn't intend that, add a `PhantomData<fn() -> T>` marker or equivalent to make `T` invariant.
Only inference makes it easier to write code, but it does violate the Rust philosophy that the types of public APIs should be explicitly written out.
Regarding annotations, the obvious "lightweight" approach is to add a single punctuation character for each variance, for instance "=" or nothing for invariance, "+" for covariance, "-" for contravariance.
Not sure whether this is better or not than just inference, but being the only violation of having explicit types for APIs is a bit disturbing.
| desired behavior as a fn that I realized I had it backward, and |
|---|
| quickly found the counterexample I give above. This gives me |
| confidence that expressing variance in terms of data and fns is more |
| reliable than trying to divine the correct results directly.* |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
divine → derive?
Instead of PhantomData<*mut T>, could PhantomData<&'static mut T> work?
@kennytm using PhantomData<&'static mut T> would require that T:'static, so it's probably not what you want.
Regarding annotations, the obvious "lightweight" approach is to add a single punctuation character for each variance, for instance "=" or nothing for invariance, "+" for covariance, "-" for contravariance.
"+" or "-" will be hard to understand for users not familiar with variance.
If we have to use explicit annotations, I'd prefer the C# approach using the keywords "in" and "out":
nothing = invariant, "out" = covariant, "in" = contravariant
@bill-myers yes, I agree but also disagree.
The reasons I would prefer not to have explicit variance annotations are listed in the RFC, but I'll repeat them:
- What do we call these annotations? Symbols like
-,+, etc are notoriously confusing. Keywords like "covariant" are as well. Keywords like "in" and "out", as used in C#, don't fit so well in Rust, where the "interface" and "data" declarations are separated (for example,Vec<out T>just doesn't make sense to me). - Having both annotations and phantom data seems very non-DRY, and I've found that the phantom data approach is both more broadly applicable and more reliable in terms of getting the right results.
That said, it is nice to be able to declare the expected variance (and I've found myself using internal Rust compiler debugging magic to force it to print out the results of inference on occasion). I imagine it might be nice to have a standard way of declaring the results you expect -- one option remains the writing of unit tests asserting the expected behavior.
Note that we've been using inference for quite some time now actually, it's just that it only applies to lifetime parameters (but it is quite crucial there; Rust would not work at all without it). Experience has been fairly positive thus far I would say -- the only real issues I see are when the expected results don't materialize (particularly around Option, where lifetime checking is much stricter, as the RFC shows).
Ah, and note that having some variance declaration also requires us to settle on a terminology around lifetime parameters. Overall it just feels like a lot more complexity to present the user with.
I'm also in the camp of preferring interfaces to be made explicit, but respect the objection that there's no (obvious) ergonomic and easily comprehensible way to do so. PhantomData is a cool idea should we choose to go with inference.
My one small contribution to the question of potential explicit syntax is that I believe variances should be thought of and presented as additional capabilities granted, relative to the baseline of "invariant", to the user of the API - just like everything else that you publicly expose in an API is an additional capability relative to not having exposed it. So one capability is "can be up-casted", another is "can be down-casted" ("cast" is maybe not the right word), if you have only one or the other it's what we call "covariant" or "contravariant", while if you have both it's "bivariant". The crucial point is that this is the reverse of how they're usually framed, which is in terms of how the type parameter is used by/within the type being defined, which induces restrictions: where in means it's used as a (function) input, meaning you can't cast it in one direction, out means it's present as a data member or other "positive" position, meaning you can't cast it in the other, and the combination of the two results in invariance. What we want is for the two keywords (whatever we're able to come up with) to correspond to how the type parameter is not used by the type being defined, which allows for greater freedom for the user of the type, with the combination of both keywords (meaning the parameter is not used at all - neither in positive nor negative positions) expressing bivariance.
(Incidentally, this also happens to be how the Functor and Contrafunctor type classes are used by Glasgow Haskell's lens library.)
Nice, I'm in favour of the inference approach and using 'annotations' for assistance (explicit variance annotations are nearly always awful). I was thinking that this is kind of hacking around writing exactly what the author means, but I think what I was thinking of is the phantom keyword stuff in the future directions section - I'd be keen to see that implemented asap.
As another reason to avoid explicit annotations, I imagine that the fact that Rust subtyping and thus variance only affects lifetimes will further confuse users - variance in other languages is almost always discussed in terms of containers and the type of elements. That we use implicit coercions for many subtyping-ish things, but that these are not covered by the variance annotations might be odd.
Question: is there any effect in using the traits here as bounds on type parameters? e.g., fn foo<X: PhantomFn() -> i32>(x: X) { ... }
| 2. Rewrite safe abstractions to use `*const` (or even `usize`) instead |
| of `*mut`, casting to `*mut` only they have a `&mut self` |
| method. This is probably the most conservative option. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for 2 - this is what I do in my personal projects, I didn't realise the std libs used *mut this way.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have currently implemented option 2: that is, *mut is invariant. I had been leaning towards what I think was 3 (ignore *mut references entirely and reauire explicit markers), but I fear that that is more error-prone. Basically if people aren't thinking carefully it's easy to make a covariant type that ought to have been invariant. Option 2 is correct by default, and requires you to opt-in (by using distinct types) to covariance. On the variance branch I've just been rewriting standard library stuff not to use raw *mut pointers but rather safe wrappers like Unique, which I think improves the code overall, since it's more declarative. It also handles the OIBIT requirements and so forth. Those are currently unstable though, we'll probably want some iteraiton on their design -- right now they are just kind of shallow wrappers around a raw pointer, we might be able to do better. (Note also that Unique on the branch is quite different from Unique on master).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hashmap (and my working branch of btree) don't use Unique because they contain what is essentially a void pointer to "bytes".
One possibly less confusing syntax for denoting the variance type would be
- T (or =T) for invariant type (_ equals T)
- T/ for covariant type T (read like T over _)
- /T for contravariant type T (read like _ over T)
The idea is that the / denotes the position of our specified type related to T in an assumed hierarchy.
I don't know how much this would complicate the parser, though.
In any event, inference has my vote.
| PhantomData // covariance |
|---|
| PhantomData<*mut T> // invariance, but see "unresolved question" |
| PhantomData<Cell> // invariance |
| PhantomData<fn() -> T> // contravariant |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be covariant? fn() -> T is roughly isomorphic to T, so the result should be the same as if PhantomData<T> were used. Alternatively, this could be changed to fn(T).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, sorry, I meant fn(T)
I was thinking that this is kind of hacking around writing exactly what the author means, but I think what I was thinking of is the phantom keyword stuff in the future directions section - I'd be keen to see that implemented asap.
Me too, I'd be happy to revise the RFC to include the phantom syntax as a first-class thing, even if I'd want to land a branch that uses the markers for now. The usability improvements of having concrete syntax would be quite high, I think, and given how central all of this is to the type system, it seems justified to me.
Question: is there any effect in using the traits here as bounds on type parameters? e.g., fn foo<X: PhantomFn() -> i32>(x: X) { ... }
You can do it but it's meaningless -- since PhantomFn defines no methods, and is implemented for all types, adding such a bound buys you nothing. The only place that PhantomFn is useful is in the supertraits listing.
| ## Why variance should be inferred |
|---|
| Actually, lifetime parameters already have a notion of variance, and |
| this varinace is fully inferred. In fact, the proper variance for type |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should read "variance".
-1 for variance inference. I believe that while having implicit variance would help user if it works properly, it would cause utter confusion when it doesn't. The fact that rustdoc hides private fields doesn't help at all when someone hits a variance error in an external library. Cases that require anything other than contravariant lifetimes require (relatively) advanced concepts any way, and the confusion most likely stems from the fact that the names are scary.
I think it is simpler to use contravariance by default, use struct Foo<no_shrink_lifetime 'a> {...} or #[no_shrink_lifetime('a)] for invariant lifetimes, and use struct Foo<allow_extend_lifetime + no_shrink_lifetime 'a> {...} or #[allow_extend_lifetime('a)] #[no_shrink_lifetime('a)] for covariance
The fact that rustdoc hides private fields doesn't help at all when someone hits a variance error in an external library.
It shouldn't be too hard (knock on wood) for rustdoc to show variance on type/lifetime parameters. There's an open issue for that here rust-lang/rust#22108
@theemathas, the cardinality of confusion on inference failure is directly proportional to the quality of error messages and documentation. As long as: a) the rules aren't too arcane to be explained to Joe coder and b) it is possible for the compiler to show the location in the code where to add markers/annotations/whatever, confusion should be quite limited.
While there has not been a great deal of feedback, there is broad agreement that the new Phantom markers are clearer than today's markers (though could perhaps be more clear with special syntax), and that some variance for type parameters is needed. While some have expressed reservations about inference here, it's worth remembering that this inference has long been performed (though not largely used) in the past. Given that the variance is strictly related to lifetimes (the only form of subtyping), being implicit here seems worth at least attempting, and the implementation has been sitting on the side being rebased for quite a long time now.
We've decided to merge this RFC and land the implementation to gain some experience with this system. Please try it out and give feedback as to whether inference ever leads to unclear results in practice.
This was referenced
Feb 12, 2015
In my library I currently have this code:
pub struct TextureType1 { handle: u32 } pub struct TextureType2 { handle: u32 } // ... around 50 different texture types
pub struct FramebufferObject<'a> { attachments: Vec, marker: ContravariantLifetime<'a>, }
impl<'a> FramebufferObject<'a> { pub fn attach1(&mut self, tex: &'a mut TextureType1) { self.attachments.push(tex.handle); }
pub fn attach2(&mut self, tex: &'a mut TextureType2) {
self.attachments.push(tex.handle);
}}
This ensures that the FramebufferObject doesn't live longer that the attached Textures.
How would I do that with PhantomData? Do I need to create some kind of enum containing all the possible PhantomData types or something?
@tomaka if I properly understand what you are doing, I think you would write something like: marker: PhantomData<&'a ()>,
In other words, the phantom data represents the hidden references to data of lifetime 'a.
Congrats guys, my code got a little uglier.
use std:📑:MarkerTrait; pub trait LoadCommandBody: MarkerTrait {}
Thanks for that.
Constructive: at least put MarkerTrait in the prelude.
MarkerTrait definitely looks hacky.
The syntax trait A : B {} is supposed to mean that all types that implement A must also implement B.
When you write trait A : MarkerTrait {}, it looks like MarkerTrait applies of the trait A itself, which is contrary to this definition. It makes no sense for a regular type like i32 to implement MarkerTrait.
@tomakaMarkerTrait is fun, every sized type implicitly implement it.
fn main() {
fn f<T: std:📑:MarkerTrait>(_: T) {}
struct S;
f(S);
}
I'm complaining about the name MarkerTrait. Why would i32 implement a trait whose name is MarkerTrait? There has to be a better way to name this, like NoSelfVariance or something.
@tomaka Perhaps Rust should differentiate inheritance from type application. As it is, traits implementing traits is more like subtyping than trait implementation--if a type implements a trait, it implements all 'supertraits' of the trait, just as if a value is an instance of a type, it is an instance of all supertypes of the type. (I think this is why Haskell chose to use the class/instance terminology for its typeclasses.) In that case, traits describe sets of types (or, more generally, relations on types [in the database sense] which Rust represents using associated types) and types describe sets of values, and values don't need to be sets.
Yes, MarkerTrait needs a new name. Is it clearer if you explain it in terms of marking something? I don't think so.
If you dislike NoSelfVariance, which at least warns folks about the strangeness, then maybe SetOfTypes, PhantomMethod, StaticNeeded, etc. And one should consider prefixing the term with a term like Common.. or Typical.. like TypicalSelfCovariance or CommonPhantomSelf. Just whatever makes the most sense when you think about writing the documentation that explains it.
As an aside, I initially wrote the above with PhantomConstructor, but appears that's actually wrong. It's a phantom getter method really.
Neither PhantomFn nor MarkerTrait exist in today's Rust. This RFC's text is fairly outdated wrt to the current state of the type system.
PhantomData is the only mechanism one uses to specify extra semantics.
Labels
Proposals relating to the Drop trait or drop semantics
Type system related proposals & ideas
Variance related proposals & ideas