Skip to content
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

Fix #54224 (const promotion regression) #54715

Merged
merged 2 commits into from
Oct 12, 2018

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Oct 1, 2018

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 1, 2018
@rust-highfive

This comment has been minimized.

@TimNN
Copy link
Contributor

TimNN commented Oct 9, 2018

Ping from triage @eddyb / @rust-lang/compiler: This PR requires your review.

@oli-obk
Copy link
Contributor Author

oli-obk commented Oct 11, 2018

r? @RalfJung

@rust-highfive rust-highfive assigned RalfJung and unassigned eddyb Oct 11, 2018
@RalfJung
Copy link
Member

I am confused. #54224 says it's not a bug, and it also doesn't mention anything about this being a regression. And then the fix for an issue that something doesn't get promoted is to make more things NOT_CONST?

@oli-obk
Copy link
Contributor Author

oli-obk commented Oct 11, 2018

This is a regression introduced in #51990 (comment)

We'll want to relax this in the future, but for now we should not change behaviour

@RalfJung
Copy link
Member

Ah, so right now we accidentally accept some of the code you added in that test case?

@RalfJung
Copy link
Member

There still is a Qualif::NOT_CONST below a self.mode test at

self.add(Qualif::NOT_CONST);
Is that correct? If yes, could you add a comment saying why this is only rejected for Fn but not for the other modes?

Same for

self.add(Qualif::NOT_CONST);

@RalfJung
Copy link
Member

Oh I see, the difference is that in the other cases we always emit a feature gate in the else. That's what went wrong here. Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 12, 2018

📌 Commit 76f8a90 has been approved by RalfJung

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 12, 2018
@bors
Copy link
Contributor

bors commented Oct 12, 2018

⌛ Testing commit 76f8a90 with merge e9e27e6...

bors added a commit that referenced this pull request Oct 12, 2018
@bors
Copy link
Contributor

bors commented Oct 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: RalfJung
Pushing e9e27e6 to master...

@bors bors merged commit 76f8a90 into rust-lang:master Oct 12, 2018
@oli-obk oli-obk deleted the nll_deref_promotion branch June 15, 2020 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants