Skip to content

Enforce stability of const fn in promoteds#50909

Merged
bors merged 2 commits intorust-lang:masterfrom
oli-obk:unstable_const_fn_promotion
May 24, 2018
Merged

Enforce stability of const fn in promoteds#50909
bors merged 2 commits intorust-lang:masterfrom
oli-obk:unstable_const_fn_promotion

Conversation

@oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 20, 2018 •
edited
Loading

r? @eddyb

fixes #50901

what's going on here? Why do we have two promoted computation algorithms?

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2018
@oli-obk oli-obk added A-stability Area: `#[stable]`, `#[unstable]` etc. beta-nominated Nominated for backporting to the compiler in the beta channel. labels May 20, 2018
@rust-highfive
Copy link
Contributor

rust-highfive commented May 20, 2018

The job x86_64-gnu-llvm-3.9 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. 

[00:05:32] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:33] tidy error: /checkout/src/test/ui/const-eval/dont_promote_unstable_const_fn.rs: missing trailing newline
[00:05:33] tidy error: /checkout/src/librustc_mir/transform/qualify_consts.rs:962: trailing whitespace
[00:05:34] some tidy checks failed
[00:05:34] 
[00:05:34] 
[00:05:34] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:34] 
[00:05:34] 
[00:05:34] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:34] Build completed unsuccessfully in 0:02:08
[00:05:34] Build completed unsuccessfully in 0:02:08
[00:05:34] Makefile:79: recipe for target 'tidy' failed
[00:05:34] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0d5190c8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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 oli-obk force-pushed the unstable_const_fn_promotion branch from c409546 to 0a94f78 Compare May 20, 2018 07:50
eddyb
eddyb reviewed May 20, 2018
View reviewed changes
src/librustc_mir/transform/qualify_consts.rs Outdated
// We are in a normal function
// with a turned off feature gate. We can still call the function
// but we can't promote it
self.qualif = Qualif::NOT_CONST;
Copy link
Member

@eddyb eddyb May 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this inside the if? IMO the flags should be as general as possible, so we can make sure less problems show up again. Also, shouldn't this be something like self.add(Qualif::NOT_CONST)? (EDIT: ah, no, below you can see it's also that; the overwrite is because the argument qualifications and the function call result are unrelated and the former have to be cleared anyway)

Copy link
Contributor Author

@oli-obk oli-obk May 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't make much difference as the other branch errors, but seems more consistent indeed

@eddyb
Copy link
Member

eddyb commented May 20, 2018

@bors try

(we'll likely need a check-only crater run on this)

@bors
Copy link
Collaborator

bors commented May 20, 2018

⌛ Trying commit 329fd90 with merge f0ca7e1...

bors added a commit that referenced this pull request May 20, 2018
@bors
Auto merge of #50909 - oli-obk:unstable_const_fn_promotion, r=<try>
f0ca7e1 
Enforce stability of const fn in promoteds

r? @eddyb

what's going on here? Why do we have two promoted computation algorithms?
@oli-obk
Copy link
Contributor Author

oli-obk commented May 20, 2018

In case of regressions we could just stabilize the relevant const fns together with fixing the stability hole

@bors
Copy link
Collaborator

bors commented May 20, 2018

☀️ Test successful - status-travis
State: approved= try=True

@oli-obk
Copy link
Contributor Author

oli-obk commented May 21, 2018

Ping @rust-lang/infra can you start a check-only crater run?

@emilyalbini emilyalbini added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 21, 2018
@emilyalbini
Copy link
Member

emilyalbini commented May 21, 2018

Crater run (check-only) started.

@emilyalbini
Copy link
Member

emilyalbini commented May 24, 2018

Hi @oli-obk (crater requester), @eddyb (PR reviewer)! Crater results are at: http://cargobomb-reports.s3.amazonaws.com/pr-50909/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)

@emilyalbini emilyalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 24, 2018
@eddyb
Copy link
Member

eddyb commented May 24, 2018

Looks clean! (spurious regression)

r=me after rebase

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 24, 2018
@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 24, 2018
@oli-obk oli-obk force-pushed the unstable_const_fn_promotion branch from 329fd90 to a11f785 Compare May 24, 2018 15:27
@oli-obk
Copy link
Contributor Author

oli-obk commented May 24, 2018

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented May 24, 2018

📌 Commit a11f785 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2018
@emilyalbini
Copy link
Member

emilyalbini commented May 24, 2018

@bors p=1 (backporting to beta)

This was referenced May 24, 2018
Merged
Merged
@bors
Copy link
Collaborator

bors commented May 24, 2018

⌛ Testing commit a11f785 with merge 0746522...

bors added a commit that referenced this pull request May 24, 2018
@bors
Auto merge of #50909 - oli-obk:unstable_const_fn_promotion, r=eddyb
0746522 
Enforce stability of const fn in promoteds

r? @eddyb

fixes #50901

what's going on here? Why do we have two promoted computation algorithms?
@bors bors mentioned this pull request May 24, 2018
Merged
@bors
Copy link
Collaborator

bors commented May 24, 2018

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

@bors bors merged commit a11f785 into rust-lang:master May 24, 2018
@emilyalbini emilyalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 25, 2018
@oli-obk oli-obk deleted the unstable_const_fn_promotion branch June 15, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@eddyb eddyb eddyb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@eddyb eddyb

Labels

A-stability Area: `#[stable]`, `#[unstable]` etc. beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

rustc_const_unstable attribute ignored in const promotion

7 participants

@oli-obk @rust-highfive @eddyb @bors @emilyalbini @kennytm @nikomatsakis