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.

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

Rollup of 11 pull requests #70202

Closed
wants to merge 52 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

DutchGhost and others added 30 commits March 16, 2020 10:51
This commit introduces 2 methods - `Option::zip` and `Option::zip_with` with
respective signatures:
- zip: `(Option<T>, Option<U>) -> Option<(T, U)>`
- zip_with: `(Option<T>, Option<U>, (T, U) -> R) -> Option<R>`
Both are under the feature gate "option_zip".

I'm not sure about the name "zip", maybe we can find a better name for this.
(I would prefer `union` for example, but this is a keyword :( )

--------------------------------------------------------------------------------

Recently in a russian rust begginers telegram chat a newbie asked (translated):
> Are there any methods for these conversions:
>
> 1. `(Option<A>, Option<B>) -> Option<(A, B)>`
> 2. `Vec<Option<T>> -> Option<Vec<T>>`
>
> ?

While second (2.) is clearly `vec.into_iter().collect::<Option<Vec<_>>()`, the
first one isn't that clear.

I couldn't find anything similar in the `core` and I've come to this solution:
```rust
let tuple: (Option<A>, Option<B>) = ...;
let res: Option<(A, B)> = tuple.0.and_then(|a| tuple.1.map(|b| (a, b)));
```

However this solution isn't "nice" (same for just `match`/`if let`), so I thought
that this functionality should be in `core`.
- remove `#[inline]` attributes (see rust-lang#69997 (comment))
- fill tracking issue in `#[unstable]` attributes
- slightly improve the docs
They used to be covered by `optin_builtin_traits` but negative impls
are now applicable to all traits, not just auto traits.

This also adds docs in the unstable book for the current state of auto traits.
The manual implementation of PartialEq, Eq and Hash for RangeInclusive was functionally equivalent to a derived implementation.

This change removes the manual implementation and adds the respective derives.
A side effect of this change is that the derives also add implementations for StructuralPartialEq and StructuralEq, which enables RangeInclusive to be used in const generics.
The memory fences used previously in Arc implementation are not properly
understood by ThreadSanitizer as synchronization primitives. This had
unfortunate effect where running any non-trivial program compiled with
`-Z sanitizer=thread` would result in numerous false positives.

Replace acquire fences with acquire loads when using ThreadSanitizer to
address the issue.
This simplifies the node manipulation, as we can (in later commits) always know
when traversing nodes that we are not in a shared root.
We no longer have a separate header because the shared root is gone; all code
can work solely with leafs now.
This makes ensure_root_is_owned return a reference to the (now guaranteed to
exist) root, allowing callers to operate on it without going through another
unwrap.

Unfortunately this is only rarely useful as it's frequently the case that both
the length and the root need to be accessed and field-level borrows in methods
don't yet exist.
RalfJung and others added 19 commits March 20, 2020 17:25
remove redundant format!() call (clippy::useless_format)
don't use ok() before calling expect() (clippy::ok_expect)
Make std::sync::Arc compatible with ThreadSanitizer

The memory fences used previously in Arc implementation are not properly
understood by thread sanitizer as synchronization primitives. This had
unfortunate effect where running any non-trivial program compiled with
`-Z sanitizer=thread` would result in numerous false positives.

Replace acquire fences with acquire loads to address the issue.

Fixes rust-lang#39608.
permit negative impls for non-auto traits

This is a prototype impl that extends `impl !Trait` beyond auto traits. It is not integrated with coherence or anything else, and hence only serves to prevent downstream impls (but not to allow downstream crates to rely on the absence of such impls for coherence purposes).

Fixes rust-lang#66544

TODO:

- [x] need a test that you can't rely on negative impls for coherence purposes
- [x] test that negative impls cannot specialize positive ones
- [x] test that positive impls cannot specialize negative ones
- [x] extend negative impl to `Clone` in order to fully fix rust-lang#66544
- [x] and maybe make `CoerceUnsized` unsafe? -- that problem is now split out into rust-lang#68015
- [x] introduce feature flag and prepare a write-up
- [x] improve diagnostics?
add #[rustc_layout(debug)]

@eddyb recently told me about the `#[rustc_layout]` attribute, and I think it would be very useful if it could be used to print all the layout information Rust has about a type. When working with layouts (e.g. in Miri), it is often not clear how certain surface language features get represented internally. I have some awful hacks locally to be able to dump this debug information; with this attribute I could get it on the playground which is so much better. :)
…bertodt

add `Option::{zip,zip_with}` methods under "option_zip" gate

This PR introduces 2 methods - `Option::zip` and `Option::zip_with` with
respective signatures:
- zip: `(Option<T>, Option<U>) -> Option<(T, U)>`
- zip_with: `(Option<T>, Option<U>, (T, U) -> R) -> Option<R>`
Both are under the feature gate "option_zip".

I'm not sure about the name "zip", maybe we can find a better name for this.
(I would prefer `union` for example, but this is a keyword :( )

--------------------------------------------------------------------------------

Recently in a russian rust begginers telegram chat a newbie asked (translated):
> Are there any methods for these conversions:
>
> 1. `(Option<A>, Option<B>) -> Option<(A, B)>`
> 2. `Vec<Option<T>> -> Option<Vec<T>>`
>
> ?

While second (2.) is clearly `vec.into_iter().collect::<Option<Vec<_>>()`, the
first one isn't that clear.

I couldn't find anything similar in the `core` and I've come to this solution:
```rust
let tuple: (Option<A>, Option<B>) = ...;
let res: Option<(A, B)> = tuple.0.and_then(|a| tuple.1.map(|b| (a, b)));
```

However this solution isn't "nice" (same for just `match`/`if let`), so I thought
that this functionality should be in `core`.
…cuviper

BTreeMap: remove shared root

This replaces the shared root with `Option`s in the BTreeMap code, and then slightly cleans up the node manipulation code taking advantage of the removal of the shared root. I expect that further simplification is possible, but wanted to get this posted for initial review.

Note that `BTreeMap::new()` continues to not allocate.

Benchmarks seem within the margin of error/unaffected, as expected for an entirely predictable branch.

```
 name                                 alloc-bench-a ns/iter  alloc-bench-b ns/iter  diff ns/iter  diff %  speedup
 btree::map::iter_mut_20              20                     21                                1   5.00%   x 0.95
 btree::set::clone_100                1,360                  1,439                            79   5.81%   x 0.95
 btree::set::clone_100_and_into_iter  1,319                  1,434                           115   8.72%   x 0.92
 btree::set::clone_10k                143,515                150,991                       7,476   5.21%   x 0.95
 btree::set::clone_10k_and_clear      142,792                152,916                      10,124   7.09%   x 0.93
 btree::set::clone_10k_and_into_iter  146,019                154,561                       8,542   5.85%   x 0.94
```
…ramertj

Derive PartialEq, Eq and Hash for RangeInclusive

The manual implementation of `PartialEq`, `Eq` and `Hash` for `RangeInclusive` was functionally equivalent to a derived implementation.

This change removes the manual implementation and adds the respective derives.
A side effect of this change is that the derives also add implementations for `StructuralPartialEq` and `StructuralEq`, which enables `RangeInclusive` to be used in const generics, closing rust-lang#70155.

This change is enabled by rust-lang#68835, which changed the field `is_empty: Option<bool>` to `exhausted: bool` removing the need for *semantic* equality instead of *structural* equality.

## PartialEq
original [`PartialEq`](https://github.com/rust-lang/rust/blob/f4c675c476c18b1a11041193f2f59d695b126bc8/src/libcore/ops/range.rs#L353-L359) implementation:
```rust
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: PartialEq> PartialEq for RangeInclusive<Idx> {
    #[inline]
    fn eq(&self, other: &Self) -> bool {
        self.start == other.start && self.end == other.end && self.exhausted == other.exhausted
    }
}
```
expanded derive implementation (using `cargo expand ops::range`):
```rust
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx> crate::marker::StructuralPartialEq for RangeInclusive<Idx> {}

#[automatically_derived]
#[allow(unused_qualifications)]
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: crate::cmp::PartialEq> crate::cmp::PartialEq for RangeInclusive<Idx> {
    #[inline]
    fn eq(&self, other: &RangeInclusive<Idx>) -> bool {
        match *other {
            RangeInclusive { start: ref __self_1_0,end: ref __self_1_1, exhausted: ref __self_1_2 } => match *self {
                RangeInclusive { start: ref __self_0_0, end: ref __self_0_1, exhausted: ref __self_0_2 } => {
                    (*__self_0_0) == (*__self_1_0) && (*__self_0_1) == (*__self_1_1) && (*__self_0_2) == (*__self_1_2)
                }
            },
        }
    }
    #[inline]
    fn ne(&self, other: &RangeInclusive<Idx>) -> bool {
        match *other {
            RangeInclusive { start: ref __self_1_0, end: ref __self_1_1, exhausted: ref __self_1_2 } => match *self {
                RangeInclusive { start: ref __self_0_0, end: ref __self_0_1exhausted: ref __self_0_2 } => {
                    (*__self_0_0) != (*__self_1_0) || (*__self_0_1) != (*__self_1_1) || (*__self_0_2) != (*__self_1_2)
                }
            },
        }
    }
}
```

These implementations both test for *structural* equality, with the same order of field comparisons, and the bound `Idx: PartialEq` is the same.
## Eq
original [`Eq`](https://github.com/rust-lang/rust/blob/f4c675c476c18b1a11041193f2f59d695b126bc8/src/libcore/ops/range.rs#L361-L362) implementation:
```rust
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: Eq> Eq for RangeInclusive<Idx> {}
```
expanded derive implementation (using `cargo expand ops::range`):
```rust
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx> crate::marker::StructuralEq for RangeInclusive<Idx> {}

#[automatically_derived]
#[allow(unused_qualifications)]
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: crate::cmp::Eq> crate::cmp::Eq for RangeInclusive<Idx> {
    #[inline]
    #[doc(hidden)]
    fn assert_receiver_is_total_eq(&self) -> () {
        {
            let _: crate::cmp::AssertParamIsEq<Idx>;
            let _: crate::cmp::AssertParamIsEq<Idx>;
            let _: crate::cmp::AssertParamIsEq<bool>;
        }
    }
}
```
These implementations are equivalent since `Eq` is just a marker trait and the bound `Idx: Eq` is the same.

## Hash
original [`Hash`](https://github.com/rust-lang/rust/blob/f4c675c476c18b1a11041193f2f59d695b126bc8/src/libcore/ops/range.rs#L364-L371) implementation:
```rust
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: Hash> Hash for RangeInclusive<Idx> {
    fn hash<H: Hasher>(&self, state: &mut H) {
        self.start.hash(state);
        self.end.hash(state);
        self.exhausted.hash(state);
    }
}
```
expanded derive implementation (using `cargo expand ops::range`):
```rust
#[automatically_derived]
#[allow(unused_qualifications)]
#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<Idx: crate::hash::Hash> crate::hash::Hash for RangeInclusive<Idx> {
    fn hash<__H: crate::hash::Hasher>(&self, state: &mut __H) -> () {
        match *self { RangeInclusive { start: ref __self_0_0, end: ref __self_0_1, exhausted: ref __self_0_2 } => {
                crate::hash::Hash::hash(&(*__self_0_0), state);
                crate::hash::Hash::hash(&(*__self_0_1), state);
                crate::hash::Hash::hash(&(*__self_0_2), state)
            }
        }
    }
}
```
These implementations are functionally equivalent, with the same order of field hashing, and the bound `Idx: Hash` is the same.
Fix oudated comment for NamedRegionMap

`ResolveLifetimes` uses a `LocalDefId` since rust-lang#66131.
…etrochenkov

expand_include: set `.directory` to dir of included file.

Resolves the regression noted in rust-lang#69838.

r? @petrochenkov
cc @eddyb @Mark-Simulacrum
more clippy fixes

    * remove redundant returns (clippy::needless_return)
    * remove redundant import (clippy::single_component_path_imports)
    * remove redundant format!() call (clippy::useless_format)
    * don't use ok() before calling expect() (clippy::ok_expect)
@Dylan-DPC-zz
Copy link
Author

@bors r+ p=11 rollup=never

@bors
Copy link
Contributor

bors commented Mar 21, 2020

📌 Commit cb62d67 has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 21, 2020
@bors
Copy link
Contributor

bors commented Mar 21, 2020

⌛ Testing commit cb62d67 with merge 35e99d4be6e7a4e7fc5c2afd74abec5e8cf6323b...

@Dylan-DPC-zz Dylan-DPC-zz added the rollup A PR which is a rollup label Mar 21, 2020
@bors
Copy link
Contributor

bors commented Mar 21, 2020

💔 Test failed - checks-azure

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 21, 2020
@Centril Centril closed this Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.