rust-lang / nomicon

The Dark Arts of Advanced and Unsafe Rust Programming
https://doc.rust-lang.org/nomicon/
Apache License 2.0
1.74k stars 255 forks source link

Simplify "Subtyping and Variance" section #262

Open muscovite opened 3 years ago

muscovite commented 3 years ago

I think the Subtyping and Variance section could be improved by some changes for simplicity and clarity. Here are a few initial thoughts. If any of these seem worth pursuing, I'm eager to take a stab at addressing them!

Main metaphor is confusing

The class-based subtyping of Animals and Cats/Dogs feels backwards. Indeed, the section itself even notes:

This is a large source of confusion, because it seems backwards to many

Some of the confusion noted by the author of issue 124 might be resolved with a metaphor whose relationships are closer analogs to Rust's lifetime relationships. For example, we could frame lifetime relationships in terms of privilege levels:

The early examples of the section might look something like this:

trait User {
    fn login();
}

trait Admin: User {
    fn change_system_time();
}

...

fn login(account: User) {
    account.login();
}

...

let a: Admin = ...;
login(a);         // ERROR: expected Admin, found User

Dog subtype is confusing

Dog is presented as a "sibling" to Cat; that is, the only relationship between the two is that they are both subtypes of Animal. However, when it comes to lifetimes, 'a and 'b will always be related by a hierarchical relationship. Either 'a is more long-lived than 'b, or, if they live for the same amount of time, they're really just the same lifetime.

Circling back to the privilege level analogy, Dog could be replaced by an account with guest-level privileges.

This section wasn't quite accurate - see comments below.

Type lifetimes should be explicitly called out

One source of confusion for me, similar to the one raised in this comment, was that the section talks about variances over some type T. However, type T implictly has a lifetime, and the variance we're concerned with is really variances over this lifetime. This concept is never explicitly mentioned. I think a comment about this near the start of the Variance section would help.

Justification of rules feels inverted

If we look at our table of variances, we see that &mut T is invariant over T. As it turns out, this completely fixes the issue!

The quoted text implies that the problematic code snippet is fixed by referencing the variance table. This feels like the reverse framing of the true issue. It's more that the problematic code snippet is an example of a class of problems that necessitates the variance rule.

Gankra commented 3 years ago

You can absolutely have unrelated lifetimes --

fn blah<
  'a, 
  'b, 
  'c: 'a + 'b
>(
  a: &'a u32, 
  b: &'b u32, 
  c: &'c u32,
)

the duration of a and b has no relationship, but c is known to cover both of their ranges. Thus Vec<&'c u32> could be passed where Vec<&'a u32> or Vec<&'b u32> are expected.

muscovite commented 3 years ago

@Gankra thanks for the correction!

I still think the metaphor is confusing due to the inversion described in the "Main metaphor is confusing" section. For example, if we were to try to annotate your code snippet with the existing Animal metaphor:

fn blah<
  'a, // Animal
  'b, // also Animal?
  'c: 'a + 'b // Cat??
>(
  a: &'a u32, 
  b: &'b u32, 
  c: &'c u32,
)

Using another metaphor, such as the proposal of users/privilege levels, we could model this as

fn blah<
  'a, // User A, ex. has permissions to edit folder 1
  'b, // User B, ex. has permissions to edit folder 2
  'c: 'a + 'b // Admin C, ex. has permissions to edit both folders 1 and 2
>(
  a: &'a u32, 
  b: &'b u32, 
  c: &'c u32,
)
marioortizmanero commented 2 years ago

Also, after reading the section I found that the writing could do with some polishing.


Mainly phrases that were too long:

As it turns out, the argument for why it's ok for Box (and Vec, Hashmap, etc.) to be covariant is pretty similar to the argument for why it's ok for lifetimes to be covariant: as soon as you try to stuff them in something like a mutable reference, they inherit invariance and you're prevented from doing anything bad.

There is no problem at all with the fact that we have forgotten that mr_snuggles was a Cat, or that we overwrote him with a Dog, because as soon as we moved mr_snuggles to a variable that only knew he was an Animal, we destroyed the only thing in the universe that remembered he was a Cat!

In the meowing dog problem we take a subtype (Cat), convert it into a supertype (Animal), and then use that fact to overwrite the subtype with a value that satisfies the constraints of the supertype but not the subtype (Dog).

These could be split up into smaller ones that are easier to digest. It's a popular technical writing recommendation. It's specially important if you're treating complex topics, like this one.


The "however" in here should also have a comma afterwards:

NOTE: The typed-ness of lifetimes is a fairly arbitrary construct that some disagree with. However it simplifies our analysis to treat lifetimes and types uniformly.

And here:

However Box makes it easier to focus on by-value aspect of references that we partially glossed over. 2

And here:

However if A is used in multiple fields:


I read &mut T as "mut T" or "mutable reference to T", so it should be "a", not "an".

So even though references are covariant over their lifetimes, they "inherit" invariance whenever they're put into a context that could do something bad with that. In this case, we inherited invariance as soon as we put our reference inside an &mut T.


I would strongly recommend to install a grammar checker to detect these cases automatically. I personally did recently and it's incredibly helpful to make my writing more clear and concise. I personally use grammar-guard, which is a plugin for NeoVim, but there's a ton of them out there.

Thanks for all the work on the Rustnomicon!

thomcc commented 1 year ago

I recently ran into several people who, because of the main metaphor, believed that supertraits/subtraits were a subtype relationship in Rust, and believed that it was this and lifetimes that variance applied to.

So, I came to file an issue about this, but this one seems close enough (maybe I'm mistaken). At the very least, I'd like to say that I think this needs work still.

EDIT: Ah, I guess there's an open PR about it. Sorry.

qsantos commented 7 months ago

For reference, the PR that the preceding comment refers to is #340, which is associated with issue #339. The “main metaphor” has been completely removed, preferring a more focused conversation on the abstract concepts.

Considering this, could this issue be closed? If there are still standing points, maybe they could be merged into https://github.com/rust-lang/nomicon/issues/339.