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

miri engine: turn error sanity checks into assertions #69880

Merged
merged 1 commit into from
Mar 23, 2020
Merged

miri engine: turn error sanity checks into assertions #69880

merged 1 commit into from
Mar 23, 2020

Conversation

RalfJung
Copy link
Member

We had these as debug assertions so far to make sure our test suite is clean, but really these are conditions that should never arise and also @eddyb told me to turn non-performance-critical debug assertions into full assertions so here we go. ;)

I propose that we do a check-only crater run to make sure this does not actually happen in practice.

r? @oli-obk

@RalfJung
Copy link
Member Author

@bors try

bors added a commit that referenced this pull request Mar 10, 2020
@bors
Auto merge of #69880 - RalfJung:miri-assert-error-sanity, r=<try>
13f916f 
miri engine: turn error sanity checks into assertions

We had these as debug assertions so far to make sure our test suite is clean, but really these are conditions that should never arise and also @eddyb told me to turn non-performance-critical debug assertions into full assertions so here we go. ;)

I propose that we do a check-only crater run to make sure this does not actually happen in practice.

r? @oli-obk
@bors
Copy link
Contributor

bors commented Mar 10, 2020

⌛ Trying commit afc9cf3 with merge 13f916f...

@bors
Copy link
Contributor

bors commented Mar 10, 2020

☀️ Try build successful - checks-azure
Build commit: 13f916f (13f916f63bc6575d5ff1f989237dfabe88981368)

@RalfJung RalfJung added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 10, 2020
@Centril
Copy link
Contributor

Centril commented Mar 11, 2020

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-69880 created and queued.
🤖 Automatically detected try build 13f916f
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot 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 Mar 11, 2020
@craterbot
Copy link
Collaborator

🚧 Experiment pr-69880 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-69880 is completed!
📊 1 regressed and 3 fixed (95301 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot 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 Mar 16, 2020
@RalfJung
Copy link
Member Author

Hm, that regression is interesting: broken MIR in DefId(0:37 ~ ice_testcase[f77b]::problematic_function[0]). Doesn't sound like it would come from this PR? ice_testcase is also a funny name for that crate. ;)

@RalfJung
Copy link
Member Author

@oli-obk I think this is ready to go, and crater didn't find any regression.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 22, 2020

r=me after a rebase

@RalfJung RalfJung force-pushed the miri-assert-error-sanity branch from afc9cf3 to 2ee2157 Compare March 22, 2020 21:49
@RalfJung
Copy link
Member Author

Ah I hadn't even realized there was a conflict (looks like bors missed it too). Done!

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Mar 22, 2020

📌 Commit 2ee2157 has been approved by oli-obk

@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 22, 2020
@Centril Centril mentioned this pull request Mar 23, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 23, 2020
@bors
Auto merge of rust-lang#70296 - Centril:rollup-wvfmb3n, r=Centril
8ff7850 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#69251 (#[track_caller] in traits)
 - rust-lang#69880 (miri engine: turn error sanity checks into assertions)
 - rust-lang#70207 (Use getentropy(2) on macos)
 - rust-lang#70227 (Only display definition when suggesting a typo)
 - rust-lang#70236 (resolve: Avoid "self-confirming" import resolutions in one more case)
 - rust-lang#70248 (parser: simplify & remove unused field)
 - rust-lang#70249 (handle ConstKind::Unresolved after monomorphizing)
 - rust-lang#70269 (remove redundant closures (clippy::redundant_closure))
 - rust-lang#70270 (Clean up E0449 explanation)

Failed merges:

r? @ghost
@bors bors merged commit a2b469c into rust-lang:master Mar 23, 2020
@RalfJung RalfJung deleted the miri-assert-error-sanity branch March 24, 2020 08:58
@RalfJung RalfJung mentioned this pull request Apr 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

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.
5 participants
@RalfJung @bors @Centril @craterbot @oli-obk