-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
End temporary lifetimes being extended by let X: &_
hints
#39066
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
Started a crater run from 27b9e6d to edb3baaa6c0e14481e4a4fa771d57f67f21d5a14. |
Crater doesn't seem to be done. I asked it to test the crates for those two commits. |
Hm, actually I don't see a working toolchain, so I asked crater to build those too:
|
@brson yeah I was getting null results for some reason, not sure what went wrong :( |
Ok, there was a bug in crater. The message broker shutdown at some point and crater never recovered. After starting back up it received the toolchain build complete messages, and I've started the crate builds. |
Crater says. Only 2 false positives. |
No positives yay! We should just merge this. @nikomatsakis - since there is nothing on Crater, do you think we can go without a warning? |
@arielb1 Hmm, interesting! I'm not sure. Can we land it but also include some of this code, so that if you do get an error, we give a nice error message directing you to a tracking issue? How hard would that be? |
@arielb1 (after all we don't have 100% coverage, crater is still linux only I think, but also outside of crates.io) |
Added some warnings in a commit - the case that causes new lifetime errors is quite pathological (I added a test), but it exists. |
expr_span, | ||
"this expression will be dropped at a different time - see #38603"); | ||
"this expression used to live longer due to #36082"); |
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.
we should make a proper tracking issue with an explanation of how to rewrite code to be unambiguous, but this seems good otherwise
|
||
|
||
fn main() { | ||
let a: &_ = *&&0; |
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.
what is this test ... testing?
6a0c247
to
33efa45
Compare
33efa45
to
82a2805
Compare
Ready for review. |
let X: &_
hints
mc::Categorization::Rvalue(r, or) if r != or => { | ||
db.note("\ | ||
before rustc 1.16, this temporary lived longer - see issue #39283 \ | ||
(https://github.com/rust-lang/rust/issues/39283)"); |
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.
nice, thanks for writing that up
@bors r+ |
📌 Commit 82a2805 has been approved by |
⌛ Testing commit 82a2805 with merge a1030b2... |
💔 Test failed - status-travis |
Strange error on the dist bot - cc @alexcrichton @bors retry |
End temporary lifetimes being extended by `let X: &_` hints cc #39283 r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
cc #39283
r? @nikomatsakis