Skip to content
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

Lifetimes: Updates to incorporate NLL #101

Merged
merged 1 commit into from Apr 25, 2019
Merged

Lifetimes: Updates to incorporate NLL #101

merged 1 commit into from Apr 25, 2019

Conversation

vorner
Copy link
Contributor

@vorner vorner commented Nov 4, 2018

  • Updated the explanations around lifetimes a bit.
  • Made sure the examples that should fail still fail in edition 2018.
  • Prefer rust,compile_fail instead of rust,ignore ‒ the latter
    allows the user to click on button and see the actual compile errors.
    Also, this'll tell us if something stops failing.

@RalfJung
Copy link
Member

RalfJung commented Nov 5, 2018

Prefer rust,compile_fail instead of rust,ignore ‒ the latter
allows the user to click on button and see the actual compile errors.
Also, this'll tell us if something stops failing.

You mean "the former"?

src/lifetimes.md Outdated Show resolved Hide resolved
@vorner
Copy link
Contributor Author

vorner commented Nov 8, 2018

You mean "the former"?

Hmm, yes. I'll rewrite that on the before-merge rebase.

@Gankra
Copy link
Contributor

Gankra commented Nov 11, 2018

note I intend to review this soon but I want to touch base with the lang team to understand NLL better before going over this in detail

@vorner
Copy link
Contributor Author

vorner commented Mar 7, 2019

Hello. Any progress here?

@vorner
Copy link
Contributor Author

vorner commented Apr 7, 2019

Hello

@gankro Is there a realistic chance you'd get around to it, or should we try to seek some other way of validating it that doesn't block on you? It's sitting here without progress for 5 months and the book contains outdated information that is no longer true :-(.

Maybe asking someone from the NLL implementers to skim the text for blatant factual lies and and you having a look if the text fits nicely with the rest of the book and is understandable would work?

@Gankra
Copy link
Contributor

Gankra commented Apr 13, 2019

Yeah reviewing this has been my white whale. A true disaster I have created and inflicted upon you. :/

Part of the issue is that this substantial of a change requires actual, like, editing for effective communication, which I find very uncomfortable to do? I would definitely appreciate technical feedback from domain experts here, but this also needs the attention of like, a Writing Professional.

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.

r=me with minor nits

src/lifetime-mismatch.md Outdated Show resolved Hide resolved
src/lifetimes.md Show resolved Hide resolved
src/lifetimes.md Outdated Show resolved Hide resolved
src/lifetimes.md Show resolved Hide resolved
@vorner
Copy link
Contributor Author

vorner commented Apr 21, 2019

I've applied the suggestions into a new fixup. If it looks good, I'll squash the fixups to have nicer history.

I'm afraid this is based on old version of master that no longer compiles all the snippets (in other chapters). I don't want to mess the review with a full rebase yet, though, so I'm postponing the rebase to current master for when the changes here are ACKed. Is that OK?

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.

Thanks for not squashing! A couple more nits.

Feel free to squash + rebase everything this time, assuming there's not anything surprising.

src/lifetime-mismatch.md Outdated Show resolved Hide resolved
src/lifetimes.md Outdated Show resolved Hide resolved
* Updated the explanations around lifetimes a bit.
* Made sure the examples that should fail still fail in edition 2018.
* Prefer `rust,compile_fail` instead of `rust,ignore` ‒ the latter
  allows the user to click on button and see the actual compile errors.
  Also, this'll tell us if something stops failing.
@Gankra
Copy link
Contributor

Gankra commented Apr 25, 2019

ready to land?

@vorner
Copy link
Contributor Author

vorner commented Apr 25, 2019

Yes :-).

@Gankra Gankra merged commit c656171 into rust-lang:master Apr 25, 2019
Manishearth added a commit to Manishearth/rust that referenced this pull request May 17, 2019
Update books

## nomicon

1 commits in fb29b147be4d9a1f8e24aba753a7e1de537abf61..c656171b749b7307f21371dd0d3278efee5573b8
2019-04-22 19:10:29 -0400 to 2019-04-25 15:31:26 -0400
- Lifetimes: Updates to incorporate NLL (rust-lang/nomicon#101)

## reference

5 commits in 2a2de9c..862b669
2019-04-22 10:25:52 -0700 to 2019-05-04 23:41:35 -0700
- Typo (rust-lang/reference#606)
- Added missing ? to Generics from InherentImpl and TraitImpl (rust-lang/reference#604)
- Add missing ( to ExternalFunctionItem (rust-lang/reference#603)
- Remove unneeded | from AssignmentExpression (rust-lang/reference#601)
- Remove unneeded ( from TypePathSegment (rust-lang/reference#602)

## book

6 commits in db919bc6bb9071566e9c4f05053672133eaac33e..29fe982990e43b9367be0ff47abc82fb2123fd03
2019-04-15 20:11:03 -0400 to 2019-05-15 17:48:40 -0400
- Ignore a non-compiling test listing and add code to fix a test listing
- Remove nostarch snapshots I've checked in layout
- Reword error messages to maybe not need to wrap lines in print
- This example doesn't compile but wasn't marked as such
- Update install instructions for VS 2019 (rust-lang/book#1923)
- Switch IRC to Discord

## rust-by-example

9 commits in 1ff0f8e018838a710ebc0cc1a7bf74ebe73ad9f1..811c697b232c611ed754d279ed20643a0c4096f6
2019-04-15 08:15:32 -0300 to 2019-04-28 18:56:42 -0300
- Fix typo in dsl.md (rust-lang/rust-by-example#1187)
- File read lines (rust-lang/rust-by-example#1186)
- For rust-lang/rust-by-example#1184 closes rust-lang/rust-by-example#1184 (rust-lang/rust-by-example#1185)
- Link to Reference for macro_rules designators (rust-lang/rust-by-example#1182)
- Improve section Meta/Docs (rust-lang/rust-by-example#1183)
- Small improvements to various files (rust-lang/rust-by-example#1173)
- 19.2 Vectors Error in Code Example (rust-lang/rust-by-example#1178)
- For rust-lang/rust-by-example#1175 (rust-lang/rust-by-example#1176)
- For rust-lang/rust-by-example#1179 (rust-lang/rust-by-example#1180)

## rustc-guide

12 commits in 99e1b1d53656be08654df399fc200584aebb50e4..3cb727b62b953d59b4360d39aa68b6dc8f157655
2019-04-20 09:57:54 -0500 to 2019-05-07 09:53:32 -0500
- Fix typo, 'which' repeated twice
- [canonicalization] fix result canonicalization example (rust-lang/rustc-dev-guide#304)
- Rename to RUSTC_LOG
- Added mention of universal ctags
- Fix link in walkthrough
- Remove IRC from discussion chats
- Bring the updating LLVM guide up to date
- use nightly rust for ci
- Fixed broken chalk links
- Add documentation for two-phase borrows
- Explain new powers of the `treat-err-as-bug` flag
- Update lowering-module test case

## edition-guide

3 commits in c413d42a207bd082f801ec0137c31b71e4bfed4c..581c6cccfaf995394ea9dcac362dc8e731c18558
2019-04-22 01:14:56 +0200 to 2019-05-06 12:47:44 -0700
- Fix typo in controlling-panics-with-std-panic.md (rust-lang/edition-guide#158)
- Fix links for book editions. (rust-lang/edition-guide#149)
- Update now that NLL is enabled in 2015. (rust-lang/edition-guide#157)

## embedded-book

3 commits in de3d55f521e657863df45260ebbca1b10527f662..9858872bd1b7dbba5ec27dc30d34eba00acd7ef9
2019-04-22 12:58:28 +0000 to 2019-05-02 18:56:54 +0000
- Update linux.md  (rust-embedded/book#167)
- Clarify list of available targets for installation  (rust-embedded/book#165)
- minor grammar fix  (rust-embedded/book#188)
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.

None yet

3 participants