-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Update E0716.md for clarity #120684
Update E0716.md for clarity #120684
Conversation
When reading through this, I got slightly hung up thinking the `let` it was referring to was the `let tmp` on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nnethercote (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
above by showing that `tmp` would be freed as we exit the block. | ||
statement -- in this case, after the outer `let` that assigns to `p`. This is | ||
illustrated in the example above by showing that `tmp` would be freed as we exit | ||
the block. |
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.
There are four let
s in this file, two in the first example and two in the second example. I think "after the let
" refers to the first let in the first example, but you've updated this as if it's referring to the first let in the second example? Also, you've said "outer let
" but there isn't an inner let
.
I suggest just changing "after the let
" to after the
let p` in the first example above". What do you think?
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.
So if you count the let tmp = foo();
on line 25 as one then there are five let
s, and this one is the "inner let
" that I was implying existed, and this is the let
that I initially thought the line in question was referring to, but either way, I agree that adding in the "outer" wording is confusing since the second example is really just used to explain what's going on in the first example. I think just making it read:
"after the let p
statement"
would be sufficient because then you can refer back to either of the two examples and it's valid.
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.
👍
@carschandler I think this is waiting for you to make the small change suggested above. Is that right? |
Clearer wording
@nnethercote thank you for the mention! I've made the change now, for a net total of....... one word added! |
Thanks! @bors r+ rollup |
Update E0716.md for clarity When reading through this, I got slightly hung up thinking the `let` it was referring to was the `let tmp` on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose.
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#109263 (fix typo in documentation for std::fs::Permissions) - rust-lang#120684 (Update E0716.md for clarity) - rust-lang#121739 (Display short types for unimplemented trait) - rust-lang#121749 (Don't lint on executable crates with `non_snake_case` names) - rust-lang#121758 (Move thread local implementation to `sys`) - rust-lang#121815 (Move `gather_comments`.) - rust-lang#121835 (Move `HandleStore` into `server.rs`.) - rust-lang#121847 (Remove hidden use of Global) - rust-lang#121861 (Use the guaranteed precision of a couple of float functions in docs) - rust-lang#121875 ( Account for unmet T: !Copy in E0277 message) r? `@ghost` `@rustbot` modify labels: rollup
Update E0716.md for clarity When reading through this, I got slightly hung up thinking the `let` it was referring to was the `let tmp` on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose.
Rollup of 9 pull requests Successful merges: - rust-lang#109263 (fix typo in documentation for std::fs::Permissions) - rust-lang#117156 (Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits) - rust-lang#120684 (Update E0716.md for clarity) - rust-lang#121739 (Display short types for unimplemented trait) - rust-lang#121815 (Move `gather_comments`.) - rust-lang#121835 (Move `HandleStore` into `server.rs`.) - rust-lang#121847 (Remove hidden use of Global) - rust-lang#121861 (Use the guaranteed precision of a couple of float functions in docs) - rust-lang#121875 ( Account for unmet T: !Copy in E0277 message) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#109263 (fix typo in documentation for std::fs::Permissions) - rust-lang#120684 (Update E0716.md for clarity) - rust-lang#121715 (match lowering: pre-simplify or-patterns too) - rust-lang#121739 (Display short types for unimplemented trait) - rust-lang#121815 (Move `gather_comments`.) - rust-lang#121835 (Move `HandleStore` into `server.rs`.) - rust-lang#121847 (Remove hidden use of Global) - rust-lang#121861 (Use the guaranteed precision of a couple of float functions in docs) - rust-lang#121875 ( Account for unmet T: !Copy in E0277 message) r? `@ghost` `@rustbot` modify labels: rollup
Update E0716.md for clarity When reading through this, I got slightly hung up thinking the `let` it was referring to was the `let tmp` on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose.
Rollup merge of rust-lang#120684 - carschandler:patch-1, r=nnethercote Update E0716.md for clarity When reading through this, I got slightly hung up thinking the `let` it was referring to was the `let tmp` on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose.
When reading through this, I got slightly hung up thinking the
let
it was referring to was thelet tmp
on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose.