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

Document variance #186

Merged
merged 1 commit into from May 3, 2018
Merged

Document variance #186

merged 1 commit into from May 3, 2018

Conversation

matthewjasper
Copy link
Contributor

@matthewjasper matthewjasper commented Dec 30, 2017

First of a few PRs to improve the documentation of lifetimes in the reference. Based on the documentation in the Nomicon.

cc #9 (rust-lang/rfcs#738)

@Havvy
Copy link
Contributor

Havvy commented Dec 30, 2017

I don't know nearly enough about this to be reviewing it.

@matthewjasper
Copy link
Contributor Author

cc @gankro @nikomatsakis @arielb1 ?

Copy link
Contributor

@Gankra Gankra left a comment

Choose a reason for hiding this comment

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

I'm high on nyquil qhat's upp

src/subtyping.md Outdated
Since `'static` outlives `'a`, `&'static str` is a subtype of `&'a str`.

Subtyping also exists for [higher-ranked types]. Replacing a higher ranked
lifetime with a concrete lifetime produces a subtype.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I don't believe the documentation you linked specifies what a "higher ranked type" is. Not entirely sure what you mean here. Literally just function pointers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function pointers and trait objects

src/subtyping.md Outdated
// Error: in type `&'static &'a i32`, reference has a longer lifetime than
// the data it references
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Unclear to me why this merits discussion (is this meaningfully different from adding a note that "the expression must be parse"?)

src/subtyping.md Outdated
* `std::cell::UnsafeCell<T>` is invariant in `T`.
* `std::marker::PhantomData<T>` is covariant in `T`.
* Trait objects are invariant in their type parameters and associated types and
covariant in their [lifetime bound].
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be better written as a table, but idk

src/subtyping.md Outdated
The variance of `struct`, `enum`, `union` and tuple types is decided by looking
at the variance of the types of their fields. If a type parameter is used more
than once then the more restrictive variance is used. For example the following
struct is covariant in `'a` and `T` and invariant in `'b` and `U`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit unclear (and either very subtle or just actually wrong for co+contra). There's basically two interesting cases for multi-occurence:

  • If everything agrees on the variance, then that variance is used
  • If two different variances would apply, it's invariant

Copy link
Contributor

@Gankra Gankra left a comment

Choose a reason for hiding this comment

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

Much better, mostly minor editing comments.

| `fn(T) -> ()` | | contravariant |
| `std::cell::UnsafeCell<T>` | | invariant |
| `std::marker::PhantomData<T>` | | covariant |
| `Trait<T> + 'a` | covariant | invariant |
Copy link
Contributor

Choose a reason for hiding this comment

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

Thing I don't know but someone should check: is &'a mut Trait<T> + 'a actually invariant in 'a? (this is implied my the entries for &mut T and Trait<T> + 'a, but I would almost expect the compiler to magic this away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like it is invariant, but there is a coercion from &mut (Trait + 'a) to &mut Trait + 'b where 'a: 'b. The same as &mut (Trait + Send) and &mut Trait.

trait A {}
// This compiles (also with `*mut`, or `Box<RefCell>`)
fn coercion<'a, 'b>(x: &'b mut (A + 'static)) -> &'b mut (A + 'a) {
    x
}
// This does not
fn not_subtype<'a, 'b, 'c>(x: &'c &'b mut (A + 'static)) -> &'c &'b mut (A + 'a) {
    x
}

src/subtyping.md Outdated
Since `'static` "lives longer" than `'a`, `&'static str` is a subtype of
`&'a str`.

Since `'static` outlives `'a`, `&'static str` is a subtype of `&'a str`.
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not declared above, could we clarify that 'a here is a standin for "any generic lifetime in scope" (or maybe just "some lifetime"?)

src/subtyping.md Outdated
Since `'static` outlives `'a`, `&'static str` is a subtype of `&'a str`.

Subtyping also exists for [higher-ranked] function pointer and trait object
types. Replacing a higher ranked lifetime with a concrete lifetime produces a
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rewrite this to "function pointers and trait objects" (current way reads awkward)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the notion of a concrete lifetime really well-defined and/or clear here?

src/subtyping.md Outdated

Subtyping also exists for [higher-ranked] function pointer and trait object
types. Replacing a higher ranked lifetime with a concrete lifetime produces a
subtype.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting phrasing, I'm not sure if it makes sense to most.

src/subtyping.md Outdated
// Explicitly f: &(for<'a> Fn(&'a i32) -> &'a i32).
let f: &(Fn(&i32) -> &i32) = &(|x| x);
let g: &(Fn(&'static i32) -> &'static i32) = f;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm mistaken, doctest implicitly wraps the example in main, so you can get rid of these distracting functions.

Also consider renaming f and g to "base" and "subtype" or something more self-descriptive.

src/subtyping.md Outdated
y: *const T,
z: UnsafeCell<&'b f64>,
w: *mut U,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This example would benefit from inline comments or outline discussion (I originally missed that U was used twice).

@Havvy
Copy link
Contributor

Havvy commented Mar 8, 2018

@gankro Wanna give this a re-review? (You're allowed to say no)

Copy link
Contributor

@Gankra Gankra left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, this totally fell off my radar. This looks great!

Since `'static` outlives the lifetime parameter `'a`, `&'static str` is a
subtype of `&'a str`.

[Higher-ranked]&#32;[function pointers] and [trait objects] have another
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this escaped space necessary? otherwise markdown parses it as a reference?

Copy link
Contributor

Choose a reason for hiding this comment

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

Markdown would read it as a link with text of "[Higher ranked]" and a link reference to "function pointers" instead of as two separate links.

@Havvy
Copy link
Contributor

Havvy commented Apr 4, 2018

Rebase this on master and leave a note saying you've done so and I'll get it merged.

@matthewjasper
Copy link
Contributor Author

rebased, but mdbook isn't building on travis.

@alercah alercah merged commit 4b4a2af into rust-lang:master May 3, 2018
@matthewjasper matthewjasper deleted the variance branch October 18, 2018 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants