Remove unzip() SizeHint hack
This was using an invalid iterator so is likely to end with buggy
behaviour.
It also doesn't even benefit many type in std including Vec so removing it
shouldn't cause any problems.
Fixes: #33468
Specialize .zip() for efficient slice and slice iteration
The idea is to introduce a private trait TrustedRandomAccess and specialize .zip() for random access iterators into a counted loop.
The implementation in the PR is internal and has no visible effect in the API
Why a counted loop? To have each slice iterator compile to just a pointer, and both pointers are indexed with the same loop counter value in the generated code. When this succeeds, copying loops are readily recognized and replaced with memcpy and addition loops autovectorize well.
The TrustedRandomAccess approach works very well on the surface. Microbenchmarks optimize well, following the ideas above, and that is a dramatic improvement of .zip()'s codegen.
```rust
// old zip before this PR: bad, byte-for-byte loop
// with specialized zip: memcpy
pub fn copy_zip(xs: &[u8], ys: &mut [u8]) {
for (a, b) in ys.iter_mut().zip(xs) {
*a = *b;
}
}
// old zip before this PR: single addition per iteration
// with specialized zip: vectorized
pub fn add_zip(xs: &[f32], ys: &mut [f32]) {
for (a, b) in ys.iter_mut().zip(xs) { *a += *b; }
}
// old zip before this PR: single addition per iteration
// with specialized zip: vectorized (!!)
pub fn add_zip3(xs: &[f32], ys: &[f32], zs: &mut [f32]) {
for ((a, b), c) in zs.iter_mut().zip(xs).zip(ys) { *a += *b * *c; }
}
```
Yet in more complex situations, the .zip() loop can still fall back to its old behavior where phantom null checks throw in fake premature end of the loop conditionals. Remember that a NULL inside
Option<(&T, &T)> makes it a `None` value and a premature (in this case)
end of the loop.
So even if we have 1) an explicit `Some` in the code and 2) the types of the pointers are `&T` or `&mut T` which are nonnull, we can still get a phantom null check at that point.
One example that illustrates the difference is `copy_zip` with slice versus Vec arguments. The involved iterator types are exactly the same, but the Vec version doesn't compile down to memcpy. Investigating into this, the function argument metadata emitted to llvm plays the biggest role. As eddyb summarized, we need nonnull for the loop to autovectorize and noalias for it to replace with memcpy.
There was an experiment to use `assume` to add a non-null assumption on each of the two elements in the specialized zip iterator, but this only helped in some of the test cases and regressed others. Instead I think the nonnull/noalias metadata issue is something we need to solve separately anyway.
These have conditionally implemented TrustedRandomAccess
- Enumerate
- Zip
These have not implemented it
- Map is sideeffectful. The forward case would be workable, but the double ended case is complicated.
- Chain, exact length semantics unclear
- Filter, FilterMap, FlatMap and many others don't offer random access and/or exact length
derive Hash (and not Copy) for ranges
Fixes#34170.
Also, `RangeInclusive` was `Copy` by mistake -- fix that, which is a [breaking-change] to that unstable type.
docs: simplify wording
It took me more then a moment to decipher "with no non-`'static`" thing :)
"`'static` type" should say the same thing more clearly.
r? @steveklabnik
Fix wrong statement in compare_exchange doc
The documentation for `core::sync::atomic::AtomicSomething::compare_exchange` contains a wrong, or imprecise, statement about the return value. It goes:
The return value is a result indicating whether the new value was written and containing
the previous value. On success this value is guaranteed to be equal to `new`.
In the second sentence, `this value` is gramatically understood as referring to `return value` from the first sentence. Due to how CAS works, the returned value is always what was in the atomic variable _before_ the operation occurred, not what was written into it during the operation. Hence, the fixed doc should say:
The return value is a result indicating whether the new value was written and containing
the previous value. On success this value is guaranteed to be equal to `current`.
This version is confirmed by the runnable examples in variants of `AtomicSomething`, e.g.
assert_eq!(some_bool.compare_exchange(true, false, Ordering::Acquire, Ordering::Relaxed),
Ok(true));
where the returned value is `Ok(current)`. This PR fixes all occurrences of this bug I could find.
An alternative solution would be to modify the second sentence so that it refers to the value _written_ into the Atomic rather than what was there before, in which case it would be correct. Example alternative formulation:
On success the value written into the `bool`/`usize`/`whatever` is guaranteed to be equal to `new`.
r? @steveklabnik
This was using an invalid iterator so is likely to end with buggy
behaviour.
It also doesn't even benefit many type in std including Vec so removing it
shouldn't cause any problems.
Support 16-bit pointers as well as i/usize
I'm opening this pull request to get some feedback from the community.
Although Rust doesn't support any platforms with a native 16-bit pointer at the moment, the [AVR-Rust][ar] fork is working towards that goal. Keeping this forked logic up-to-date with the changes in master has been onerous so I'd like to merge these changes so that they get carried along when refactoring happens. I do not believe this should increase the maintenance burden.
This is based on the original work of Dylan McKay (@dylanmckay).
[ar]: https://github.com/avr-rust/rust
core: check pointer equality when comparing byte slices
If pointer address and length are the same, it should be the same slice.
In experiments, I've seen that this doesn't happen as often in debug builds, but release builds seem to optimize to using a single pointer more often.
Implement `count` for `EscapeUnicode`
and cleanup the code for `count` for `EscapeDefault` (instead of repeating the `match` for `size_hint` and `count`).
This PR marks EscapeUnicode and EscapeDefault as ExactSizeIterator. The constraints for the trait implementations held even before this PR, but I am not sure if this is something we want to guarantee/expose (I would love feedback on this, especially on what would be the appropriate way to handle stabilisation, if needed).
Part of #24214, split from #31049.
The test for `count` was added in #33103.
Trait documentation clarifications
Hi! I've felt a bit of friction lately in figuring out how to write custom implementations of the `derive`able traits, so I decided to add to the docs :)
The docs for `Copy` are already excellent-- clear, useful sections that I only reordered a bit-- they're now:
* General explanation
* When can my type be `Copy`?
* When can my type _not_ be `Copy`?
* When should my type be `Copy`?
* Derivable
* How can I implement `Copy`?
I didn't add all these sections for all the traits, but I did make sure all the derivable traits had a consistent "Derivable" section that explained what the derived implementation does and a "How can I implement" section that has an example.
Please check me for correctness-- I tried to do research to make sure I was saying accurate things but I'm still learning! ❤️ I'd also love suggestions on information to add that is still missing-- I think these traits are important and deserve to have awesome docs!
Fixes to mir dataflow
Fixes to mir dataflow
This collects a bunch of changes to `rustc_borrowck::borrowck::dataflow` (which others have pointed out should probably migrate to some crate that isn't tied to the borrow-checker -- but I have not attempted that here, especially since there are competing approaches to dataflow that we should also evaluate).
These changes:
1. Provide a family of related analyses: MovingOutStatements (which is what the old AST-based dataflo computed), as well as MaybeInitialized, MaybeUninitalized, and DefinitelyInitialized.
* (The last two are actually inverses of each other; we should pick one and drop the other.)
2. Fix bugs in the pre-existing analysis implementation, which was untested and thus some obvious bugs went unnoticed, which brings us to the third point:
3. Add a unit test infrastructure for the MIR dataflow analysis.
* The tests work by adding a new intrinsic that is able to query the analysis state for a particular expression (technically, a particular L-value).
* See the examples in compile-fail/mir-dataflow/inits-1.rs and compile-fail/mir-dataflow/uninits-1.rs
* These tests are only checking the results for MaybeInitialized, MaybeUninitalized, and DefinitelyInitialized; I am not sure if it will be feasible to generalize this testing strategy to the MovingOutStatements dataflow operator.
