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 #120064

Closed
wants to merge 29 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

cjgillot and others added 29 commits January 7, 2024 20:07
Enable crt-static for FreeBSD to enable statically compiled binaries.
The advantage of this is that it does not need to be assigned to a
variable to be used in a `Context` creation, which is the most common
thing to want to do with a noop waker.

If an owned noop waker is desired, it can be created by cloning, but the
reverse is harder. Alternatively, both versions could be provided, like
`futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but
that seems to me to be API clutter for a very small benefit, whereas
having the `&'static` reference available is a large benefit.

Previous discussion on the tracking issue starting here:
rust-lang#98286 (comment)
`Waker::noop()` now returns a `&'static Waker` reference, so it can be
passed directly to `Context` creation with no temporary lifetime issue.
Save liveness results for DestinationPropagation

`DestinationPropagation` needs to verify that merge candidates do not conflict with each other. This is done by verifying that a local is not live when its counterpart is written to.

To get the liveness information, the pass runs `MaybeLiveLocals` dataflow analysis repeatedly, once for each propagation round. This is quite costly, and the main driver for the perf impact on `ucd` and `diesel`. (See rust-lang#115105 (comment))

In order to mitigate this cost, this PR proposes to save the result of the analysis into a `SparseIntervalMatrix`, and mirror merges of locals into that matrix: `liveness(destination) := liveness(destination) union liveness(source)`.

<details>
<summary>Proof</summary>

We denote by `'` all the quantities of the transformed program. Let $\varphi$ be a mapping of locals, which maps `source` to `destination`, and is identity otherwise. The exact liveness set after a statement is $out'(statement)$, and the proposed liveness set is $\varphi(out(statement))$.

Consider a statement. Suppose that the output state verifies $out' \subset phi(out)$. We want to prove that $in' \subset \varphi(in)$ where $in = (out - kill) \cup gen$, and conclude by induction.

We have 2 cases: either that statement is kept with locals renumbered by $\varphi$, or it is a tautological assignment and it removed.

1. If the statement is kept: the gen-set and the kill-set of $statement' = \varphi(statement)$ are $gen' = \varphi(gen)$ and $kill' = \varphi(kill)$ exactly.
From soundness requirement 3, $\varphi(in)$ is disjoint from $\varphi(kill)$.
This implies that $\varphi(out - kill)$ is disjoint from $\varphi(kill)$, and so $\varphi(out - kill) = \varphi(out) - \varphi(kill)$. Then $\varphi(in) = (\varphi(out) - \varphi(kill)) \cup \varphi(gen) = (\varphi(out) - kill') \cup gen'$.
We can conclude that $out' \subset \varphi(out) \implies in' \subset \varphi(in)$.

2. If the statement is removed. As $\varphi(statement)$ is a tautological assignment, we know that $\varphi(gen) = \varphi(kill) = \\{ destination \\}$, while $gen' = kill' = \emptyset$. So $\varphi(in) = \varphi(out) \cup \\{ destination \\}$. Then $in' = out' \subset out \subset \varphi(in)$.

By recursion, we can conclude by that $in' \subset \varphi(in)$ everywhere.
</details>

This approximate liveness results is only suboptimal if there are locals that fully disappear from the CFG due to an assignment cycle. These cases are quite unlikely, so we do not bother with them.

This change allows to reduce the perf impact of DestinationPropagation by half on diesel and ucd (rust-lang#115105 (comment)).

cc ``@JakobDegen``
…leywiser

Enable Static Builds for FreeBSD

Enable crt-static for FreeBSD to enable statically compiled binaries.
…and-opaque-types-do-mix-sometimes, r=compiler-errors

Don't ICE if TAIT-defining fn contains a closure with `_` in return type

The `delay_span_bug` got added in rust-lang@0e82aae to reduce the amount of errors emitted for functions that have `_` in their return type, because inference doesn't apply to function items. But this logic shouldn't apply to closures, because their return types *can* be inferred.

Fixes rust-lang#119916.
Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.

The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. It also avoids unnecessarily executing the dynamically dispatched drop function when the noop waker is dropped.

If an owned noop waker is desired, it can be created by cloning, but the reverse is harder to do since it requires declaring a constant. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large reduction in boilerplate.

[Previous discussion on the tracking issue starting here](rust-lang#98286 (comment))
…-ozkan

Consistently unset RUSTC_BOOTSTRAP when compiling bootstrap

Since rust-lang#113906, all x.py invocations performed by rust-analyzer have RUSTC_BOOTSTRAP=1 set. This was to fix rust-lang#112391 (comment) &mdash; rust-analyzer uses some default cargo from the system when fetching workspace layout, and the standard library uses some unstable cargo feature, so x.py would previously fail if the system toolchain wasn't nightly.

https://github.com/rust-lang/rust/blob/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/Cargo.toml#L1

This PR changes x.py to cancel out this RUSTC_BOOTSTRAP=1 when compiling bootstrap. It will only remain set when running the compiled bootstrap executable. This fixes spurious bootstrap rebuilds in the event that a rust-analyzer x.py invocation is alternated with someone running x.py themself on the command line, if any dependency of bootstrap looks at `option_env!("RUSTC_BOOTSTRAP")`, which is the case since rust-lang#119654.

**Before:**

```console
$ RUSTC_BOOTSTRAP=1 ./x.py check library/core
Building bootstrap
   Compiling proc-macro2 v1.0.76
   Compiling quote v1.0.35
   Compiling syn v2.0.48
   Compiling clap_derive v4.4.7
   Compiling serde_derive v1.0.195
   Compiling clap v4.4.13
   Compiling clap_complete v4.4.6
   Compiling build_helper v0.1.0
   Compiling bootstrap v0.0.0
    Finished dev [unoptimized] target(s) in 6.31s
Checking stage0 library artifacts {core} (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.23s
Build completed successfully in 0:00:07

$ ./x.py check library/core
Building bootstrap
   Compiling proc-macro2 v1.0.76
   Compiling quote v1.0.35
   Compiling syn v2.0.48
   Compiling clap_derive v4.4.7
   Compiling serde_derive v1.0.195
   Compiling clap v4.4.13
   Compiling clap_complete v4.4.6
   Compiling build_helper v0.1.0
   Compiling bootstrap v0.0.0
    Finished dev [unoptimized] target(s) in 5.30s
Checking stage0 library artifacts {core} (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.25s
Build completed successfully in 0:00:06
```

**After:**

```console
$ RUSTC_BOOTSTRAP=1 ./x.py check library/core
Building bootstrap
    Finished dev [unoptimized] target(s) in 0.06s
Checking stage0 library artifacts {core} (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.14s
Build completed successfully in 0:00:01

$ ./x.py check library/core
Building bootstrap
    Finished dev [unoptimized] target(s) in 0.04s
Checking stage0 library artifacts {core} (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.13s
Build completed successfully in 0:00:01
```
… r=compiler-errors

Gracefully handle missing typeck information if typeck errored

fixes rust-lang#116893

I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
…ty-eagerly, r=oli-obk

Construct closure type eagerly

Construct the returned closure type *before* checking the body, in the same match as we were previously deducing the coroutine types based off of the closure kind.

This simplifies some changes I'm doing in the async closure PR, and imo just seems easier to read (since we only need one match on closure kind, instead of two). There's no reason I can tell that we needed to create the closure type *after* the body was checked.

~~This also has the side-effect of making it so that the universe of the closure synthetic infer vars are lower than any infer vars that come from checking the body. We can also get rid of `next_root_ty_var` hack from closure checking (though in general we still need this, rust-lang#119106). cc `@lcnr` since you may care about this hack 😆~~

r? `@oli-obk`
Fix `rustc_abi` build on stable

rust-lang#119446 broke the ability of `rustc_abi` to build on stable, which is required by rust-analyzer. This fixes it.
…rrors

pat_analysis: Don't rely on contiguous `VariantId`s outside of rustc

Today's pattern_analysis uses `BitSet` and `IndexVec` on the provided enum variant ids, which only makes sense if these ids count the variants from 0. In rust-analyzer, the variant ids are global interning ids, which would make `BitSet` and `IndexVec` ridiculously wasteful. In this PR I add some shims to use `FxHashSet`/`FxHashMap` instead outside of rustc.

r? `@compiler-errors`
Fix typo in comments (in_place_collect)
…er-errors

Use FnOnceOutput instead of FnOnce where expected

fixes rust-lang#119847
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 17, 2024
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jan 17, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=11

@bors
Copy link
Contributor

bors commented Jan 17, 2024

📌 Commit e03780f has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 Jan 17, 2024
@bors
Copy link
Contributor

bors commented Jan 17, 2024

⌛ Testing commit e03780f with merge e649aef...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#115291 (Save liveness results for DestinationPropagation)
 - rust-lang#119855 (Enable Static Builds for FreeBSD)
 - rust-lang#119975 (Don't ICE if TAIT-defining fn contains a closure with `_` in return type)
 - rust-lang#119984 (Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.)
 - rust-lang#120001 (Consistently unset RUSTC_BOOTSTRAP when compiling bootstrap)
 - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored)
 - rust-lang#120031 (Construct closure type eagerly)
 - rust-lang#120032 (Fix `rustc_abi` build on stable)
 - rust-lang#120039 (pat_analysis: Don't rely on contiguous `VariantId`s outside of rustc)
 - rust-lang#120044 (Fix typo in comments (in_place_collect))
 - rust-lang#120056 (Use FnOnceOutput instead of FnOnce where expected)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

---- [coverage-run] tests/coverage/closure_macro_async.rs stdout ----
diff of coverage:

15    LL|       |
16    LL|       |macro_rules! on_error {
17    LL|       |    ($value:expr, $error_message:expr) => {
-    LL|       |        $value.or_else(|e| { // FIXME(85000): no coverage in closure macros
+    LL|       |        $value.or_else(|e| {
+    LL|       |            // FIXME(85000): no coverage in closure macros
19    LL|       |            let message = format!($error_message, e);
20    LL|       |            if message.len() > 0 {
21    LL|       |                println!("{}", message);

The actual coverage differed from the expected coverage.
The actual coverage differed from the expected coverage.
Actual coverage saved to /checkout/obj/build/aarch64-unknown-linux-gnu/test/coverage/closure_macro_async.coverage-run/closure_macro_async.coverage
error: 1 errors occurred comparing coverage output.
status: exit status: 0
status: exit status: 0
command: "/checkout/obj/build/aarch64-unknown-linux-gnu/llvm/build/bin/llvm-cov" "show" "--format=text" "--show-line-counts-or-regions" "--Xdemangler" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage0-tools-bin/rust-demangler" "--instr-profile" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/coverage/closure_macro_async.coverage-run/default.profdata" "--object" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/coverage/closure_macro_async.coverage-run/a"
--- stdout -------------------------------
    1|       |#![feature(coverage_attribute)]
    2|       |#![feature(noop_waker)]
    3|       |// edition: 2018
    4|       |
    5|       |macro_rules! bail {
    6|       |    ($msg:literal $(,)?) => {
    7|       |        if $msg.len() > 0 {
    8|       |            println!("no msg");
   10|       |            println!($msg);
   11|       |        }
   12|       |        return Err(String::from($msg));
   13|       |    };
   13|       |    };
   14|       |}
   15|       |
   16|       |macro_rules! on_error {
   17|       |    ($value:expr, $error_message:expr) => {
   18|       |        $value.or_else(|e| {
   19|       |            // FIXME(85000): no coverage in closure macros
   20|       |            let message = format!($error_message, e);
   21|       |            if message.len() > 0 {
   22|       |                println!("{}", message);
   23|       |                Ok(String::from("ok"))
   24|       |            } else {
   25|       |                bail!("error");
   27|       |        })
   28|       |    };
   29|       |}
   30|       |
   30|       |
   31|      1|fn load_configuration_files() -> Result<String, String> {
   32|      1|    Ok(String::from("config"))
   33|      1|}
   34|       |
   35|      1|pub async fn test() -> Result<(), String> {
   36|      1|    println!("Starting service");
   37|      1|    let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
   38|       |
   38|       |
   39|      1|    let startup_delay_duration = String::from("arg");
   40|      1|    let _ = (config, startup_delay_duration);
   42|      1|}
   43|       |
   43|       |
   44|       |#[coverage(off)]
   45|       |fn main() {
   46|       |    executor::block_on(test()).unwrap();
   47|       |}
   48|       |
   49|       |mod executor {
   50|       |    use core::future::Future;
   51|       |    use core::pin::pin;
   52|       |    use core::task::{Context, Poll, Waker};
   53|       |
   54|       |    #[coverage(off)]
   55|       |    pub fn block_on<F: Future>(mut future: F) -> F::Output {
   56|       |        let mut future = pin!(future);
   57|       |        let mut context = Context::from_waker(Waker::noop());
   59|       |        loop {
   59|       |        loop {
   60|       |            if let Poll::Ready(val) = future.as_mut().poll(&mut context) {
   61|       |                break val;
   63|       |        }
   64|       |    }
   65|       |}
------------------------------------------

@bors
Copy link
Contributor

bors commented Jan 17, 2024

💔 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 Jan 17, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-we5zgxg branch March 16, 2024 18:20
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. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.