Rollup of 9 pull requests #63341
Merged
Rollup of 9 pull requests #63341
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This prevents uninhabited fields from "infecting" the abi and largest_niche of the generator layout. This fixes a latent bug, where an uninhabited field could be promoted to the generator prefix and cause the entire generator to become uninhabited.
…ay through execution
I tested the generator optimizations in rust-lang#60187 and rust-lang#61922 on the Fuchsia build, and noticed that some small generators (about 8% of the async fns in our build) increased in size slightly. This is because in rust-lang#60187 we split the fields into two groups, a "prefix" non-overlap region and an overlap region, and lay them out separately. This can introduce unnecessary padding bytes between the two groups. In every single case in the Fuchsia build, it was due to there being only a single variant being used in the overlap region. This means that we aren't doing any overlapping, period. So it's better to combine the two regions into one and lay out all the fields at once, which is what this change does.
…ssions, r=cramertj Fix generator size regressions due to optimization I tested the generator optimizations in rust-lang#60187 and rust-lang#61922 on the Fuchsia build, and noticed that some small generators (about 8% of the async fns in our build) increased in size slightly. This is because in rust-lang#60187 we split the fields into two groups, a "prefix" non-overlap region and an overlap region, and lay them out separately. This can introduce unnecessary padding bytes between the two groups. In every single case in the Fuchsia build, it was due to there being only a single variant being used in the overlap region. This means that we aren't doing any overlapping, period. So it's better to combine the two regions into one and lay out all the fields at once, which is what this change does. r? @cramertj cc @eddyb @Zoxc
add a pair of whitespace after remove parentheses fix [issue-63156](rust-lang#63156). add a pair of whitespace after remove parentheses.
tests for async/await drop order This is just me helping out with rust-lang#62121 where I can. I'm also going to use this as a public place to collect my thoughts about what has already been done and what hasn't (adding comments to the dropbox paper doc was quickly getting spammy). I've tried to keep my commit messages similar to the line items on https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiKouT0L41mSnK1741s~TiiRAg-nMyZGrra7dz9KcFRMLKJy as possible. A bunch of my tests are likely to be either redundant with other tests, or lower quality than other tests that people are writing. A reasonable approach might be to tell me which commits you want to keep and I'll throw away the rest of them. The part from the dropbox paper doc that I'm concentrating on here is: (items marked with `?` are ones that I can't immediately think of how to test, so I will leave for other people. Items with checkboxes are things that I have done or will try to do next) ### Dynamic semantics - `async`/`await` with unusual locals: - ? partially uninhabited - ? conditionally initialized - ~drop impls~ already done in src/test/ui/async-await/drop-order/* - ? nested drop impls - ~partially moved (e.g., `let x = (vec![], vec![]); drop(x.0); foo.await; drop(x.1);`)~ see rust-lang#63310 - Control flow: - basic - complex - [x] `.await` while holding variables of different sizes - (possibly) drop order - [x] including drop order for locals when a future is dropped part-way through execution - Parameters' drop order is covered in my commit f40190a - ~An async fn version of `dynamic-drop.rs`~ - already done by matthewjasper in https://github.com/rust-lang/rust/pull/62193/files - ? interaction with const eval, promoteds, and temporaries - [x] drop the resulting future and check that local variables and parameters are dropped in the expected order (interaction with cancellation, in other words) - also in f40190a Explanation of commits: * 0a1bdd4 is the simplest place I could think of to explicitly test `.await` while holding variables of different sizes. I'm pretty sure that this will end up being redundant with something else, so I'm happy to drop it. * f40190a is a copy-paste from `drop-order-for-async-fn-parameters.rs` with `NeverReady.await` dumped on the end of each testcase. * Normally I don't like copy-paste-based tests, but `drop-order-for-async-fn-parameters-by-ref-binding.rs` is also copy-paste, so I thought it might be okay. * [x] I'm a bit sad that this doesn't cover non-parameter locals, but I think it should be easy enough to extend in that direction, so I might have a crack at that tomorrow. * c4940e0 makes a bunch of local variables and moves them into either `{}` blocks or `async move {}` blocks, checking for any surprising differences. * I have tried to give the test functions descriptive names * I have not duplicated the tests for methods with/without self. * I think that all of these tests could be rewritten to be clearer if I could write down the expected drop order next to each test.
don't ignore mir_dump folder I dumped some MIR and wondered why `git status` wouldn't show the tree as dirty, reminding me to clean up after myself. Turns out this folder was explicitly gitignored. I don't think it should be. If someone doesn't want to clean up that way, they can add it to `.git/info/exclude`. (That file seems like it could need some general cleanup, honestly, but that's for another day.)
…=oli-obk PlaceRef's base is already a reference r? @oli-obk
Tests around moving parts of structs and tuples across await points r? cramertj Per the [dropbox paper](https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiR3vlp1s_Kw0yzWZ1sWMnaIAQ-nMyZGrra7dz9KcFRMLKJy) about more tests, it appears there are some tests wanted around local variables (under the section ["Dynamic semantics"](https://paper.dropbox.com/doc/async.await-Call-for-Tests--AiR3vlp1s_Kw0yzWZ1sWMnaIAg-nMyZGrra7dz9KcFRMLKJy#:uid=122335511260129643493892&h2=Dynamic-semantics)). Here is one commit, and I can probably get code up for other scenarios listed there, although I may not have the full background to know what is being targeted by the tests. Please assist me if I'm off course, thanks! --- - Executed all 4 new tests - Executed `tidy` command
…ewjasper doc: the content has since been moved to the guide
…=Mark-Simulacrum Remove unnecessary features from compiler error code list
|
@bors r+ p=9 rollup=never |
|
📌 Commit c8ea26e has been approved by |
bors
added
the
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Aug 7, 2019
Rollup of 9 pull requests Successful merges: - #63034 (Fix generator size regressions due to optimization) - #63035 (Use MaybeUninit to produce more correct layouts) - #63163 (add a pair of whitespace after remove parentheses) - #63294 (tests for async/await drop order) - #63307 (don't ignore mir_dump folder) - #63308 (PlaceRef's base is already a reference) - #63310 (Tests around moving parts of structs and tuples across await points) - #63314 (doc: the content has since been moved to the guide) - #63333 (Remove unnecessary features from compiler error code list) Failed merges: r? @ghost
|
☀️ Test successful - checks-azure |
This was referenced Aug 7, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
Failed merges:
r? @ghost