Skip to content

Cleanup metadata and incremental cache processing of constants#49079

Merged
bors merged 1 commit intorust-lang:masterfrom
oli-obk:cross_miri
Mar 19, 2018
Merged

Cleanup metadata and incremental cache processing of constants#49079
bors merged 1 commit intorust-lang:masterfrom
oli-obk:cross_miri

Conversation

@oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Mar 16, 2018 •
edited
Loading

fixes #49033
fixes #49081

we really need tests for this. do we have any cross compilation tests? I couldn't find any

@rust-highfive
Copy link
Contributor

rust-highfive commented Mar 16, 2018

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 16, 2018
@michaelwoerister
Copy link
Member

michaelwoerister commented Mar 16, 2018

I'll review this.

michaelwoerister
michaelwoerister reviewed Mar 16, 2018
View reviewed changes
Copy link
Member

@michaelwoerister michaelwoerister left a comment

Choose a reason for hiding this comment

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

Thanks, @oli-obk! Looking good!

r=me with the nits addressed.

src/librustc/mir/interpret/mod.rs Outdated
Copy link
Member

@michaelwoerister michaelwoerister Mar 16, 2018

Choose a reason for hiding this comment

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

It would be nice to define constants for those.

src/librustc/mir/interpret/mod.rs Outdated
Copy link
Member

@michaelwoerister michaelwoerister Mar 16, 2018

Choose a reason for hiding this comment

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

Why not just pass a usize?

src/librustc/ty/maps/on_disk_cache.rs Outdated
Copy link
Member

@michaelwoerister michaelwoerister Mar 16, 2018

Choose a reason for hiding this comment

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

Could you add a comment that explains what this assertion is about?

src/librustc_metadata/encoder.rs Outdated
Copy link
Member

@michaelwoerister michaelwoerister Mar 16, 2018

Choose a reason for hiding this comment

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

Same as above.

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 16, 2018

I decided to fix #49081 here, too, because it's hard/annoying to to before this PR and if I made a second PR they'd just break each other anyway.

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 16, 2018

needs a re-review

@oli-obk oli-obk changed the title Unregress 64 bit rustc reading metadata created by 32 bit rustc Cleanup metadata and incremental cache processing of constants Mar 16, 2018
@alexcrichton
Copy link
Member

alexcrichton commented Mar 16, 2018

@bors: p=1

(I've seen a number of projects that would benefit from getting this quickly!)

@oli-obk oli-obk mentioned this pull request Mar 18, 2018
Closed
@michaelwoerister
Copy link
Member

michaelwoerister commented Mar 19, 2018

Thanks a lot, @oli-obk!

@bors r+

I wonder if we can make this a little less complicated in the future.

@bors
Copy link
Collaborator

bors commented Mar 19, 2018

📌 Commit 49dac83 has been approved by michaelwoerister

@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 19, 2018
@oli-obk oli-obk mentioned this pull request Mar 19, 2018
Closed
@bors
Copy link
Collaborator

bors commented Mar 19, 2018

⌛ Testing commit 49dac83 with merge a04b88d...

bors added a commit that referenced this pull request Mar 19, 2018
@bors
Auto merge of #49079 - oli-obk:cross_miri, r=michaelwoerister
a04b88d 
Cleanup metadata and incremental cache processing of constants

fixes #49033
fixes #49081

we really need tests for this. do we have any cross compilation tests? I couldn't find any
@bors
Copy link
Collaborator

bors commented Mar 19, 2018

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

@bors bors merged commit 49dac83 into rust-lang:master Mar 19, 2018
@oli-obk oli-obk deleted the cross_miri branch June 15, 2020 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@michaelwoerister michaelwoerister michaelwoerister left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@michaelwoerister michaelwoerister

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

No milestone

Development

Successfully merging this pull request may close these issues.

MIRI: Tried to cache DefId(_), but was already existing as AllocId(_) ICE during cross-compilation and const eval

6 participants

@oli-obk @rust-highfive @michaelwoerister @alexcrichton @bors @cramertj