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 9 pull requests #72203

Closed
wants to merge 31 commits into from
Closed

Conversation

RalfJung
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

matthewjasper and others added 30 commits April 26, 2020 16:27
- Remove a type parameter from `[A]RcFromIter`.
- Remove an implementation of `[A]RcFromIter` that didn't actually
  specialize anything.
- Remove unused implementation of `IsZero` for `Option<&mut T>`.
- Change specializations of `[A]RcEqIdent` to use a marker trait version
of `Eq`.
- Remove `BTreeClone`. I couldn't find a way to make this work with
  `min_specialization`.
- Add `rustc_unsafe_specialization_marker` to `Copy` and `TrustedLen`.
Co-Authored-By: Oliver Scherer <[email protected]>
For the given code paths, the amount of space used in the previous chunk
is irrelevant.

(This will almost never make a difference to behaviour, but it makes the
code clearer.)
`DroplessArena` and `TypedArena` use an aggressive growth strategy: the
first chunk is 4 KiB, the second is 8 KiB, and it keeps on doubling
indefinitely. DHAT profiles show that sometimes this results in large
chunks (e.g. 16-128 MiB) that are barely filled. Although these don't
contribute to RSS, they clog up the DHAT profiles.

This commit changes things so that the doubling stops at 2 MiB. This is
large enough that chunk allocations are still rare (you might get 100s
instead of 10s of them) but avoids lots of unused space in the worst
case. It gives a slight speed-up to cycle counts in some cases.
…ckler

Use min_specialization in liballoc

- Remove a type parameter from `[A]RcFromIter`.
- Remove an implementation of `[A]RcFromIter` that didn't actually
  specialize anything.
- Remove unused implementation of `IsZero` for `Option<&mut T>`.
- Change specializations of `[A]RcEqIdent` to use a marker trait version
of `Eq`.
- Remove `BTreeClone`. I couldn't find a way to make this work with
  `min_specialization`.
- Add `rustc_unsafe_specialization_marker` to `Copy` and `TrustedLen`.

After this only libcore is the only standard library crate using `feature(specialization)`.
cc rust-lang#31844
Miri interning: replace ICEs by proper errors

Fixes rust-lang#71316

I also did some refactoring, as I kept being confused by all the parameters to `intern_shallow`, some of which have invalid combinations (such as a mutable const). So instead `InternMode` now contains all the information that is needed and invalid combinations are ruled out by the type system.

Also I removed interpreter errors from interning. We already ignored almost all errors, and the `ValidationFailure` errors that we handled separately actually cannot ever happen here. The only interpreter failure that was actually reachable was the UB on dangling pointers -- and arguably, a dangling raw pointer is not UB, so the error was not even correct. It's just that the rest of the compiler does not like "dangling" `AllocId`.

It should be possible to review the 3 commits separately.

r? @oli-obk
Cc @rust-lang/wg-const-eval
Use `LocalDefId` in `DumpVisitor::nest_tables`

This is a partial fix for rust-lang#71104
…rowth, r=oli-obk

Be less aggressive with `DroplessArena`/`TypedArena` growth.

`DroplessArena` and `TypedArena` use an aggressive growth strategy: the first chunk is 4 KiB, the second is 8 KiB, and it keeps on doubling indefinitely. DHAT profiles show that sometimes this results in large chunks (e.g. 16-128 MiB) that are barely filled.

This commit changes things so that the doubling stops at 2 MiB. This is large enough that chunk allocations are still rare (you might get 100s instead of 10s of them) but avoids lots of unused space in the worst case. It makes the same change to `TypedArena`, too.
Fix unused_parens false positive when using binary operations

Fixes rust-lang#71290

r? @cuviper who provided instructions
Update transitive dependency to work towards removing syn <1.0 dep

This bumps a couple of transitive dependencies in hopes of eventually not transitively depending on syn <1.0 and friends. The only upstream changes that this is blocked on seems to be mattico/elasticlunr-rs#27 and rust-lang/mdBook#1210.

While working on rust-lang#71875 I noticed we still use syn 0.15 here and there so this is a drive-by PR which aims to help with things a bit.
…ochenkov

Incomplete features can also be unsound

Some incomplete features do not just ICE, they are also currently unsound (e.g. rust-lang#72029, and also `specialization` -- which is not yet marked incomplete but [should be](rust-lang#71420)). This makes the message reflect that.

While at it I also added a link to the tracking issue, which hopefully should explain what is incomplete/unsound about the feature.
…tebank

Don't ICE on missing `Unsize` impl

Previously code of the form

```rust
#![feature(unsize, dispatch_from_dyn)]

use std::marker::Unsize;
use std::ops::DispatchFromDyn;

pub struct Foo<'a, T: ?Sized> {
    _inner: &'a &'a T,
}

impl<'a, T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Foo<'a, U>> for Foo<'a, T> {}
```

would generate an ICE due to the missing `Unsize` impl being run through the `suggest_change_mut` suggestion. This PR adds an early exit and a pointer to the appropriate docs regarding `Unsize` instead:

```
error[E0277]: the trait bound `&'a T: std::marker::Unsize<&'a U>` is not satisfied
  --> src/test/ui/issues/issue-71036.rs:11:1
   |
11 | impl<'a, T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Foo<'a, U>> for Foo<'a, T> {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Unsize<&'a U>` is not implemented for `&'a T`
   |
   = note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
   = note: required because of the requirements on the impl of `std::ops::DispatchFromDyn<&'a &'a U>` for `&'a &'a T`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
```

r? @estebank

Resolves rust-lang#71036
@RalfJung
Copy link
Member Author

@rustbot modify labels: +rollup
@bors r+ rollup=never p=9

@bors
Copy link
Contributor

bors commented May 14, 2020

📌 Commit 2b03aa2 has been approved by RalfJung

@rustbot rustbot added the rollup A PR which is a rollup label May 14, 2020
@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 14, 2020
@RalfJung
Copy link
Member Author

@Dylan-DPC Yes looks like we did race.^^

Are you on Zulip? Might be worth having a rollup coordination channel somewhere or so. (Or I can also stop doing rollups -- I just want to help.)

@RalfJung
Copy link
Member Author

Closing in favor of #72202.

@RalfJung RalfJung closed this May 14, 2020
@RalfJung
Copy link
Member Author

In particular I am wondering how you are deciding which PRs to include. I take all the ones marked rollup=always, plus the first N in the queue (with N picked somehow at a whim and depending on how big the queue is), but excluding those where PR CI has not passed (yet). This ensures queue fairness. You are using some other policy and I do not know which one that is, so maybe I should adjust mine.

@RalfJung RalfJung deleted the rollup-78uu14m branch May 14, 2020 16:30
@Dylan-DPC-zz
Copy link

We don't have a policy as such. I got through the queue and pick the ones I predict won't conflict and are safe to rollup and are small enough. Also generally giving priority to ones which are in the queue for a long time, or prs created long back.

I appreciate the help. I used to end up in "rollup races" with centril as well. The trick to avoid a race is to wait for a minutes and check if there's a rollup or not. I'm generally active in the evening slot (evening in CEST) so you can expect me to rollup at that time more often that in the mornings :) Don't worry about adjusting any policy, just do whatever suits you :)

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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.