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

Perform Sync check on static items in wf-check instead of during const checks #91251

Merged
merged 1 commit into from
Nov 28, 2021
Merged

Perform Sync check on static items in wf-check instead of during const checks #91251

merged 1 commit into from
Nov 28, 2021

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Nov 26, 2021

r? @RalfJung

This check is solely happening on the signature of the static item and not on its body, therefor it belongs into wf-checking instead of const checking.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2021
@RalfJung
Copy link
Member

I'm not really familiar with wf-checking or how we usually split the work between wf-checking, type-checking, const-checking.

This should not change which programs get accepted, right? It just changes diagnostics due to the check now running in a different 'phase'?

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 26, 2021

This should not change which programs get accepted, right? It just changes diagnostics due to the check now running in a different 'phase'?

yea, we never get to the body of the thing if wf-checking its signature fails

I'm not really familiar with wf-checking or how we usually split the work between wf-checking, type-checking, const-checking.

That's fine, I'll reroll the reviewer within the compiler team if/when you're ok with the behavioral changes of this PR.

@RalfJung
Copy link
Member

behavioral changes

Didn't you just say there are none?^^
The diagnostic changes look okay to me.

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 26, 2021

well... diagnostics are user-visible behaviour XD but yes, no new code compiles, no code stops compiling

r? rust-lang/compiler

@matthewjasper
Copy link
Contributor

@bors r+

@matthewjasper matthewjasper reopened this Nov 28, 2021
@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 28, 2021

📌 Commit 1869412 has been approved by matthewjasper

@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 Nov 28, 2021
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 28, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 28, 2021
@bors
Auto merge of rust-lang#91320 - matthiaskrgr:rollup-r209seq, r=matthi…
db9d361 
…askrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#91251 (Perform Sync check on static items in wf-check instead of during const checks)
 - rust-lang#91308 (Fix ICE when lowering `trait A where for<'a> Self: 'a`)
 - rust-lang#91319 (Change output path to {{build-base}} for rustdoc scrape_examples ui test)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 233c50e into rust-lang:master Nov 28, 2021
@rustbot rustbot added this to the 1.59.0 milestone Nov 28, 2021
@jruderman jruderman mentioned this pull request Dec 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@matthewjasper matthewjasper

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.
6 participants
@oli-obk @RalfJung @matthewjasper @bors @rust-highfive @rustbot