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 6 pull requests #71934

Closed
wants to merge 23 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

ssomers and others added 23 commits April 25, 2020 00:05
This also abstracts checking for a command into `require`.

Before:

```
Updating only changed submodules
Submodules updated in 0.01 seconds
Traceback (most recent call last):
  File "./x.py", line 11, in <module>
    bootstrap.main()
  ...
  File "/home/joshua/src/rust/src/bootstrap/bootstrap.py", line 137, in run
    ret = subprocess.Popen(args, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 394, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1047, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
```

After:

```
error: unable to run `curl --version`: [Errno 2] No such file or directory
Please make sure it's installed and in the path.
```
When encountering a binary operation involving a type parameter that has
no bindings, suggest adding the appropriate bound.
When encountering a projection that isn't satisfied by a type parameter,
suggest constraining the type parameter.
…ropriate syntax

When encountering `where <A as Foo>::Bar = B`, it is possible that `Bar`
is an associated type. If so, suggest `where A: Foo<Bar = B>`.

CC rust-lang#20041.
Provide suggestions for type parameters missing bounds for associated types

When implementing the binary operator traits it is easy to forget to restrict the `Output` associated type. `rustc` now accounts for different cases to lead users in the right direction to add the necessary restrictions. The structured suggestions in the following output are new:

```
error: equality constraints are not yet supported in `where` clauses
  --> $DIR/missing-bounds.rs:37:33
   |
LL | impl<B: Add> Add for E<B> where <B as Add>::Output = B {
   |                                 ^^^^^^^^^^^^^^^^^^^^^^ not supported
   |
   = note: see issue rust-lang#20041 <rust-lang#20041> for more information
help: if `Output` is an associated type you're trying to set, use the associated type binding syntax
   |
LL | impl<B: Add> Add for E<B> where B: Add<Output = B> {
   |                                 ^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
  --> $DIR/missing-bounds.rs:11:11
   |
7  | impl<B> Add for A<B> where B: Add {
   |      - this type parameter
...
11 |         A(self.0 + rhs.0)
   |           ^^^^^^^^^^^^^^ expected type parameter `B`, found associated type
   |
   = note: expected type parameter `B`
             found associated type `<B as std::ops::Add>::Output`
help: consider further restricting this bound
   |
7  | impl<B> Add for A<B> where B: Add + std::ops::Add<Output = B> {
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0369]: cannot add `B` to `B`
  --> $DIR/missing-bounds.rs:31:21
   |
31 |         Self(self.0 + rhs.0)
   |              ------ ^ ----- B
   |              |
   |              B
   |
help: consider restricting type parameter `B`
   |
27 | impl<B: std::ops::Add<Output = B>> Add for D<B> {
   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

That output is given for the following cases:

```rust
struct A<B>(B);
impl<B> Add for A<B> where B: Add {
    type Output = Self;

    fn add(self, rhs: Self) -> Self {
        A(self.0 + rhs.0) //~ ERROR mismatched types
    }
}

struct D<B>(B);
impl<B> Add for D<B> {
    type Output = Self;

    fn add(self, rhs: Self) -> Self {
        Self(self.0 + rhs.0) //~ ERROR cannot add `B` to `B`
    }
}

struct E<B>(B);
impl<B: Add> Add for E<B> where <B as Add>::Output = B {
    type Output = Self;

    fn add(self, rhs: Self) -> Self {
        Self(self.0 + rhs.0)
    }
}
```
…r=Mark-Simulacrum

Btreemap iter intertwined

3 commits:

1. Introduced benchmarks for `BTreeMap::iter()`. Benchmarks named `iter_20` were of the whole iteration process, so I renamed them. Also the benchmarks of `range` that I wrote earlier weren't very good. I included an (awkwardly named) one that compares `iter()` to `range(..)` on the same set, because the contrast is surprising:
```
 name                                           ns/iter
 btree::map::range_unbounded_unbounded          28,176
 btree::map::range_unbounded_vs_iter            89,369
```
Both dig up the same pair of leaf edges. `range(..)` also checks that some keys are correctly ordered, the only thing `iter()` does more is to copy the map's length.

2. Slightly refactoring the code to what I find more readable (not in chronological order of discovery), boosts performance:
```
>cargo-benchcmp.exe benchcmp a1 a2 --threshold 5
 name                                   a1 ns/iter  a2 ns/iter  diff ns/iter   diff %  speedup
 btree::map::find_rand_100              18          17                    -1   -5.56%   x 1.06
 btree::map::first_and_last_10k         64          71                     7   10.94%   x 0.90
 btree::map::iter_0                     2,939       2,209               -730  -24.84%   x 1.33
 btree::map::iter_1                     6,845       2,696             -4,149  -60.61%   x 2.54
 btree::map::iter_100                   8,556       3,672             -4,884  -57.08%   x 2.33
 btree::map::iter_10k                   9,292       5,884             -3,408  -36.68%   x 1.58
 btree::map::iter_1m                    10,268      6,510             -3,758  -36.60%   x 1.58
 btree::map::iteration_mut_100000       478,575     453,050          -25,525   -5.33%   x 1.06
 btree::map::range_unbounded_unbounded  28,176      36,169             7,993   28.37%   x 0.78
 btree::map::range_unbounded_vs_iter    89,369      38,290           -51,079  -57.16%   x 2.33
 btree::set::clone_100_and_remove_all   4,801       4,245               -556  -11.58%   x 1.13
 btree::set::clone_10k_and_remove_all   529,450     496,030          -33,420   -6.31%   x 1.07
```
But you can tell from the `range_unbounded_*` lines that, despite an unwarranted, vengeful attack on the range_unbounded_unbounded benchmark, this change still doesn't allow `iter()` to catch up with `range(..)`.

3. I guess that `range(..)` copes so well because it intertwines the leftmost and rightmost descend towards leaf edges, doing the two root node accesses close together, perhaps exploiting a CPU's internal pipelining? So the third commit distils a version of `range_search` (which we can't use directly because of the `Ord` bound), and we get another boost:
```
cargo-benchcmp.exe benchcmp a2 a3 --threshold 5
 name                                   a2 ns/iter  a3 ns/iter  diff ns/iter   diff %  speedup
 btree::map::first_and_last_100         40          43                     3    7.50%   x 0.93
 btree::map::first_and_last_10k         71          64                    -7   -9.86%   x 1.11
 btree::map::iter_0                     2,209       1,719               -490  -22.18%   x 1.29
 btree::map::iter_1                     2,696       2,205               -491  -18.21%   x 1.22
 btree::map::iter_100                   3,672       2,943               -729  -19.85%   x 1.25
 btree::map::iter_10k                   5,884       3,929             -1,955  -33.23%   x 1.50
 btree::map::iter_1m                    6,510       5,532               -978  -15.02%   x 1.18
 btree::map::iteration_mut_100000       453,050     476,667           23,617    5.21%   x 0.95
 btree::map::range_included_excluded    405,075     371,297          -33,778   -8.34%   x 1.09
 btree::map::range_included_included    427,577     397,440          -30,137   -7.05%   x 1.08
 btree::map::range_unbounded_unbounded  36,169      28,175            -7,994  -22.10%   x 1.28
 btree::map::range_unbounded_vs_iter    38,290      30,838            -7,452  -19.46%   x 1.24
```
But I think this is just fake news from the microbenchmarking media. `iter()` is still trying to catch up with `range(..)`. And we can sure do without another function. So I would skip this 3rd commit.

r? @Mark-Simulacrum
…lacrum

SipHasher with keys initialized to 0 should just use new()

I believe that is what the `new()` is for, for good reasons.
…lacrum

x.py: Give a more helpful error message if curl isn't installed

Before:

```
Updating only changed submodules
Submodules updated in 0.01 seconds
Traceback (most recent call last):
  File "./x.py", line 11, in <module>
    bootstrap.main()
  ...
  File "/home/joshua/src/rust/src/bootstrap/bootstrap.py", line 137, in run
    ret = subprocess.Popen(args, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 394, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1047, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
```

After:

```
Updating only changed submodules
Submodules updated in 0.01 seconds

spurious failure, trying again

spurious failure, trying again

spurious failure, trying again

spurious failure, trying again
failed to run: curl -s -y 30 -Y 10 --connect-timeout 30 --retry 3 -Sf -o /tmp/tmpSWF21P.sha256 https://static.rust-lang.org/dist/2020-04-22/rust-std-beta-x86_64-unknown-linux-gnu.tar.gz.sha256: [Errno 2] No such file or directory
Build completed unsuccessfully in 0:00:00
```
Explain our RwLock implementation

Turns out that [with the latest POSIX docs](https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_wrlock.html), our `RwLock` implementation is actually correct. However, we cannot fully rely on that due to bugs in older glibc (fix released in 2016). Update the comments to explain that.

I also clarified our Mutex docs a bit and fixed another instance of rust-lang#55865.

r? @Amanieu
Fixes rust-lang#53127
…crum

Add command aliases from Cargo to x.py commands

Fixes rust-lang#71357
@Dylan-DPC-zz
Copy link
Author

@bors r+ rollup=nevre p=6

@bors
Copy link
Contributor

bors commented May 6, 2020

📌 Commit bd89441 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 May 6, 2020
@Dylan-DPC-zz
Copy link
Author

@bors rollup=never

@Dylan-DPC-zz
Copy link
Author

@bors p=6

@bors
Copy link
Contributor

bors commented May 6, 2020

⌛ Testing commit bd89441 with merge 99210d17f08a376cad26499df3f432d8c4816f0f...

@bors
Copy link
Contributor

bors commented May 6, 2020

💔 Test failed - checks-actions

@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 May 6, 2020
@Dylan-DPC-zz
Copy link
Author

@bors retry

@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 May 6, 2020
@bors
Copy link
Contributor

bors commented May 6, 2020

⌛ Testing commit bd89441 with merge 6bb9ccf6d5905a070febcf0f119c805b3410ccb2...

@bors
Copy link
Contributor

bors commented May 6, 2020

💔 Test failed - checks-actions

@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 May 6, 2020
@Dylan-DPC-zz Dylan-DPC-zz deleted the rollup-71oigdi branch May 6, 2020 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

9 participants