-
Notifications
You must be signed in to change notification settings - Fork 13k
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
add "temporary value borrowed for too long" error #54164
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #54088) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
Few nits
src/librustc_mir/diagnostics.rs
Outdated
@@ -2187,6 +2187,53 @@ fn main() { | |||
``` | |||
"##, | |||
|
|||
E0713: r##" | |||
When you write `&expr` and `expr` is not a "path" rooted in some |
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, there is actually a standard template for these errors we ought to follow.
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.
I took a stab at a conformant write-up:
https://gist.github.com/nikomatsakis/10349c6242aad639b2e89eada6fffb7a
(you can get the raw text from there)
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.
Make sure to check over the examples in that gist; don't just cut-and-paste their content blindly. In particular, the fn bar
's, starting on line 8, probably should be returning a &i32
, not a i32
.
(The code as written doesn't compile because it tries to dereference an i32
, but that's not the error we are trying to illustrate here.)
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.
Note that, unless the example is tagged with ignore, we actually test this.
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.
(True; not everyone runs the full test suite locally, but the error would have been caught.)
self, | ||
span, | ||
E0713, | ||
"temporary value borrowed for too long{OGN}", |
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.
let's phrase this: "temporary value dropped while borrowed"
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.
@bors r+
☔ The latest upstream changes (presumably #54310) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors r+ |
📌 Commit 0397eb0 has been approved by |
(I want this to land asap because its effects are going to interact with whatever diagnostic I develop as our short-term fix for resolving #21114 ) |
⌛ Testing commit 0397eb0 with merge e6cba0cdbea6a8119bd0991f00edc1629a50ce3b... |
💔 Test failed - status-appveyor |
I'm pretty sure this needs to be rebased, because the diagnostic code it adds to (This is something that the Travis-autobuild on your pull request won't catch until you rebase, but bors will catch it during the trial merge after an |
@bors r+ |
📌 Commit d773f7c6a6d2ae9fc0a401991791cd4a299f5f50 has been approved by |
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 r+ |
📌 Commit 2af199d has been approved by |
add "temporary value borrowed for too long" error Issue #54131 r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
Issue #54131
r? @nikomatsakis