Explain motivation behind lifetimes #36755

Rantanen · 2016-09-26T19:50:05Z

Start the lifetime section with an explanation of the issues that lack of explicit lifetimes cause and how the explicit lifetimes solve these.

I had really hard time figuring out why I would need to care about the explicit reference lifetimes when going through the book at first. With strong background in C++, I'm familiar with the dangling reference problem - but given the section seems to focus more on the lifetime syntax and various ways to define lifetimes on functions and structs, I was unable to understand how they are used to solve the reference problem.

This PR is an attempt at getting the reader to understand what the explicit lifetimes are used for and why they are an awesome thing instead of a bit of syntax that just has to be written.

It's been less than a week that I've been diving into Rust so I'm far from certain about the terminology and technical correctness. I tried mimicking the existing terminology from the lifetimes section, but still no promises on getting it right.

rust-highfive · 2016-09-26T19:50:23Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

durka

Nice expanded example!

durka · 2016-09-26T20:17:01Z

src/doc/book/lifetimes.md

+
+```rust
+fn foo() {
+    let r = &0;         // Introduce reference: r


Just write let r; here. Initializing it to &0 and then re-assigning it later triggers another error because you didn't declare r mutable, which is irrelevant to the point about lifetimes.

durka · 2016-09-26T20:19:06Z

src/doc/book/lifetimes.md

-fn bar<'a>(x: &'a i32) {
+fn foo(id: u32) {
+    let data = "#1=foo;#2=bar";
+    let v : &str;


style is usually let v: &str;

durka · 2016-09-26T20:19:55Z

src/doc/book/lifetimes.md

+explicit in the function declaration:
+
+```rust
+fn data_value<'a,'b>(data: &'a str, key: &'b str) -> &'a str {


minor style nit: <'a, 'b> with space

durka · 2016-09-26T20:20:43Z

src/doc/book/lifetimes.md

-["Lifetime Elision"][lifetime-elision] below) them in common cases.
-Before we get to that, though, let’s break the explicit example down:
+["Lifetime Elision"][lifetime-elision] below) them in common cases. Before we
+get to that, though, let’s look at a simple example with explicit lifetime:


Eh2406 · 2016-09-26T20:28:01Z

"less than a week" and already helping with the docs! Thank you!

durka · 2016-09-26T21:29:47Z

Travis says:

failures:

---- Lifetimes_1 stdout ----
    error[E0106]: missing lifetime specifier
 --> <anon>:2:45
  |
2 |     fn data_value(data: &str, key: &str) -> &str {
  |                                             ^ expected lifetime parameter
  |
  = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `data` or `key`

error: aborting due to previous error(s)

thread 'Lifetimes_1' panicked at 'Box<Any>', src/librustc/session/mod.rs:183
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- Lifetimes_0 stdout ----
    error: `i` does not live long enough
  --> <anon>:6:14
   |
6  |         r = &i;         // Store reference of i in r
   |              ^ does not live long enough
7  |     }                   // i goes out of scope and is dropped.
   |     - borrowed value only lives until here
...
10 | }
   | - borrowed value needs to live until here

error[E0384]: re-assignment of immutable variable `r`
 --> <anon>:6:9
  |
3 |     let r = &0;         // Introduce reference: r
  |         - first assignment to `r`
...
6 |         r = &i;         // Store reference of i in r
  |         ^^^^^^ re-assignment of immutable variable

error: aborting due to previous error(s)

thread 'Lifetimes_0' panicked at 'Box<Any>', src/librustc/session/mod.rs:183

---- Lifetimes_2 stdout ----
    error[E0269]: not all control paths return a value
 --> <anon>:2:5
  |
2 |     fn data_value<'a,'b>(data: &'a str, key: &'b str) -> &'a str {
  |     ^

error: aborting due to previous error

thread 'Lifetimes_2' panicked at 'Box<Any>', src/librustc_errors/lib.rs:694
thread 'Lifetimes_2' panicked at 'couldn't compile the test', src/librustdoc/test.rs:283


failures:
    Lifetimes_0
    Lifetimes_1
    Lifetimes_2

test result: FAILED. 10 passed; 3 failed; 10 ignored; 0 measured

You'll need to put "```rust,no_run" at the top of your examples when you know they won't compile. Or possibly there is a way nowadays to annotate them with the expected error, cc @GuillaumeGomez ?

Rantanen · 2016-09-27T00:02:42Z

Took care of those issues hopefully.

Removed the = &0 assignment. Was debating, whether I should still add : &u32 for clarity but omitted it for now. I'm still unnerved by how good Rust's type inference is when the variable isn't used immediately. :)
<'a,'b> -> <'a, 'b>. One instance in code, one in text.
v : &str -> v: &str.
lifetime -> lifetimes.
Added ignore in the code examples. This was used in the other code examples in the same file. What's the difference between ignore and no_run?

Unfortunately I can't test the last one locally as the local Rust installation is interfering with the make check I'm guessing. Some of the llvm-phase tests are failing to found crate 'std' compiled by an incompatible version of rustc when running make check.

GuillaumeGomez · 2016-09-27T08:54:31Z

@durka: There is. If you expect an error, you have to add "compile_fail" and the expected error code (this second part isn't mandatory but it's better to have it). So for example:

    ```compile_fail,E0532
// you code...
    ```

steveklabnik

Thank you for this! This is good overall, but some small nits.

steveklabnik · 2016-09-27T14:02:42Z

src/doc/book/lifetimes.md

@@ -50,29 +50,87 @@ complicated. For example, imagine this set of operations:
 4. You decide to use the resource.

 Uh oh! Your reference is pointing to an invalid resource. This is called a
-dangling pointer or ‘use after free’, when the resource is memory.
+dangling pointer or ‘use after free’, when the resource is memory. In a
+simple case this would look like:


Teaching advice: I would stay away from "simple". Say something like "In a small case like this" or just "in a case like this."

The reason is, it might be simple to you, but not to your reader. If they hear you say "this is simple" and then they look at it and go "I have no idea what's going on", they get the impression that the topic isn't for them, and bail.

This applies almost universally in teaching, but especially in the chapter on the most complex topic in Rust 😄

steveklabnik · 2016-09-27T14:03:00Z

src/doc/book/lifetimes.md


 To fix this, we have to make sure that step four never happens after step
-three. The ownership system in Rust does this through a concept called
-lifetimes, which describe the scope that a reference is valid for.
+three. In the simple example above the Rust compiler is able to report the issue


steveklabnik · 2016-09-27T14:04:13Z

src/doc/book/lifetimes.md

+    let data = "#1=foo;#2=bar";
+    let v: &str;
+    {
+        let key = format!("#{}", id);        // -o key goes into scope


we use + rather than o elsehwere in this chapter for this

steveklabnik · 2016-09-27T14:04:43Z

src/doc/book/lifetimes.md

-["Lifetime Elision"][lifetime-elision] below) them in common cases.
-Before we get to that, though, let’s break the explicit example down:
+["Lifetime Elision"][lifetime-elision] below) them in common cases. Before we
+get to that, though, let’s look at a simple example with explicit lifetimes:


same here with simple

steveklabnik · 2016-09-27T14:04:51Z

src/doc/book/lifetimes.md

@@ -359,3 +418,4 @@ fn args<'a, 'b, T: ToCStr>(&'a mut self, args: &'b [T]) -> &'a mut Command; // e
 fn new(buf: &mut [u8]) -> BufWriter; // elided
 fn new<'a>(buf: &'a mut [u8]) -> BufWriter<'a>; // expanded
 ```
+


Rantanen · 2016-09-27T14:45:46Z

Changed to use `compile_fail` where applicable.

The i does not live long enough error didn't list the error code so the first example is missing that one.
Used # to include code in the compilation (but not HTML output) in one of the examples. This made ignore/compile_fail obsolete as the example compiles with the new code.

Changes based on the review comments.

Implemented more or less as suggested.

Changed the second example.

The original felt contrived to me and I was never that happy with it. It tried to have a purpose while demonstrating the problem. The new one still has a purpose, but (at least to me) it feels more obvious, which makes it less distracting.

Of course someone could argue that the example should be completely abstract with fn f(a: &str, b: &str) and variables a and b with no format! shenanigans. I'm fine changing it into this as well.

Minor:

Renamed the foo functions to main to make the examples work as stand alone programs.

(Also found make check-docs for testing the docs locally despite the normal make check failing \o/)

Rantanen · 2016-09-27T14:58:48Z

There seems to be some inconsistencies concerning the main() functions vs "global" scope in the other examples in the book.

Which one is the preferred style for the examples?

Without explicit `main()`:

fn take(v: Vec<i32>) {
    // what happens here isn’t important.
}

let v = vec![1, 2, 3];

take(v);

println!("v[0] is: {}", v[0]);

With explicit `main()`

fn main() {
    let a = 5;

    let _y = double(a);
    println!("{}", a);
}

fn double(x: i32) -> i32 {
    x * 2
}

Personally (now that I realize you can do it), I rather like the one without the main() for having better example-to-noise ratio.

steveklabnik · 2016-09-27T15:34:09Z

In general, without explicit main is considered best. Sometimes, you have to have it though. But 99% of the time, you don't.

bors · 2016-10-12T01:17:40Z

☔ The latest upstream changes (presumably #37090) made this pull request unmergeable. Please resolve the merge conflicts.

Rantanen · 2016-10-12T20:49:08Z

Didn't see guidelines for resolving the merge conflicts. I went with merging master to the PR. Is this okay, or would a rebase on top of the new master have been preferred?

Also curious on whether I should keep maintaining the PR. I'm guessing most of the documentation resources are going into rust-lang/book currently. If I should keep maintaining it, is there any status information?

(Mainly I'm curious on whether this PR is in some kind of a limbo due to open change requests. Is there a way to mark those as "done". Couldn't find any documentation in the GitHub review article.)

steveklabnik · 2016-10-12T22:29:25Z

A rebase on top of the new master is better, actually.

Also curious on whether I should keep maintaining the PR. I'm guessing most of the documentation resources are going into rust-lang/book currently. If I should keep maintaining it, is there any status information?

Yes please! That book is still months out from launching, this helps everyone in the meantime. And since my focus is on that, it's extra valuable to have people help out here.

(Mainly I'm curious on whether this PR is in some kind of a limbo due to open change requests. Is there a way to mark those as "done". Couldn't find any documentation in the GitHub review article.)

There's two reasons:

I missed the "a new commit was pushed" email to this PR, which is my fault.
This PR is assigned to @Manishearth , and should have been assigned to me. So I didn't see it in my "hey what are the PRs I'm assigned to" page. It's my fault for not double checking and assigning to me when I initially gave you feedback, sorry about that!

So first, let's fix that:

@bors: r? @steveklabnik

Second, I let me review this again, and then you can do the rebase and/or take care of any other nits, and then we can merge this. So sorry again.

steveklabnik · 2016-10-12T22:31:45Z

Yes, this looks great. If you can do that rebase, I can get this merged. Sorry again.

Start the lifetime section with an explanation of the issues that lack of explicit lifetimes cause and how lifetimes alleviate these.

Rantanen · 2016-10-12T22:49:31Z

No problem! Was partly my fault to begin with as I didn't request you to review the code like suggested in the contribution guidelines.

Rebase done.

GuillaumeGomez · 2016-10-13T13:13:49Z

Thanks!

@bors: r+ rollup

bors · 2016-10-13T13:13:50Z

📌 Commit cb90723 has been approved by GuillaumeGomez

bors · 2016-10-13T13:27:56Z

⌛ Testing commit cb90723 with merge 9cb498d...

bors · 2016-10-13T14:36:00Z

💔 Test failed - auto-linux-cross-opt

Rantanen · 2016-10-13T15:47:13Z

Seems like the auto-linux-cross-opt build was broken last night (UTC anyway) so I'm assuming there's nothing for me to do to get that resolved.

checking whether the C compiler works... no
configure: error: in `/buildslave/rust-buildbot/slave/auto-linux-cross-opt/build/obj/powerpc64-unknown-linux-gnu/rt/jemalloc':
configure: error: C compiler cannot create executables
See `config.log' for more details
make: *** [powerpc64-unknown-linux-gnu/rt/jemalloc/lib/libjemalloc_pic.a] Error 77
make: *** Waiting for unfinished jobs....

durka · 2016-10-13T15:49:49Z

That's the same bogus failure from #37119.

On Thu, Oct 13, 2016 at 11:47 AM, Mikko Rantanen notifications@github.com
wrote:

Seems like the auto-linux-cross-opt
https://buildbot.rust-lang.org/builders/auto-linux-cross-opt?numbuilds=25
build was broken last night (UTC anyway) so I'm assuming there's nothing
for me to do to get that resolved.

checking whether the C compiler works... no
configure: error: in /buildslave/rust-buildbot/slave/auto-linux-cross-opt/build/obj/powerpc64-unknown-linux-gnu/rt/jemalloc': configure: error: C compiler cannot create executables Seeconfig.log' for more details
make: *** [powerpc64-unknown-linux-gnu/rt/jemalloc/lib/libjemalloc_pic.a] Error 77
make: *** Waiting for unfinished jobs....

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36755 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAC3n8f82SQAadrE-O0CWr_O5B3CIGP1ks5qzlKigaJpZM4KG7Cl
.

arielb1 · 2016-10-13T16:39:09Z

@bors retry

alexcrichton · 2016-10-13T20:23:26Z

@bors: retry

Explain motivation behind lifetimes Start the lifetime section with an explanation of the issues that lack of explicit lifetimes cause and how the explicit lifetimes solve these. ---------------- I had really hard time figuring out why I would need to care about the explicit reference lifetimes when going through the book at first. With strong background in C++, I'm familiar with the dangling reference problem - but given the section seems to focus more on the lifetime syntax and various ways to define lifetimes on functions and structs, I was unable to understand how they are used to solve the reference problem. This PR is an attempt at getting the reader to understand what the explicit lifetimes are used for and why they are an awesome thing instead of a bit of syntax that just has to be written. It's been less than a week that I've been diving into Rust so I'm far from certain about the terminology and technical correctness. I tried mimicking the existing terminology from the lifetimes section, but still no promises on getting it right.

Rollup of 10 pull requests - Successful merges: #36307, #36755, #36961, #37102, #37115, #37119, #37122, #37123, #37141, #37159 - Failed merges:

rust-highfive assigned Manishearth Sep 26, 2016

durka reviewed Sep 26, 2016

View reviewed changes

steveklabnik suggested changes Sep 27, 2016

View reviewed changes

rust-highfive assigned steveklabnik and unassigned Manishearth Oct 12, 2016

steveklabnik approved these changes Oct 12, 2016

View reviewed changes

Explain motivation behind lifetimes

cb90723

Start the lifetime section with an explanation of the issues that lack of explicit lifetimes cause and how lifetimes alleviate these.

Rantanen force-pushed the master branch from de7232d to cb90723 Compare October 12, 2016 22:41

sophiajt mentioned this pull request Oct 14, 2016

Rollup of 10 pull requests #37170

Merged

bors added a commit that referenced this pull request Oct 14, 2016

Auto merge of #37170 - jonathandturner:rollup, r=jonathandturner

a8d189a

Rollup of 10 pull requests - Successful merges: #36307, #36755, #36961, #37102, #37115, #37119, #37122, #37123, #37141, #37159 - Failed merges:

bors merged commit cb90723 into rust-lang:master Oct 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explain motivation behind lifetimes #36755

Explain motivation behind lifetimes #36755

Rantanen commented Sep 26, 2016

rust-highfive commented Sep 26, 2016

durka left a comment

durka Sep 26, 2016

durka Sep 26, 2016

durka Sep 26, 2016

durka Sep 26, 2016

Eh2406 commented Sep 26, 2016

durka commented Sep 26, 2016

Rantanen commented Sep 27, 2016

GuillaumeGomez commented Sep 27, 2016

steveklabnik left a comment

steveklabnik Sep 27, 2016

steveklabnik Sep 27, 2016

steveklabnik Sep 27, 2016

steveklabnik Sep 27, 2016

steveklabnik Sep 27, 2016

Rantanen commented Sep 27, 2016

Rantanen commented Sep 27, 2016

steveklabnik commented Sep 27, 2016

bors commented Oct 12, 2016

Rantanen commented Oct 12, 2016

steveklabnik commented Oct 12, 2016

steveklabnik commented Oct 12, 2016

Rantanen commented Oct 12, 2016

GuillaumeGomez commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

Rantanen commented Oct 13, 2016

durka commented Oct 13, 2016

arielb1 commented Oct 13, 2016

alexcrichton commented Oct 13, 2016

Explain motivation behind lifetimes #36755

Explain motivation behind lifetimes #36755

Conversation

Rantanen commented Sep 26, 2016

rust-highfive commented Sep 26, 2016

durka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eh2406 commented Sep 26, 2016

durka commented Sep 26, 2016

Rantanen commented Sep 27, 2016

GuillaumeGomez commented Sep 27, 2016

steveklabnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rantanen commented Sep 27, 2016

Changed to use compile_fail where applicable.

Changes based on the review comments.

Changed the second example.

Minor:

Rantanen commented Sep 27, 2016

Without explicit main():

With explicit main()

steveklabnik commented Sep 27, 2016

bors commented Oct 12, 2016

Rantanen commented Oct 12, 2016

steveklabnik commented Oct 12, 2016

steveklabnik commented Oct 12, 2016

Rantanen commented Oct 12, 2016

GuillaumeGomez commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

Rantanen commented Oct 13, 2016

durka commented Oct 13, 2016

arielb1 commented Oct 13, 2016

alexcrichton commented Oct 13, 2016

Changed to use `compile_fail` where applicable.

Without explicit `main()`:

With explicit `main()`