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 some debug_assert into assert #69565

Merged
merged 2 commits into from
Mar 4, 2020
Merged

miri engine: turn some debug_assert into assert #69565

merged 2 commits into from
Mar 4, 2020

Conversation

RalfJung
Copy link
Member

@eddyb said to avoid debug assertions in rustc. These checks here look like they are probably not too expensive. Cc @oli-obk

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(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 Feb 28, 2020
@RalfJung
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Feb 28, 2020

⌛ Trying commit 6f568e7 with merge 8d08490855c62c792e8587b3c8ef97606893653e...

eddyb
eddyb approved these changes Feb 29, 2020
View reviewed changes
Copy link
Member

@eddyb eddyb 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 if perf is fine

@bors
Copy link
Contributor

bors commented Feb 29, 2020

☀️ Try build successful - checks-azure
Build commit: 8d08490855c62c792e8587b3c8ef97606893653e (8d08490855c62c792e8587b3c8ef97606893653e)

@rust-timer
Copy link
Collaborator

Queued 8d08490855c62c792e8587b3c8ef97606893653e with parent 0eb878d, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 8d08490855c62c792e8587b3c8ef97606893653e, comparison URL.

@RalfJung
Copy link
Member Author

There's +1.5% on many "clean incremental" and "patched incremental" tests (i.e., tests that run very shortly). I suppose @nnethercote wouldn't like to lose those, so I'll have to make a guess about which assertion might be the culprit here.

@RalfJung
Copy link
Member Author

r? @eddyb (to fix assignee)

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@rust-highfive rust-highfive assigned eddyb and unassigned matthewjasper Feb 29, 2020
@bors
Copy link
Contributor

bors commented Feb 29, 2020

⌛ Trying commit 5982e9d with merge 62010d6e5f1c1827a115c971e7f6fd12e21bf65c...

@bors
Copy link
Contributor

bors commented Feb 29, 2020

☀️ Try build successful - checks-azure
Build commit: 62010d6e5f1c1827a115c971e7f6fd12e21bf65c (62010d6e5f1c1827a115c971e7f6fd12e21bf65c)

@rust-timer
Copy link
Collaborator

Queued 62010d6e5f1c1827a115c971e7f6fd12e21bf65c with parent 55aee8d, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 62010d6e5f1c1827a115c971e7f6fd12e21bf65c, comparison URL.

@RalfJung
Copy link
Member Author

Now it looks like noise.

@bors r=eddyb

@bors
Copy link
Contributor

bors commented Feb 29, 2020

📌 Commit 5982e9d 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 Feb 29, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 3, 2020
@Dylan-DPC
Rollup merge of rust-lang#69565 - RalfJung:assert, r=eddyb
f53764a 
miri engine: turn some debug_assert into assert

@eddyb said to avoid debug assertions in rustc. These checks here look like they are probably not too expensive. Cc @oli-obk
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 3, 2020
Closed
bors added a commit that referenced this pull request Mar 3, 2020
@bors
Auto merge of #69652 - Dylan-DPC:rollup-0mlvtin, r=Dylan-DPC
bf320fe 
Rollup of 9 pull requests

Successful merges:

 - #69565 (miri engine: turn some debug_assert into assert)
 - #69609 (Remove `usable_size` APIs)
 - #69620 (doc(librustc_error_codes): add long error explanation for E0719)
 - #69626 (Toolstate: don't duplicate nightly tool list.)
 - #69628 (Fix a leak in `DiagnosticBuilder::into_diagnostic`.)
 - #69633 (Update my mailmap entry)
 - #69634 (clean up E0378 explanation)
 - #69637 (Don't convert Results to Options just for matching.)
 - #69641 (Update books)

Failed merges:

r? @ghost
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 3, 2020
Merged
bors added a commit that referenced this pull request Mar 3, 2020
@bors
Auto merge of #69678 - Dylan-DPC:rollup-yoaueud, r=Dylan-DPC
592e9c3 
Rollup of 6 pull requests

Successful merges:

 - #69565 (miri engine: turn some debug_assert into assert)
 - #69621 (use question mark operator in a few places.)
 - #69650 (cleanup more iterator usages (and other things))
 - #69653 (use conditions directly)
 - #69665 (Invoke OptimizerLastEPCallbacks in PreLinkThinLTO)
 - #69670 (Add explanation for E0379)

Failed merges:

r? @ghost
@bors bors merged commit fab54fc into rust-lang:master Mar 4, 2020
@RalfJung RalfJung deleted the assert branch March 4, 2020 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyb eddyb eddyb approved these changes

Assignees

@eddyb eddyb

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.
6 participants
@RalfJung @rust-highfive @rust-timer @bors @eddyb @matthewjasper