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 7 pull requests #74493

Merged
merged 22 commits into from
Jul 19, 2020
Merged

Rollup of 7 pull requests #74493

merged 22 commits into from
Jul 19, 2020

Conversation

Manishearth
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

poliorcetics and others added 22 commits June 26, 2020 15:11
They are deprecated so doing extra work for error recovery doesn't make sense
Add core::task::ready! macro

This PR adds `ready!` as a top-level macro to `libcore` following the implementation of `futures_core::ready`, tracking issue rust-lang#70922. This macro is commonly used when implementing `Future`, `AsyncRead`, `AsyncWrite` and `Stream`. And being only 5 lines, it seems like a useful and straight forward addition to std.

## Example

```rust
use core::task::{Context, Poll};
use core::future::Future;
use core::pin::Pin;

async fn get_num() -> usize {
    42
}

pub fn do_poll(cx: &mut Context<'_>) -> Poll<()> {
    let mut f = get_num();
    let f = unsafe { Pin::new_unchecked(&mut f) };

    let num = ready!(f.poll(cx));
    // ... use num

    Poll::Ready(())
}
```

## Naming

In `async-std` we chose to nest the macro under the `task` module instead of having the macro at the top-level. This is a pattern that currently does not occur in std, mostly due to this not being possible prior to Rust 2018.

This PR proposes to add the `ready` macro as `core::ready`. But another option would be to introduce it as `core::task::ready` since it's really only useful when used in conjunction with `task::{Context, Poll}`.

## Implementation questions

I tried rendering the documentation locally but the macro didn't show up under `core`. I'm not sure if I quite got this right. I used the [`todo!` macro PR](https://github.com/rust-lang/rust/pull/56348/files) as a reference, and our approaches look similar.

## References

- [`futures::ready`](https://docs.rs/futures/0.3.4/futures/macro.ready.html)
- [`async_std::task::ready`](https://docs.rs/async-std/1.5.0/async_std/task/index.html)
- [`futures_core::ready`](https://docs.rs/futures-core/0.3.4/futures_core/macro.ready.html)
Document the trait keyword

Partial fix of rust-lang#34601.

This document the trait keyword. To avoid doing too much and forcing more updates as functionalities evolve, I put two links to the reference, especially for trait objects. This mainly documents the "big" parts, not so much the small details that might trip someone experimenting.

@rustbot modify labels: T-doc,C-enhancement
impl Index<RangeFrom> for CStr

This change implements (partial) slicing for `CStr`.

Since a `CStr` must end in a null byte, it's not possible to trim from the right and still have a valid `CStr`. But, it *is* possible to trim from the left. This lets us be a bit more flexible and treat them more like strings.

```rust
let string = CStr::from_bytes_with_nul(b"Hello World!\0");
let result = CStr::from_bytes_with_nul(b"World!\0");
assert_eq!(&string[6..], result);
```
rustc_metadata: Make crate loading fully speculative

Instead of reporting `span_err`s on the spot crate loading errors are now wrapped into the `CrateError` enum and returned, so they are reported only at the top level `resolve_crate` call, and not reported at all if we are resolving speculatively with `maybe_resolve_crate`.

As a result we can attempt loading crates for error recovery (e.g. import suggestions) without any risk of producing extra errors.
Also, this means better separation between error reporting and actual logic.

Fixes rust-lang#55103
Fixes rust-lang#56590
… r=oli-obk

Make unreachable_unchecked a const fn

This PR makes `std::hint::unreachable_unchecked` a const fn so we can use it inside a const function.
r? @RalfJung
Fixes rust-lang#53188.
…e-utf8, r=Mark-Simulacrum

Revert "Use an UTF-8 locale for the linker."

Reverts rust-lang#74416

This is suspected to have caused significant compile time regressions: https://perf.rust-lang.org/compare.html?start=39d5a61f2e4e237123837f5162cc275c2fd7e625&end=d3df8512d2c2afc6d2e7d8b5b951dd7f2ad77b02&stat=instructions:u
@Manishearth
Copy link
Member Author

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

@bors
Copy link
Contributor

bors commented Jul 18, 2020

📌 Commit a83e294 has been approved by Manishearth

@rustbot rustbot added the rollup A PR which is a rollup label Jul 18, 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 Jul 18, 2020
@Manishearth Manishearth reopened this Jul 18, 2020
@Manishearth
Copy link
Member Author

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

@bors
Copy link
Contributor

bors commented Jul 18, 2020

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jul 18, 2020

📌 Commit a83e294 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Jul 19, 2020

⌛ Testing commit a83e294 with merge 0701419...

@bors
Copy link
Contributor

bors commented Jul 19, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Manishearth
Pushing 0701419 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 19, 2020
@bors bors merged commit 0701419 into rust-lang:master Jul 19, 2020
@nnethercote
Copy link
Contributor

This was a big perf win. Presumably it was #74478 that was responsible.

However, the improvements here are smaller than those seen on perf CI runs in that PR. (E.g. the best improvement dropped from -42% to -33%.) So it's possible that one of the other PRs in this this rollup caused a significant regression. But it's difficult to say for certain, and I don't know which other PR may be the cause. Argh.

@Mark-Simulacrum
Copy link
Member

The perf results for #74478 landing in this PR (i.e., basically an up to date run) indicates that it essentially accounts for all of the benefits of this PR.

Presumably some of the offsets from the original try run were offset by some other changes -- though it is unclear exactly how that could be the case. OTOH, this landed atop 1fa54ad which was a known linker regression (based on perf runs elsewhere). So that may explain some of the offsets in terms of performance we managed to win back.

I consider this PR to be fully understood from a perf perspective.

@Manishearth Manishearth deleted the rollup-ust7yr4 branch July 23, 2020 20:10
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.