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

Enforce stability of const fn in promoteds #50909

Merged
merged 2 commits into from
May 24, 2018

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 20, 2018

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
Collaborator

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
// 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

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

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
Contributor

bors commented May 20, 2018

⌛ Trying commit 329fd90 with merge f0ca7e1...

bors added a commit that referenced this pull request May 20, 2018
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
Contributor

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?

@pietroalbini pietroalbini 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
@pietroalbini
Copy link
Member

Crater run (check-only) started.

@pietroalbini
Copy link
Member

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)

@pietroalbini pietroalbini 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
Contributor

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
@pietroalbini
Copy link
Member

@bors p=1 (backporting to beta)

@bors
Copy link
Contributor

bors commented May 24, 2018

⌛ Testing commit a11f785 with merge 0746522...

bors added a commit that referenced this pull request May 24, 2018
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
Copy link
Contributor

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
@pietroalbini pietroalbini 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
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
Development

Successfully merging this pull request may close these issues.

rustc_const_unstable attribute ignored in const promotion
7 participants