NLL: temps in block tail expression diagnostic #54782
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
pnkfelix
commented
Oct 3, 2018
src/librustc_mir/build/block.rs
Outdated
| @@ -91,8 +91,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { | |||
| let source_info = this.source_info(span); | |||
| for stmt in stmts { | |||
| let Stmt { kind, opt_destruction_scope } = this.hir.mirror(stmt); | |||
| let kind2 = kind.clone(); | |||
There was a problem hiding this comment.
oops this binding of kind2 (and associated clone) wasn't supposed to be part of the PR. Will fix.
pnkfelix
force-pushed
the
issue-54556-semi-on-tail-diagnostic
branch
from
57b9ad5 to
eb37f5f
Compare
pnkfelix
changed the title
pnkfelix
added
A-NLL
|
I just realized that I included the updates to the existing tests but forgot to include the unit tests I made specifically for this. I'll add a commit with them onto the end of this after I sanity-check their output (very soon). |
nikomatsakis
approved these changes
Oct 3, 2018
| /// expression is ignored by the block's expression context. | ||
| /// | ||
| /// Examples include `{ ...; tail };` and `let _ = { ...; tail };` | ||
| /// but not e.g. `let _x = { ...; tail };` |
There was a problem hiding this comment.
Might be nice to explain briefly what this is used for
|
@bors r+ So from a technical perspective, I feel good about this. I'm not sure 100% about the wording but it seems like a solid improvement from what we had. Perhaps we can iterate after landing. |
|
📌 Commit 3b17f1bfe38f6257ea12c7d5e5d236ffcf70615a has been approved by |
bors
added
S-waiting-on-bors
|
☔ The latest upstream changes (presumably #54666) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
…bexpr a block tail expression. Slightly refactored the `LocalDecl` construction API in the process.
…lanation_to_diagnostic`. (I found it confusing to have calls to an `emit` method in our error_reporting module where that `emit` method *wasn't* the `DiagnosticBuffer::emit` method.)
This is preparation for allowing the `BorrowExplanation` carry things like `mir::Location` or `mir::Local` and still be able to generate usable diagnostic text.
… temporary r-values. Changed `BorrowExplanation::UsedLaterWhenDropped` to handle both named locals and also unnamed (aka temporaries). If the dropped temporary does not implement `Drop`, then we print its full type; but when the dropped temporary is itself an ADT `D` that implements `Drop`, then diagnostic points the user directly at `D`.
pnkfelix
force-pushed
the
issue-54556-semi-on-tail-diagnostic
branch
from
3b17f1b to
056cfff
Compare
|
@bors r=nikomatsakis |
|
📌 Commit 704877f has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 704877f with merge ec3d08835a74038c0f549035b37d8b7ec6f42a99... |
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors retry |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Oct 7, 2018
…r=nikomatsakis NLL: temps in block tail expression diagnostic This change adds a diagnostic that explains when temporaries in a block tail expression live longer than block local variables that they borrow, and attempts to suggest turning the tail expresion into a statement (either by adding a semicolon at the end, when its result value is clearly unused, or by introducing a `let`-binding for the result value and then returning that). Fix #54556
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds a diagnostic that explains when temporaries in a block tail expression live longer than block local variables that they borrow, and attempts to suggest turning the tail expresion into a statement (either by adding a semicolon at the end, when its result value is clearly unused, or by introducing a
let-binding for the result value and then returning that).Fix #54556