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
Rewrite lifetimes section #607
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
I think this is ready for review. I accidentally messed up my commit history so I went ahead and started new and rebased. |
@@ -0,0 +1,14 @@ | |||
Some lifetime patterns are overwelmingly common and so they may be elide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
I tweaked a bunch of stuff but a lot of it is still the same. I'd happily squash to 1 or 2 commits if you like it as is but want fewer commits. |
cc #613 (I see that this pull request resolves that by adding a section on lifetime bounds) |
write would look like this: | ||
Since lifetimes *currently* have no explicit type or name associated with them, | ||
usage will require generics (similar to [closures][anonymity]). Somewhat | ||
peculiarly, lifetime annotation has a second additional meaning. `foo<'a, 'b>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not really how I think of it. I presume foo<'a,'b>
refers to a type? In that case, foo<'a, 'b>
is just as simple as "foo
, where 'a
and 'b
are substituted for the lifetime parameters within the definition", just as Vec<i32>
is "Vec
, where i32
is substituted for the type parameter in the definition". Now, it is also true that if you have foo<'a ,'b>
, this type can only be referenced where 'a
and 'b
are in scope, so that implies that the structure is limited by the minimum of 'a
and 'b
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, foo<'a, 'b>
is meant to represent a type such as a struct
or a function or something else.
That makes sense and I think I can tweak it to use a different approach. I've been generally explaining struct Num(i32)
declarations and usage independently. This has worked okay so far. A better approach might be to introduce struct Ref<'a>(&'a i32)
via a "ref is subordinate to owner" approach (the ref must come from somewhere but I've kinda ignored that part).
I'll try it and see if I can improve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nikomatsakis I tweaked it to be more like you suggested.
61f8388
to
b8a2aeb
Compare
// cannot be coerced into a larger one. | ||
// | ||
//let y: &'a i32 = &_x; | ||
// ERROR: `_x` does not live long enough | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nikomatsakis This function above was slightly problematic with your suggested explanation since you cannot rely on input borrowing being returned and deduce the lifetime requirements from that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, this function makes no sense at all -- a random lifetime parameter that is not used anywhere is not particularly useful or meaningful. (Really, it ought to be illegal.)
That said, I don't quite follow how this example is in conflict with the explanation I suggested. I'm not even 100% sure what explanation you're referring to :). I thought I was explaining how to interpret a type like Foo<'a>
, which does not appear in this example, right?
But in any case, the existing text is correct: whatever scope 'a
corresponds to (and, given that 'a
is completely unused, the caller can pick whatever scope they can name), it will be something larger than the current function, and _x
is SMALLER than the current function, so this code is in error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this explanation. Anyway, the rules for Foo<'a>
as a struct
or foo<'a>
as a function are basically the same. Rather than try to explain the same thing twice once for structs
and once for functions, I tried to use foo<'a, 'b>
as a stand-in for both. Maybe it wasn't successful...
Good. If the explanation is okay then I'm okay with it. As for this example being random...it's not really. It's due to trying the obvious thing following an unhelpful error message. Consider this:
fn test() { let x: &'a i32 = &7; }
Error:
<anon>:1:21: 1:23 error: use of undeclared lifetime name `'a` [E0261]
<anon>:1 fn test() { let x: &'a i32 = &7; }
Oh, it needs declaring. Let's try:
fn test<'a>() { let x: &'a i32 = &7; }
Error:
<anon>:1:35: 1:36 error: borrowed value does not live long enough
<anon>:1 fn test<'a>() { let x: &'a i32 = &7; }
Hmm...is manual annotation of lifetimes possible?
Made another slight tweak based on @nikomatsakis's comment. I'm not sure if I should keep asking for r? after fixing things or what. Or who I should be asking... @nikomatsakis @alexcrichton Thanks for critiquing so far. Both your comments/critiques have been helpful. Especially @nikomatsakis. |
Rebased |
If someone could list an ETA for review by Friday that would be helpful to me. Stuff doesn't happen much over the weekends usually. |
It has been a week. Review would still be nice. Maybe some things still need changes but I'm not sure how I should change them. |
Still looking for review. It's been another week. Just trying to keep this from being forgotten. |
with the run up to rustcamp, i am very behind. Sorry :( |
@mdinger hoping to get to this sometime today, we'll see with the release |
That would be so great! I'd pretty much given up on someone reviewing this. |
Actually finally reviewing this. built it locally, reading now. |
annotated. These functions are generic and we must tell the compiler what is | ||
the relationship between the lifetimes of the objects that appear in the | ||
arguments and the output. | ||
The borrow checker utilizes explicit lifetime annotation to reason about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say 'annotations" here, which is a small difference, but makes it way easier for me to parse.
|
||
// Borrows (`&`) of both variables are passed into the function. | ||
// If a borrow is truly a borrow, the variable must be returned, | ||
// otherwise a borrow would be transferring ownership. This means |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make this sentence
When you pass a function a borrow as an argument, the borrow ends at the end of the function. The alternative to passing a borrow would be trasnferring ownership.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this change. As written, I'm trying to leverage the idea that people understand that borrows must always come back. If they don't come back then they can't be borrows; they must be something else. It's the only option. Then, a transfer would make perfect sense.
With your suggestion, I would immediately ask "why must they be returned"? I wouldn't necessarily see any reason they must be returned other than that is what the docs say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that you don't return, like
fn foo<'a>(&'a i32)
Does not return anything, but is just fine. You don't need to explicitly return the reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not talking about explicit returning. I'm talking about doing something which would preclude the reference from ever being able to return to it's owner. For example, if I tried this:
// Would this be the lifetime drawing?
fn foo<'a>(a: &'a NonCopyType) { // ---|
// |
let var = *a; // ------------------|
...
bar(var);
}
Why would it fail? Well, it would force a move but a move can't be done with a borrow. Why not? Because that would make it something other than a borrow. I consider this related to lifetime size. foo()
above has lifetime size which must be at least as large as the function.
Why must this lifetime size be larger than the function? Because borrows must always be returnable. The borrow being returned at the end is just a minor point you could learn after this is realized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@steveklabnik Should I just find you on IRC tomorrow to try and hash out what to do about this section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe yeah. I"m not sure how much I'll be on. Still thinking about this.
Whew! Done. I think this is definitely an improvement, just some nits. |
Okay cool. I'll try to get the changes fixed quickly. I'd like this to land if possible. |
Fixed |
rebased |
Rewrite lifetimes section
I'm still not 100% sure about that one example, but this has been sitting around forever, and I want to get it landed. Thanks again for being patient @mdinger ❤️ |
woooo! @steveklabnik @nikomatsakis @alexcrichton Thanks for reviewing even if only for a little while! |
[WIP]I completely glossed over elision and I may just insert a link to the book section where it discusses elision...not sure[TODO]
See also
sectionsThis reddit thread details a more advanced example which I don't get yet. Maybe I'll look at it again later.[Defer to later]Some testcases might be nice.[Defer to later][EDIT]
Fixes #154