Skip to content

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
Collaborator

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
Collaborator

bors commented May 6, 2020

⌛ Testing commit bd89441 with merge 99210d17f08a376cad26499df3f432d8c4816f0f...

@bors
Copy link
Collaborator

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
Collaborator

bors commented May 6, 2020

⌛ Testing commit bd89441 with merge 6bb9ccf6d5905a070febcf0f119c805b3410ccb2...

@bors
Copy link
Collaborator

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