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

Skip field retagging on ZSTs, it can take forever #2517

Merged
merged 2 commits into from
Aug 29, 2022

Conversation

saethlin
Copy link
Member

@saethlin saethlin commented Aug 28, 2022

I just tried running the alloc's tests with miri-test-libstd with field retagging enabled. The test suite eventually hangs on a few tests which pass around ZSTs that have a lot of fields.

I don't really know how to test this effectively. The test passes, but if you remove this fast-path it effectively just hangs the interpreter. And since it hangs inside a step, there's no hope for doing some kind of timeout within the test.

@RalfJung
Copy link
Member

RalfJung commented Aug 28, 2022 via email

@saethlin saethlin force-pushed the zst-field-retagging branch from f9b9e4d to 45bfd71 Compare August 29, 2022 00:28
@saethlin saethlin force-pushed the zst-field-retagging branch from 45bfd71 to 70b960b Compare August 29, 2022 04:31
@RalfJung
Copy link
Member

Thanks. :)
@bors r+

@bors
Copy link
Contributor

bors commented Aug 29, 2022

📌 Commit c9b36b4 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 29, 2022

⌛ Testing commit c9b36b4 with merge 284b59c...

@bors
Copy link
Contributor

bors commented Aug 29, 2022

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 284b59c to master...

@bors bors merged commit 284b59c into rust-lang:master Aug 29, 2022
@saethlin saethlin deleted the zst-field-retagging branch August 30, 2022 00:56
RalfJung added a commit to rust-lang/miri-test-libstd that referenced this pull request Sep 2, 2022
This reverts commit 137592c.
This times out; probably needs rust-lang/miri#2517
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants