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

Adjust Miri value visitor, and doc-comment layout components #69257

Merged
merged 5 commits into from
Mar 2, 2020

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Feb 18, 2020

I realized that I still didn't have quite the right intuition for how our LayoutDetails work, so I had to adjust the Miri value visitor to the things I understood better now. I also added some doc-comments to LayoutDetails as a hopefully canonical place to note such things.

The main visitor change is that we first look at all the fields (according to FieldPlacement), and then check the variants and handle Multiple appropriately. I did not quite realize how orthogonal "fields" and "variants" are.
I also moved the check for the scalar ABI to after checking all the fields; this leads to better (more type-driven) error messages.

And it looks like we can finally remove that magic hack for ty::Generator. :D

r? @oli-obk for the Miri/visitor changes and @eddyb for the layout docs
The Miri PR is at: rust-lang/miri#1178

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 18, 2020
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

r=me on the visitor and test changes

@RalfJung
Copy link
Member Author

r? @eddyb for the rest

@rust-highfive

This comment has been minimized.

@eddyb
Copy link
Member

eddyb commented Mar 2, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Mar 2, 2020

📌 Commit 12054dc 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 2, 2020
@bors
Copy link
Contributor

bors commented Mar 2, 2020

⌛ Testing commit 12054dc with merge c839a7b...

@bors
Copy link
Contributor

bors commented Mar 2, 2020

☀️ Test successful - checks-azure
Approved by: eddyb
Pushing c839a7b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 2, 2020
@bors bors merged commit c839a7b into rust-lang:master Mar 2, 2020
@RalfJung RalfJung deleted the layout-visitor branch March 2, 2020 12:49
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #69257!

Tested on commit c839a7b.
Direct link to PR: #69257

💔 rls on linux: test-pass → test-fail (cc @Xanewok).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Mar 2, 2020
Tested on commit rust-lang/rust@c839a7b.
Direct link to PR: <rust-lang/rust#69257>

💔 rls on linux: test-pass → test-fail (cc @Xanewok).
bors added a commit to rust-lang/miri that referenced this pull request Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants