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.

Sign up for GitHub

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

Jump to bottom

Don't promote function calls to nonpromotable things #58784

Merged
merged 2 commits into from
Mar 11, 2019
Merged

Don't promote function calls to nonpromotable things #58784

merged 2 commits into from
Mar 11, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 27, 2019 •
edited

fixes #58767 and fixes #58634

r? @eddyb

should we additionally check the function call return type? It might be a promotable function (or any const fn inside a const fn), but its return type might contain interior mutability.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 27, 2019
@oli-obk oli-obk added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 28, 2019
@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 28, 2019
@pnkfelix
Copy link
Member

discussed at T-compiler meeting. approved for beta backport.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 28, 2019
@eddyb
Copy link
Member

eddyb commented Mar 1, 2019

IsNotPromotable should be removed in favor of just IsNotConst, but right now this seems good.

Looks like the reason this regressed is because "not promotable" used to be turned into "not const" but at some point in the refactor I simplified it too much and only preserved "not const" from callee/arguments, but not "not promotable".

r=me with a FIXME about removing IsNotPromotable

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 1, 2019

@bors r=eddyb

@bors
Copy link
Contributor

bors commented Mar 1, 2019

📌 Commit 8c16507 has been approved by eddyb

@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 Mar 1, 2019
@bors
Copy link
Contributor

bors commented Mar 4, 2019

⌛ Testing commit 8c16507 with merge f40e37747ba74cd7fd974cb092033474d4d8c6f9...

@bors
Copy link
Contributor

bors commented Mar 4, 2019

💔 Test failed - status-appveyor

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

ljedrz commented Mar 4, 2019

The error looks spurious; maybe my try rights are already in force?

@bors retry

@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 Mar 4, 2019
@bors
Copy link
Contributor

bors commented Mar 4, 2019

⌛ Testing commit 8c16507 with merge 113a74e1c5bdcf894fd624d1465b342324779fc6...

@bors
Copy link
Contributor

bors commented Mar 4, 2019

💔 Test failed - status-appveyor

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

oli-obk commented Mar 5, 2019

Build completed successfully in 2:58:53
Command exited with code 259

@bors retry (different builder failed that passed in the previous run)

@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 Mar 5, 2019
@oli-obk oli-obk mentioned this pull request Mar 5, 2019
Closed
@oli-obk oli-obk mentioned this pull request Mar 10, 2019
Closed
@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 10, 2019

@bors r=eddyb p=1

@bors
Copy link
Contributor

bors commented Mar 10, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Mar 10, 2019

📌 Commit 8c16507 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Mar 10, 2019

⌛ Testing commit 8c16507 with merge 2d04b00...

bors added a commit that referenced this pull request Mar 10, 2019
@bors
Auto merge of #58784 - oli-obk:accidental_promotion, r=eddyb
2d04b00 
Don't promote function calls to nonpromotable things

fixes #58767 and fixes #58634

r? @eddyb

should we additionally check the function call return type? It might be a promotable function (or any `const fn` inside a `const fn`), but its return type might contain interior mutability.
@bors
Copy link
Contributor

bors commented Mar 10, 2019

💔 Test failed - checks-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 10, 2019
@rust-highfive
Copy link
Collaborator

The job i686-gnu-nopt of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
[02:57:51] travis_fold:start:test_stage2-rustdoc
travis_time:start:test_stage2-rustdoc
Testing rustdoc stage2 (i686-unknown-linux-gnu -> i686-unknown-linux-gnu)
[02:57:51]    Compiling rustdoc v0.0.0 (/checkout/src/librustdoc)
The job exceeded the maximum time limit for jobs, and has been terminated.

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 @TimNN. (Feature Requests)

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 10, 2019

@bors retry timeout

@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 Mar 10, 2019
@bors
Copy link
Contributor

bors commented Mar 11, 2019

⌛ Testing commit 8c16507 with merge c2ddf5a...

bors added a commit that referenced this pull request Mar 11, 2019
@bors
Auto merge of #58784 - oli-obk:accidental_promotion, r=eddyb
c2ddf5a 
Don't promote function calls to nonpromotable things

fixes #58767 and fixes #58634

r? @eddyb

should we additionally check the function call return type? It might be a promotable function (or any `const fn` inside a `const fn`), but its return type might contain interior mutability.
@bors
Copy link
Contributor

bors commented Mar 11, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: eddyb
Pushing c2ddf5a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 11, 2019
@bors bors merged commit 8c16507 into rust-lang:master Mar 11, 2019
@oli-obk oli-obk deleted the accidental_promotion branch March 11, 2019 11:45
@oli-obk oli-obk mentioned this pull request Mar 11, 2019
Closed
@pietroalbini pietroalbini mentioned this pull request Mar 16, 2019
Merged
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 16, 2019
bors added a commit that referenced this pull request Mar 16, 2019
@bors
Auto merge of #59235 - pietroalbini:beta-rollup, r=pietroalbini
7473869 
[beta] Rollup backports

Cherry-picked:

* Include bounds from promoted constants in NLL #57202
* Warning period for detecting nested impl trait #58608
* Don't promote function calls to nonpromotable things #58784
* Make migrate mode work at item level granularity #58788
* Expand where negative supertrait specific error is shown #58861
* Expand where negative supertrait specific error is shown #58861

Rolled up:

* [BETA] Update cargo #59217

r? @ghost
@lqd lqd mentioned this pull request Mar 25, 2019
Closed
@oli-obk oli-obk mentioned this pull request Apr 8, 2019
Merged
Centril added a commit to Centril/rust that referenced this pull request Apr 12, 2019
@Centril
Rollup merge of rust-lang#59796 - oli-obk:const_arg_ice, r=eddyb
eb29fd0 
Retire `IsNotConst` naming

This naming scheme caused a lot of confusion lately (including ICEs) due to misrefactored code. Also clean up the initialization code for said flag.

r? @eddyb

previous discussions: rust-lang#58784 (comment) rust-lang#58403 (comment)
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
@Centril
Rollup merge of rust-lang#59796 - oli-obk:const_arg_ice, r=eddyb
ad30fd3 
Retire `IsNotConst` naming

This naming scheme caused a lot of confusion lately (including ICEs) due to misrefactored code. Also clean up the initialization code for said flag.

r? @eddyb

previous discussions: rust-lang#58784 (comment) rust-lang#58403 (comment)
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
@Centril
Rollup merge of rust-lang#59796 - oli-obk:const_arg_ice, r=eddyb
2fbd7be 
Retire `IsNotConst` naming

This naming scheme caused a lot of confusion lately (including ICEs) due to misrefactored code. Also clean up the initialization code for said flag.

r? @eddyb

previous discussions: rust-lang#58784 (comment) rust-lang#58403 (comment)
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
@Centril
Rollup merge of rust-lang#59796 - oli-obk:const_arg_ice, r=eddyb
37b76f2 
Retire `IsNotConst` naming

This naming scheme caused a lot of confusion lately (including ICEs) due to misrefactored code. Also clean up the initialization code for said flag.

r? @eddyb

previous discussions: rust-lang#58784 (comment) rust-lang#58403 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@eddyb eddyb

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rustc panic when building bobbin-sdk ICE on rustc 1.34.0-nightly (633d75ac1 2019-02-21): index out of bounds
7 participants
@oli-obk @pnkfelix @eddyb @bors @ljedrz @rust-highfive @pietroalbini