-
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
Do not promote constants that contain unpromotable code #51570
Conversation
@bors try |
Do not promote constants that contain unpromotable code r? @eddyb cc @nikomatsakis I'll add more tests, just getting this out for a crater run
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 |
💔 Test failed - status-travis |
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 |
1 similar comment
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 |
378ef7d
to
e76172c
Compare
Supersedes #51500 |
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 |
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 try |
Do not promote constants that contain unpromotable code r? @eddyb cc @nikomatsakis I'll add more tests, just getting this out for a crater run
☀️ Test successful - status-travis |
@rust-lang/infra Can you start a check-only crater run? |
cc @pietroalbini ↑ |
Check-only Crater run started on |
Hi @oli-obk (crater requester), @eddyb (PR reviewer)! Crater results are at: http://cargobomb-reports.s3.amazonaws.com/pr-51570/index.html. 'Blacklisted' crates (spurious failures etc) can be found here. If you see any spurious failures not on the list, please make a PR against that file. (interested observers: Crater is a tool for testing the impact of changes on the crates.io ecosystem. You can find out more at the repo if you're curious) |
Out of the crates previously building, 3 crates regressed: No spurious failure. |
I opened a PR to |
|
||
|
||
fn main() { | ||
let x: &'static usize = &FOO; //~ ERROR does not live long enough |
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 we discussed this at the compiler meeting, and constants should be promoted irrelevant of the constant's body in case the constant's value is never inspected.
Examples like the one below (&(FOO % 42)
) will still not get promoted.
Examples like &SomeStruct { a: FOO }
will get promoted, because the value is only copied, not inspected.
☔ The latest upstream changes (presumably #51884) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #51110) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from triage @oli-obk! There are conflicts that needs to be solved. |
blocked on #52318 |
ok, this is ready for the next review round |
☔ The latest upstream changes (presumably #52264) made this pull request unmergeable. Please resolve the merge conflicts. |
571868e
to
b7915b7
Compare
I am not sure what exactly the promotion rules are that are implemented here, but I just made a proposal at https://www.ralfj.de/blog/2018/07/19/const.html and would be curious for your feedback :) |
☔ The latest upstream changes (presumably #52476) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from triage, @eddyb: The PR author has requested another review from you. |
AFAIK @oli-obk decided to do this somewhat differently, based on various discussions after my blog post. |
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 |
Ping from triage! What's the status of this? |
I don't have time for this right now and it needs major changes |
r? @eddyb
cc @nikomatsakis
fixes #50814