Skip to content

dropck: The drop order is now defined #113

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

Merged
merged 1 commit into from
Mar 25, 2019
Merged

dropck: The drop order is now defined #113

merged 1 commit into from
Mar 25, 2019

Conversation

vorner
Copy link
Contributor

@vorner vorner commented Dec 16, 2018

So the examples are out of date, they no longer pose a problem. However,
there's still a case where it does, so let's use that instead.

Also, mention the definition of drop order in the text, as it is
related.

@Gankra
Copy link
Contributor

Gankra commented Feb 26, 2019

Could you explain the motivation for rewriting all the examples? As far as I know nothing has been done that changes their correctness, and I don't think your changes at all make things more clear.

@vorner
Copy link
Contributor Author

vorner commented Feb 27, 2019

It's been a while, so I have to guess a bit. However, the problem with the examples I had is, they still don't compile as they are supposed not to compile. But the reason they do not compile is different and the reason stated on the page is outdated. It's not because one does not outlive the other, but because the order of drop is reversed. If you switch the order in the tuple/pattern (eg. use (days, inspector) instead of (inspector, days)) it now starts compiling. So the example is not relevant to the chapter at all any more.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=117cf19b8585f2c86f6f530b41a72fc9

I tried to modify the inspector example, but didn't come up with a way. So I created new ones that demonstrate the issue. If you have an idea how to modify the example in a way that doesn't compile in either order of the tuple, I'm all for it.

@Gankra
Copy link
Contributor

Gankra commented Mar 6, 2019

Could you not just change the examples to let tup = (a, b) and using tup.0 and tup.1 (or perhaps introduce a struct just for better naming?)

@vorner
Copy link
Contributor Author

vorner commented Mar 7, 2019

Could you not just change the examples to let tup = (a, b) and using tup.0 and tup.1 (or perhaps introduce a struct just for better naming?)

OK, I'll try playing with it.

@vorner
Copy link
Contributor Author

vorner commented Mar 11, 2019

Just note for myself when I get to it again: the tuple seems to do the trick ‒ this compiles in both orders of the tuple: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=7573478e37c067e48a1e728c499773ea

I'll try to return the examples to (close to) the original once I have a bit more time. Thanks for the suggestion.

@Gankra
Copy link
Contributor

Gankra commented Mar 11, 2019

Just for clarity (named fields) the struct would probably be better, but ok, great!

@vorner
Copy link
Contributor Author

vorner commented Mar 24, 2019

I've returned to to much closer state to the original. It's now wrapped in a structure, to make the borrow checker not being able to track the lifetimes as separate and added references to the defined drop order of fields.

I've force-pushed into the branch, as the diff is shorter to master than to the previous version.

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.

Ok this looks great, thanks!

So the examples are out of date, they no longer pose a problem. However,
there's still a case where it does, so let's use that instead.

Also, mention the definition of drop order in the text, as it is
related.
@Gankra
Copy link
Contributor

Gankra commented Mar 25, 2019

thank you!!!!

@Gankra Gankra merged commit c02e0e7 into rust-lang:master Mar 25, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 6, 2019
Update books

## nomicon

1 commits in f1ff93b66844493a7b03101c7df66ac958c62418..c02e0e7754a76886e55b976a3a4fac20100cd35d
2019-02-26 13:37:28 -0500 to 2019-03-25 16:52:56 -0400
- dropck: The drop order is now defined (rust-lang/nomicon#113)

## reference

3 commits in 27ad493..98f90ff
2019-03-26 02:06:15 +0100 to 2019-04-06 09:29:08 -0700
- Document repr packed(N). (rust-lang/reference#553)
- Fix broken link in glossary. (rust-lang/reference#558)
- Typo fixes (rust-lang/reference#556)

## embedded-book

2 commits in 07fd3880ea0874d82b1d9ed30ad3427ec98b4e8a..7989c723607ef5b13b57208022259e6c771e11d0
2019-03-27 15:40:52 +0000 to 2019-04-04 12:14:37 +0000
- fix rust-embedded/book#182  (rust-embedded/book#183)
- Add openocd to list of installable packages  (rust-embedded/book#179)
Centril added a commit to Centril/rust that referenced this pull request Apr 6, 2019
Update books

## nomicon

1 commits in f1ff93b66844493a7b03101c7df66ac958c62418..c02e0e7754a76886e55b976a3a4fac20100cd35d
2019-02-26 13:37:28 -0500 to 2019-03-25 16:52:56 -0400
- dropck: The drop order is now defined (rust-lang/nomicon#113)

## reference

3 commits in 27ad493..98f90ff
2019-03-26 02:06:15 +0100 to 2019-04-06 09:29:08 -0700
- Document repr packed(N). (rust-lang/reference#553)
- Fix broken link in glossary. (rust-lang/reference#558)
- Typo fixes (rust-lang/reference#556)

## embedded-book

2 commits in 07fd3880ea0874d82b1d9ed30ad3427ec98b4e8a..7989c723607ef5b13b57208022259e6c771e11d0
2019-03-27 15:40:52 +0000 to 2019-04-04 12:14:37 +0000
- fix rust-embedded/book#182  (rust-embedded/book#183)
- Add openocd to list of installable packages  (rust-embedded/book#179)
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.

3 participants