Overhaul Lifetimes Section (was "Help Needed: Extend lifetimes section") #4

vext01 · 2017-03-16T16:15:53Z

Hi,

As mentioned in #2, I'm trying to better document the lifetimes system of Rust, as it's the part all beginners trip on.

I was trying to combine the collective knowledge of what the nomicon already tells us about desugraing to see implicit lifetimes, and the intuition given by the answer of my stack overflow question (http://stackoverflow.com/questions/42807679/clarify-the-meaning-of-binding-two-references-to-differently-scoped-referents-to/42808207#42808207).

All was going well until I tried to show a counter example where the lifetime constraints imposed by a function signature can't be satisfied (the last rust code block added in this PR). At this point something has gone wrong, since I am unable to come to the same conclusion as the rust compiler. The compiler rejects the program, the intuitive method I am trying to explain within this PR does not reject the program!

Can an expert take a read of the stack overflow question, then the change in this PR and comment on where I have gone wrong?

Am I de-sugaring correctly?
Are the rules for converting lifetimes correct?
Are the rules for detecting an inconsistency correct?

I really wonder if there is a better way to express how the compiler is assigning and checking lifetimes, as it's not very intuitive as it stands. Before reading the nomicon, I've always though of the lifetime of a reference as the lifetime of the referent, but the nomicon seems to explain it in another way.

Cheers

vext01 · 2017-03-16T16:16:36Z

(excuse the typos in the commit messages, I will squash this all into one nice commit at the end)

steveklabnik · 2017-03-16T16:48:09Z

(excuse the typos in the commit messages, I will squash this all into one nice commit at the end)

No worries at all!

vext01 · 2017-03-16T17:44:28Z

I think the problem here is the rules for "converting" lifetimes. I don't think it helps to think in that way.

Given the de-sugared code:

fn main() {
    'a {
        let s1 = String::from("short");
        'b {
            let res: &'b str;
            'c {
                let s2 = String::from("a long long long string");
                'd {
                    // Annonymous scope for the borrows of s1 and s2
                    // Assigning to the outer scope causes s1 and s2 to have 'b
                    res: &'b = shortest(&'d s1, &'d s2);
                    println!("{}", res);
                }
            }
        }
    }
}

fn shortest<'k>(x: &'k str, y: &'k str) -> &'k str {
    ...

The way my colleague see this is, the lifetimes are inconsistent because:

The sig of shortest() tells us the return value has the same lifetime as the two args.
The args were marked &'d by de-sugaring, so we must show that the return value lifetime, &'d, is consistent with the lifetime we annotated earlier: &'b.
To be consistent, &'d would have to live longer than &'d, which it does not, so after the call res may refer to a freed value.

Therefore the program is inconsistent. Makes sense to me, but there might be a more intuitive way of thinking about this.

vext01 · 2017-03-17T12:32:08Z

Alright. Can someone with jedi rust skills check the most recent version.

I've borrowed (no pun intended) the style of this post, which makes a great deal of sense to me.

If you guys like this, then I would then need to go back and revise the prior sections to mark references with the lifetimes of their referents, and I would probably want to adjust the flow a bit too.

There's also the issue of (I think) move semantics giving rise to additional gotchas (since they limit the lifetimes of values), but I think I'd like to tackle that as a separate PR.

@steveklabnik Are you able to review this, or do you know someone who could?

Thanks.

vext01 · 2017-03-17T12:34:42Z

(I'm unable to test under mdbook, due to ogham/rust-ansi-term#27, so I've been just pushing and viewing in GH ;) )

steveklabnik · 2017-03-17T13:20:04Z

@vext01 I will try to soon, but in addition, @aturon and I have been talking about having lang team / docs team coordination to work on the reference; we may want that for the nomicon as well.

vext01 · 2017-03-17T14:22:44Z

OK great. Let me know if I can be of any help, as I'm keen to learn about this stuff.

vext01 · 2017-03-27T09:52:20Z

Sorry to persist, but has anyone at moz had a chance to review this?

steveklabnik · 2017-03-27T13:28:01Z

I have not, sorry :( I should be able to at least give it a cursory review soon; still have to figure out those meetings

aturon · 2017-03-27T23:19:10Z

cc @gankro

frewsxcv · 2017-04-02T21:25:42Z

src/lifetimes.md

+
+Consider the following program:
+
+```rust,ignore


Is there any reason why this is ignore? Seems like it should be compilable and runable, no?

Same with some of the other examples here

frewsxcv · 2017-04-02T21:28:45Z

src/lifetimes.md

+
+```rust,ignore
+print_shortest(&s1, &s1);
+


Nit: unnecessary newline here

frewsxcv · 2017-04-02T21:30:14Z

src/lifetimes.md

+}
+```
+
+Again, at the call-site of `shortest` the comipiler needs to check the


Typo: comipiler

frewsxcv · 2017-04-02T21:30:32Z

src/lifetimes.md

+
+Again, at the call-site of `shortest` the comipiler needs to check the
+consistency of the arguments in the caller with the signature of the callee.
+The signature of `shortest` fisrt says that the two reference arguments have


Typo: fisrt

steveklabnik

some nits

steveklabnik · 2017-04-04T14:21:37Z

src/lifetimes.md

+}
+```
+
+`print_shortest` simply prints the shorter of its two pass-by-reference


"simply" should be taken out

steveklabnik · 2017-04-04T14:22:14Z

src/lifetimes.md

+s1` with `&'s2 s2`. After this both arguments are of lifetime `&'s2` and the
+call-site is consistent with the signature of `print_shortest`.
+
+[More formally, the basis for the above rule is in *type variance*. Under this


this would be all in >s, not [].

steveklabnik · 2017-04-04T14:22:28Z

src/lifetimes.md

+variance isn't strictly required to understand the Rust borrow checker. We've
+tried here to instead to explain using intuitive terminlolgy.]
+
+# Inter-procedural Borrow Checking of Function Return Values


this should be a smaller heading than #

vext01 · 2017-04-05T13:44:25Z

Thanks everyone for the comments.

Formatting aside, is what I have written correct? Notice how, when de-sugaring, I initialise references with the lifetime of the referent, whereas the previous examples do something slightly different, often using a narrower lifetime.

Of course, a narrower lifetime can be fine (and probably beneficial, if it can be shown valid), but I wonder if that can be considered a separate "optimisation" that we don't mention here. I think it's important that we have a single intuitive narrative for explaining this to new users. I can't understate this. Lifetimes are what make Rust unique, but are also very hard to grasp (I'm still grasping myself to be honest).

So assuming my de-sugaring is correct (I hope someone checked, because I'm really very new to this), should I go back and revise the earlier examples to fit this style too?

Thanks

vext01 · 2017-05-06T10:54:57Z

Anyone? I was hoping this kind of PR would be prioritised, since the borrow checker is the one thing that no newcomer understands for a long time.

Mark-Simulacrum · 2017-05-06T11:49:54Z

Sorry about that! We'll try to get you a review soon.

cc @steveklabnik @frewsxcv

steveklabnik · 2017-05-06T14:17:11Z

Yes, sorry; I've been traveling _and_ have not yet quite worked out how lang + docs should collaborate on the reference; dealing with all of this is one of my goals for the upcoming week.

…

On Sat, May 6, 2017 at 7:49 AM, Mark Simulacrum ***@***.***> wrote: Sorry about that! We'll try to get you a review soon. cc @steveklabnik <https://github.com/steveklabnik> @frewsxcv <https://github.com/frewsxcv> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABsirm1jYgokpbV6RvwxMuYyCYzv7HHks5r3F5igaJpZM4MfiS5> .

steveklabnik · 2017-05-15T17:42:21Z

Hey @vext01;

Sorry again about the extreme delay here; it has nothing to do with you and everything to do with me.

so, i was talking with @nikomatsakis and/or @aturon about how we can work through things like this; I think the plan will be to eventually go through everything with them, rather than looking at individual PRs. So, I don't think we need actual lang team signoff here.

I think that this is all correct, but it's not my area of specialty. It's unclear to me who should actually sign off on it for that reason :/ Maybe @gankro himself?

Gankra · 2017-05-16T17:10:48Z

So I'm a bit concerned that there seems to be some redundancy with other sections. For instance https://doc.rust-lang.org/nomicon/subtyping.html discusses this merging behaviour a bit already (though it comes later).

I need to re-read this chapter to see what gets discussed where.

Gankra · 2017-05-16T16:58:49Z

src/lifetimes.md

+            let s2 = String::from("a long long long string");
+            print_shortest(&s1, &s2);
+        }
+}


Missing }

Gankra · 2017-05-16T16:59:04Z

src/lifetimes.md

+            let s2 = String::from("a long long long string");
+            print_shortest(&'s1 s1, &'s2 s2);
+        }
+}


Missing }

Gankra · 2017-05-16T17:06:57Z

src/lifetimes.md

+*caller* (`main`) are consistent with the lifetimes in the signature of the
+*callee* (`print_shortest`).
+
+The signature of `print_shortest` simply requires that both of it's arguments


Kill all "simply"s

vext01 · 2017-05-17T09:49:19Z

I've fixed the comments on my new prose.

Outstanding topics we need to discuss (quoting my earlier comments):

Notice how, when de-sugaring, I initialise references with the lifetime of the referent, whereas the previous examples do something slightly different, often using a narrower lifetime.

There's also the issue of (I think) move semantics giving rise to additional gotchas (since they limit the lifetimes of values), but I think I'd like to tackle that as a separate PR.

We might decide to do these in separate PRs, as this PR will start to bitrot.

I'm glad the rust team are starting to think about how to deal with docs. I can't emphasize enough the importance of documenting rust, since it has radical new ideas which even lifelong programmers (including myself) are struggling to put to work. You need to make it as easy as possible to learn rust, and documentation is all you've got! :)

If you keep me in the loop, I am happy to review/comment of PRs. I hope I can offer the "newbie's-eye-view".

vext01 · 2017-05-17T10:05:30Z

Fixed build.

steveklabnik · 2017-05-26T17:01:53Z

@gankro when you're happy with this, I think I'm happy with this 😄

Rufflewind · 2017-05-27T02:16:59Z

src/lifetimes.md

+(`s2`).
+
+> Formally, function return values are said to be *contravariant*, the opposite
+> of *covariant*.


I feel the argument/return position might be a bit of a red herring here.

I wouldn’t use the term contravariant to refer to return types, at least not in this sense. They appear so because a return type was compared to an argument type, but if you compared a return type to another return type they would appear covariant just as well!

The lifetime coercion (subtyping) rule is simply &'a T -> &'b T provided that 'a outlives 'b. Argument/return type doesn’t play a role here.* The compiler is allowed to shrink the lifetime of a reference, but never extend it. Formally speaking, reference types are said to be “lifetime-covariant”.

Another rule is that an object must always outlive its binding. This means you can’t, in a variable of a larger scope, store a reference to an object of a smaller scope:

'r_scope { let r: &'r_ref; 'inner { let inner = String::new(); r = &'r_ref inner; // doesn't work because 'r_ref ≥ 'r_scope > 'inner // yet 'r_ref ≤ 'inner } }

From the first example, you learned that 'k has to be 's2 (or shorter). This remains true for the second example.

In the second example, the returned reference has lifetime 'k. When it gets stored in res, the compiler may shrink 'k if necessary. Therefore the lifetime 'res_ref of the stored reference must be 'k or shorter. Here, I used 'res_ref to distinguish between the lifetime of the reference res: &'res_ref str, and the lifetime of the scope itself 'res_scope { let res; ... }.

From the structure of the code itself, we know that the scope of s2 is contained fully within that of res. Because an object must always outlive its binding, it follows that 'res_ref must outlive 'res_scope, which is strictly longer than 's2.

If we combine these three facts together,

'k must be 's2 or shorter

'res_ref must be 'k or shorter

'res_ref must be strictly longer than 's2

we see that there can be no solution that satisfies all three constraints :(

There is a way to fix it though, by making 's2 and 'res_scope equal:

fn main() { let s1 = String::from("short"); let (res, s2); s2 = String::from("a long long long string"); res = shortest(&s1, &s2); println!("{}", res); }

--

* Due to variance, the rule changes for more complicated for composite types like Cell<&'a ()> or fn(&'a ()) -> &'b () but that doesn't apply here.

I wouldn’t use the term contravariant to refer to return types, at least not in this sense. They appear so because a return type was compared to an argument type, but if you compared a return type to another return type they would appear covariant just as well!

I'm not sure I understand what you mean. Can you expand on this?

Compare these two statements:

Rule I:

A function argument of type &'p T can be coerced with an
argument of type &'q T if the lifetime of &'p T is equal or longer than &'q T.

Rule II:

The return value of a function &'r T can be converted to an argument &'s T if the lifetime of &'r T is equal or shorter than &'s T.

Rule I compares arguments vs argument. Rule II is return value vs argument. If you compare things in opposite positions, you will always the opposite result, so Rule II is just a corollary to Rule I.

Moreover, I don't think "co-/contravariant" is the appropriate term for this. Formally, we say F is covariant in its parameter if and only if A ⊆ B implies F<A> ⊆ F<B>, and contravariant if and only if A ⊆ B implies F<A> ⊇ F<B>. There is no mention of argument vs return types at all.

I agree that the language "The return value of a function &'r T can be converted to an argument" feels a bit odd.

However, I personally think that the type co/contra-variance is a useful thing to point out to people a) because it's surprisingly unintuitive (indeed, there are real programming languages that have got this wrong in their type systems) b) because that can be used as a hook to explain the relationship to lifetimes. However, one doesn't necessarily need to use those words provided one gets the concept across somehow. What I think the document is trying to say is "if a function takes a parameter of type T and returns a type U, you can pass a (non-strict) supertype T' and receive back a (non-strict) supertype U' " and then making the link between that and the length of lifetimes.

Did you mean "if a function takes T and returns U, you can pass a subtype T' and supertype U'"?

Quite correct -- mea culpa!

Here's my concrete suggestion for this PR:

Rewrite Rule I (L289-290) to say something along the lines of "A reference can always be shrunk to one of a shorter lifetime. In other words, &'a T can be implicitly converted to &'b T as long as 'a outlives 'b."

Replace "for function arguments to be co-variant" (L299) with "a reference &'a T is always covariant in its lifetime parameter 'a."

Replace Rule II and adjacent paragraphs (L348-372) with something like:

Again, at the call-site of shortest the compiler needs to check the consistency of the arguments in the caller with the signature of the callee. The signature of shortest says that all three references must have the same lifetime 'k, so we have to find a lifetime 'k such that:

&s1: &'s1 str can be converted to the first argument &'k str,

&s2: &'s2 str can be converted to the second argument &'k str, and

the return value &'k str can be converted to res: &'res str

res: &'res = shortest::<'k>(&'s1 s1, &'s2 s2); // ^^^^^^^ // 's1 -> 'k ^^^^^^^ // 's2 -> 'k // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ // 'k -> 'res

This leads to three requirements:

's1 must outlive 'k,

's2 must outlive 'k, and

'k must outlive 'res.

However, 'res strictly outlives 's2 by virtue of being a larger scope that encloses s2, so it's impossible to satisfy these conditions. The borrow checker rightfully rejects our program because we are making a reference (res) which outlives one of the values it may refer to (s2)."

OK. I'll make these edits (and see if it's understandable to a beginner) when i find some time.

Thanks for your comments Rufflewind.

@vext01 any chance you can find the time sometime soon?

I'll try to find time this week.

I've learnt more Rust since I last looked at this, so I may have some edits of my own.

vext01 · 2017-11-27T17:30:20Z

I've implemented Rufflewind's recommended changes, and read the whole thing over.

I shortened a fair amount of my prose, as I felt I had been rambling a bit.

Let me know what you think.

vext01 · 2017-11-27T17:32:45Z

btw, I killed the stuff on variance, as I felt it was confusing and vague.

Rufflewind · 2017-11-28T04:18:50Z

I think the revised version look great!

vext01 · 2017-11-28T09:52:35Z

Shall I squash?

vext01 · 2017-12-05T14:53:52Z

ping

vext01 · 2018-01-05T12:02:23Z

I think someone should make it their new year's resolution to merge this :)

QuietMisdreavus · 2018-01-15T17:01:41Z

src/lifetimes.md

+
+> A reference can always be shrunk to one of a shorter lifetime. In other
+> words, `&'a T` can be implicitly converted to `&'b T` as long as `'a`
+outlives `'b.`


Should this line have a > on it too? (Github's rendering adds it to the blockquote just fine, but i'd rather have it all neat. >_>)

Gankra

So first off: I apologize greatly for having kicked this ostensibly simple review down the road for so long. I've been a mess for the last year and this is one of the things I avoided as a result.

Ultimately I've avoided this for a few reasons:

I just don't understand the motivation for this change; or at least it doesn't feel like the nomicon is the right place for this.
It's extending the scoped lifetime illustration which has never been great
All discussions of lifetimes are a mess as a result of non-lexical lifetimes, and this entire section probably needs to be rewritten to focus on the new system (and this addition just exacerbates the situation, I think)

As such I'm inclined to cut our losses and just close this. It absolutely sucks that you have been strung along this long by my negligence, but it happened. I'm sorry.

Gankra · 2018-01-15T21:33:02Z

src/lifetimes.md

@@ -1,6 +1,6 @@
 # Lifetimes

-Rust enforces these rules through *lifetimes*. Lifetimes are effectively
+Rust ensures memory safety through *lifetimes*. Lifetimes are effectively


This feels too strong, (lifetimes are a single piece of the puzzle that comes together for mem safety), but this is probably pedantry.

Gankra · 2018-01-15T21:33:59Z

src/lifetimes.md

@@ -213,3 +213,142 @@ totally ok*, because it keeps us from spending all day explaining our program
 to the compiler. However it does mean that several programs that are totally
 correct with respect to Rust's *true* semantics are rejected because lifetimes
 are too dumb.
+
+# Inter-procedural Borrow Checking


I've never seen this hyphenated; drop it.

This is also maybe a Bad Title, because borrowchecking is specifically not interprocedural.

Perhaps it's the wrong term, but I was referring to the way that the arguments at call-sites must be checked against a function definition, so in some way lifetimes do cross the function boundary. If you see what I mean?

Gankra · 2018-01-15T22:30:30Z

src/lifetimes.md

+```
+
+(for brevity, we don't show the third implicit scope that would be introduced
+to limit to lifetimes of the borrows in the call to `print_shortest`)


"the" lifetime.

Regardless, It seems like this can either be cut, or is Actually Very Important?

Gankra · 2018-01-15T22:49:39Z

src/lifetimes.md

+they refer to were introduced in different scopes. At the call site of
+`print_shortest` the compiler must now check that the lifetimes in the
+*caller* (`main`) are consistent with the lifetimes in the signature of the
+*callee* (`print_shortest`).


This is not how the lifetime system works. It's a constraint solver; they never have "wrong" lifetimes, and the lifetimes aren't checked if they're "right". It just tries to solve the system of constraints and either reports success or failure.

vext01 · 2018-01-17T10:27:24Z

Hi @gankro!

Thanks for looking at my PR.

I just don't understand the motivation for this change; or at least it doesn't feel like the nomicon is the right place for this.

I started working on this because I didn't understand how lifetimes work behind the scenes. Although I've gotten better at Rust since then, there are still times where I "brute force" my way past cryptic lifetime errors offered up by the compiler. And it's not just me, I occasionally see my friends and colleagues (experienced programmers by the way) struggling in the same way.

I think this is indicative of a problem with the mental model which non-expert Rust programmers have when writing Rust code. I also think that we need to find an intuitive way to describe what is really going on here, because guessing how to fix these weird errors just doesn't satisfy my curiosity.

It's extending the scoped lifetime illustration which has never been great.

Yes agreed.

All discussions of lifetimes are a mess as a result of non-lexical lifetimes, and this entire section probably needs to be rewritten to focus on the new system (and this addition just exacerbates the situation, I think)

Yeah, I've seen the RFCs about non-lexical lifetimes.

I apologize greatly for having kicked this ostensibly simple review down the road for so long.
...
It absolutely sucks that you have been strung along this long by my negligence, but it happened. I'm sorry.
...
As such I'm inclined to cut our losses and just close this.

Don't sweat it! I completely understand. Sometimes these things don't work out. And in this instance I'd rather we get it right than cling on pretending that my precious code is helping :)

Before I close this, can I recommend that we make an issue on this (or another) repo that basically says "find a more intuitive way to explain lifetimes to users". If you let me know which repo, I will happily raise the issue.

You might consider describing the constraint solver in this repo though? I am happy to help reviewing if you can write the prose. [I have experience with linear constraint solving and SAT].

Thanks 😄

vext01 · 2018-02-09T10:14:18Z

Ping?

vext01 · 2018-03-15T10:01:23Z

I'm going to close this as there seems to be a lack of interest/time.

vext01 added 6 commits March 15, 2017 16:27

First attempt at extending the lifetimes section.

4bf5fa6

Tweaks.

4b794ca

MOre tweaks, shorten.

0a6ad61

More shortening.

005053e

Try to explain via de-sugraing, as is the style of the nomicon.

87b4ca4

Tweaks, but somethign is wrong.

c5c93f2

vext01 added 3 commits March 17, 2017 12:05

Try again.

7b93ec8

Drop parentheses when referring to functions inline. Fix a typo.

37fe162

Small fixes.

09274f3

vext01 changed the title ~~Help Needed: Extend lifetimes section~~ Overhaul Lifetimes Section (was "Help Needed: Extend lifetimes section") Mar 17, 2017

frewsxcv reviewed Apr 2, 2017

View reviewed changes

steveklabnik reviewed Apr 4, 2017

View reviewed changes

Gankra reviewed May 16, 2017

View reviewed changes

Fix outstanding comments from the Rust devs.

f6b3f2a

Re-add some ignores, fix one code example.

48f7e91

Rufflewind reviewed May 27, 2017

View reviewed changes

vext01 added 2 commits November 27, 2017 17:26

Merge branch 'master' into extend_lifetimes_section

c1833cc

Address comments and small improvements.

e4c2e8b

QuietMisdreavus reviewed Jan 15, 2018

View reviewed changes

Gankra reviewed Jan 15, 2018

View reviewed changes

vext01 closed this Mar 15, 2018

Overhaul Lifetimes Section (was "Help Needed: Extend lifetimes section") #4

Overhaul Lifetimes Section (was "Help Needed: Extend lifetimes section") #4

Uh oh!

Conversation

vext01 commented Mar 16, 2017

Uh oh!

vext01 commented Mar 16, 2017

Uh oh!

steveklabnik commented Mar 16, 2017

Uh oh!

vext01 commented Mar 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vext01 commented Mar 17, 2017

Uh oh!

vext01 commented Mar 17, 2017

Uh oh!

steveklabnik commented Mar 17, 2017

Uh oh!

vext01 commented Mar 17, 2017

Uh oh!

vext01 commented Mar 27, 2017

Uh oh!

steveklabnik commented Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aturon commented Mar 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steveklabnik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vext01 commented Apr 5, 2017

Uh oh!

vext01 commented May 6, 2017

Uh oh!

Mark-Simulacrum commented May 6, 2017

Uh oh!

steveklabnik commented May 6, 2017 via email

Uh oh!

steveklabnik commented May 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gankra commented May 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vext01 commented May 17, 2017

Uh oh!

vext01 commented May 17, 2017

Uh oh!

steveklabnik commented May 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vext01 commented Mar 16, 2017 •

edited

Loading

steveklabnik commented Mar 27, 2017 •

edited

Loading

steveklabnik commented May 15, 2017 •

edited

Loading