bors [Wed, 7 Aug 2019 00:45:00 +0000 (00:45 +0000)]
Auto merge of #63341 - Centril:rollup-hkhxahb, r=Centril
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)
Rollup merge of #63310 - gorup:partial-moves, r=cramertj
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
Rollup merge of #63307 - RalfJung:gitignore, r=alexcrichton
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.)
Rollup merge of #63294 - alsuren:async-tests, r=cramertj
tests for async/await drop order
This is just me helping out with https://github.com/rust-lang/rust/issues/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 https://github.com/rust-lang/rust/pull/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.
* c4940e0f90 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.
Rollup merge of #63034 - tmandry:reduce-generator-size-regressions, r=cramertj
Fix generator size regressions due to optimization
I tested the generator optimizations in #60187 and #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 #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.
Fix generator size regressions due to optimization
I tested the generator optimizations in #60187 and #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 #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.
bors [Tue, 6 Aug 2019 17:19:05 +0000 (17:19 +0000)]
Auto merge of #61515 - shepmaster:boxed-slice-to-array, r=cramertj
Add implementations for converting boxed slices into boxed arrays
This mirrors the implementations of reference slices into arrays.
# Discussion
- [x] Should we use const generics? ([probably not](https://github.com/rust-lang/rust/pull/61515#issuecomment-498690649))
- [ ] [What's the safety rationale here](https://github.com/rust-lang/rust/pull/61515#discussion_r290296613)?
- [ ] [Should the errors return the original object](https://github.com/rust-lang/rust/pull/61515#discussion_r290336592)?
bors [Tue, 6 Aug 2019 13:37:22 +0000 (13:37 +0000)]
Auto merge of #63328 - Centril:rollup-482ujaf, r=Centril
Rollup of 6 pull requests
Successful merges:
- #62459 (Use internal iteration in the Sum and Product impls of Result and Option)
- #62821 (Not listed methods)
- #62837 (Fix theme picker blur handler: always hide instead of switching)
- #63286 (Replace error callback with Result)
- #63296 (Deduplicate rustc_demangle in librustc_codegen_llvm)
- #63298 (assume_init: warn about valid != safe)
Rollup merge of #63296 - alexcrichton:deduplicate-demangle, r=Mark-Simulacrum
Deduplicate rustc_demangle in librustc_codegen_llvm
This commit removes the crates.io dependency of `rustc-demangle` from
`rustc_codegen_llvm`. This crate is actually already pulled in to part
of the `librustc_driver` build and with the upcoming pipelining
implementation in Cargo it causes build issues if `rustc-demangle` is
left to its own devices.
This is not currently required, but once pipelining is enabled for
rustc's own build it will be required to build correctly.
Rollup merge of #62837 - Kinrany:patch-1, r=GuillaumeGomez
Fix theme picker blur handler: always hide instead of switching
Fixes a minor bug in UI generated by rustdoc.
For example, this page: https://doc.rust-lang.org/std/
Reproduction steps:
1. Click the theme picker twice
* The list of themes will be shown and then hidden
2. Click anywhere else
* The list of themes will be show again, which is unexpected
The bug was caused by blur event handler toggling the state of the element instead of always hiding it regardless of the current state.
Rollup merge of #62459 - timvermeulen:result_sum_internal_iteration, r=scottmcm
Use internal iteration in the Sum and Product impls of Result and Option
This PR adds internal iteration to the `ResultShunt` iterator type underlying the `Sum` and `Product` impls of `Result`. I had to change `ResultShunt` to hold a mutable reference to an error instead, similar to `itertools::ProcessResults`, in order to be able to pass the `ResultShunt` itself by value (which is necessary for internal iteration).
`ResultShunt::process` can unfortunately no longer be an associated function because that would make it generic over the lifetime of the error reference, which wouldn't work, so I turned it into the free function `process_results`.
I removed the `OptionShunt` type and forwarded the `Sum` and `Product` impls of `Option` to their respective impls of `Result` instead, to avoid having to repeat the internal iteration logic.
That PR makes error messages worse than before, and we couldn't come up with a way of actually making them better, so revert it for now. Any idea for making this error message better is welcome!
Rollup merge of #63230 - tmandry:disallow-possibly-uninitialized, r=Centril
Make use of possibly uninitialized data [E0381] a hard error
This is one of the behaviors we no longer allow in NLL. Since it can
lead to undefined behavior, I think it's definitely worth making it a
hard error without waiting to turn off migration mode (#58781).
Closes #60450.
My ulterior motive here is making it impossible to leave variables
partially initialized across a yield (see #60889, discussion at #63035), so
tests are included for that.
cc #54987
---
I'm not sure if bypassing the buffer is a good way of doing this. We could also make a `force_errors_buffer` or similar that gets recombined with all the errors as they are emitted. But this is simpler and seems fine to me.
r? @Centril
cc @cramertj @nikomatsakis @pnkfelix @RalfJung
Rollup merge of #63184 - JasonShin:master, r=sfackler
Explaining the reason why validation is performed in to_str of path.rs
I thought it's good to explain the reason for the validation during the conversion between Path/PathBuffer into str, which explains the reason for returning an Option at this point (good for beginners who are reading through the docs).
Rollup merge of #63017 - matklad:no-fatal, r=petrochenkov
Remove special code-path for handing unknown tokens
In `StringReader`, we have a buffer of fatal errors, which is used only in a single case: when we see something which is not a reasonable token at all, like `🦀`. I think a more straightforward thing to do here is to produce an explicit error token in this case, and let the next layer (the parser), deal with it.
However currently this leads to duplicated error messages. What should we do with this? Naively, I would think that emitting (just emitting, not raising) `FatalError` should stop other errors, but looks like this is not the case? We can also probably tweak parser on the case-by-case basis, to avoid emitting "expected" errors if the current token is an `Err`. I personally also fine with cascading errors in this case: it's quite unlikely that you actually type a fully invalid token.
@petrochenkov, which approach should we take to fight cascading errors?
Rollup merge of #61457 - timvermeulen:double_ended_iters, r=scottmcm
Implement DoubleEndedIterator for iter::{StepBy, Peekable, Take}
Now that `DoubleEndedIterator::nth_back` has landed, `StepBy` and `Take` can have an efficient `DoubleEndedIterator` implementation. I don't know if there was any particular reason for `Peekable` not having a `DoubleEndedIterator` implementation, but it's quite trivial and I don't see any drawbacks to having it.
I'm not very happy about the implementation of `Peekable::try_rfold`, but I didn't see another way to only take the value out of `self.peeked` in case `self.iter.try_rfold` didn't exit early.
I only added `Peekable::rfold` (in addition to `try_rfold`) because its `Iterator` implementation has both `fold` and `try_fold` (and for similar reasons I added `Take::try_rfold` but not `Take::rfold`). Do we have any guidelines on whether we want both? If we do want both, maybe we should investigate which iterator adaptors override `try_fold` but not `fold` and add the missing implementations. At the moment I think that it's better to always have iterator adaptors implement both, because some iterators have a simpler `fold` implementation than their `try_fold` implementation.
The tests that I added may not be sufficient because they're all just existing tests where `next`/`nth`/`fold`/`try_fold` are replaced by their DEI counterparts, but I do think all paths are covered. Is there anything in particular that I should probably also test?
bors [Tue, 6 Aug 2019 04:43:03 +0000 (04:43 +0000)]
Auto merge of #62987 - Thomasdezeeuw:ioslice-advance, r=Thomasdezeeuw
Add {IoSlice, IoSliceMut}::advance
API inspired by the [`Buf::advance`](https://docs.rs/bytes/0.4.12/bytes/trait.Buf.html#tymethod.advance) method found in the [bytes](https://docs.rs/bytes) crate.
Tyler Mandry [Sat, 3 Aug 2019 00:08:16 +0000 (17:08 -0700)]
Make use of possibly uninitialized data a hard error
This is one of the behaviors we no longer allow in NLL. Since it can
lead to undefined behavior, I think it's definitely worth making it a
hard error without waiting to turn off migration mode (#58781).
Closes #60450.
My ulterior motive here is making it impossible to leave variables
partially initialized across a yield (see discussion at #63035), so
tests are included for that.
bors [Mon, 5 Aug 2019 04:36:51 +0000 (04:36 +0000)]
Auto merge of #63248 - petrochenkov:nomarker, r=matthewjasper
Move special treatment of `derive(Copy, PartialEq, Eq)` from expansion infrastructure to elsewhere
As described in https://github.com/rust-lang/rust/pull/62086#issuecomment-515195477.
Reminder:
- `derive(PartialEq, Eq)` makes the type it applied to a "structural match" type, so constants of this type can be used in patterns (and const generics in the future).
- `derive(Copy)` notifies other derives that the type it applied to implements `Copy`, so `derive(Clone)` can generate optimized code and other derives can generate code working with `packed` types and types with `rustc_layout_scalar_valid_range` attributes.
First, the special behavior is now enabled after properly resolving the derives, rather than after textually comparing them with `"Copy"`, `"PartialEq"` and `"Eq"` in `fn add_derived_markers`.
The markers are no longer kept as attributes in AST since derives cannot modify items and previously did it through hacks in the expansion infra.
Instead, the markers are now kept in a "global context" available from all the necessary places, namely - resolver.
For `derive(PartialEq, Eq)` the markers are created by the derive macros themselves and then consumed during HIR lowering to add the `#[structural_match]` attribute in HIR.
This is still a hack, but now it's a hack local to two specific macros rather than affecting the whole expansion infra.
Ideally we should find the way to put `#[structural_match]` on the impls rather than on the original item, and then consume it in `rustc_mir`, then no hacks in expansion and lowering will be required.
(I'll make an issue about this for someone else to solve, after this PR lands.)
The marker for `derive(Copy)` cannot be emitted by the `Copy` macro itself because we need to know it *before* the `Copy` macro is expanded for expanding other macros.
So we have to do it in resolve and block expansion of any derives in a `derive(...)` container until we know for sure whether this container has `Copy` in it or not.
Nasty stuff.
projects / rust.git / log