Skip to content

smoke-test for async fn with mir-opt-level=0#71444

Merged
bors merged 2 commits into
rust-lang:masterfrom
RalfJung:test-async-no-opt
Apr 28, 2020
Merged

smoke-test for async fn with mir-opt-level=0#71444
bors merged 2 commits into
rust-lang:masterfrom
RalfJung:test-async-no-opt

Conversation

@RalfJung

Copy link
Copy Markdown
Member

MIR opt levels heavily influence which MIR transformations run, and we barely test non-default opt levels. I am particularly worried about async fn lowering and how it might (not) work when the set of preceding MIR passes changes -- see #70073.

This adds some basic smoke testing, where at least a few async fn run-pass test are ensured to also work with mir-opt-level=0.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

(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 Apr 22, 2020
@jonas-schievink

Copy link
Copy Markdown
Contributor

I wouldn't do this for the size tests as those could be affected by other optimizations running. For the smoke tests this seems good though.

@RalfJung

Copy link
Copy Markdown
Member Author

The size can be affected by MIR optimizations?

@RalfJung RalfJung force-pushed the test-async-no-opt branch from be87eb1 to 9ea5eed Compare April 22, 2020 21:34
@RalfJung

Copy link
Copy Markdown
Member Author

Okay this does not touch the size tests any more.

@jonas-schievink

Copy link
Copy Markdown
Contributor

The size can be affected by MIR optimizations?

Potentially yeah, at least in the future when we have some more advanced optimizations.

@RalfJung

Copy link
Copy Markdown
Member Author

@jonas-schievink any chance you could review this?

@jonas-schievink jonas-schievink left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. If you want you can also do this for some of the generator tests for some more diversity. But generally r=me

@RalfJung

Copy link
Copy Markdown
Member Author

@jonas-schievink like so?

@jonas-schievink

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Apr 28, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 3a129df has been approved by jonas-schievink

@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 Apr 28, 2020
@bors

bors commented Apr 28, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 3a129df with merge d7afaa7...

@bors

bors commented Apr 28, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-azure
Approved by: jonas-schievink
Pushing d7afaa7 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 28, 2020
@bors bors merged commit d7afaa7 into rust-lang:master Apr 28, 2020
@RalfJung RalfJung deleted the test-async-no-opt branch April 30, 2020 08:43
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