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

Closed
wants to merge 22 commits into from
Closed

Conversation

jieyouxu
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

amandasystems and others added 22 commits May 28, 2024 13:05
This shunts all the complexity of siphoning off the drop-use facts
into `LivenessResults::add_extra_drop_facts()`, which may or may
not be a good approach.
When a lazy logical operator (`&&` or `||`) occurs outside of an `if`
condition, it normally doesn't have any associated control-flow branch, so we
don't have an existing way to track whether it was true or false.

This patch adds special code to handle this case, by inserting extra MIR blocks
in a diamond shape after evaluating the RHS. This gives us a place to insert
the appropriate marker statements, which can then be given their own counters.
We do this for `&*` and `&mut*` already; might as well do it for raw pointers too.
Especially for command wrappers like `Rustc`, it's very easy to build up
a command invocation but forget to actually execute it, e.g. by using
`run()`. This commit adds "drop bombs" to command wrappers, which are
armed on command wrapper construction, and only defused if the command
is executed (through `run`, `run_fail` or `run_fail_assert_exit_code`).
If the test writer forgets to execute the command, the drop bomb will
"explode" and panic with an error message. This is so that tests don't
silently pass with constructed-but-not-executed command wrappers.

We don't add `#[must_use]` for command wrapper helper methods because
they return `&mut Self` and can be annoying e.g. if a helper method is
conditionally called, such as

```
if condition {
    cmd.arg("-Cprefer-dynamic"); // <- unused_must_use fires
}
cmd.run(); // <- even though cmd is eventually executed
```

We also add `#[must_use]` attributes to functions in the support library
where suitable to help catch unintentionally discarded values.
The coverage-dump tool already needs `rustc_demangle` for its own purposes, so
the amount of extra code needed for a demangle mode is very small.
This avoids the need to build `rust-demangler` when running coverage tests,
since we typically need to build `coverage-dump` anyway.
This appears to be the canonical way to build a tool with the stage 0 compiler.
…g, r=oli-obk

Revert propagation of drop-live information from Polonius

rust-lang#64749 introduced a flow of drop-use data from Polonius to `LivenessResults::add_extra_drop_facts()`, which makes `LivenessResults` agree with Polonius on liveness in the presence of free regions that may be dropped. Later changes accidentally removed this flow. This PR restores it.
…-obk

Apply `x clippy --fix` and `x fmt` on Rustc

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->

Just run `x clippy --fix` and `x fmt`, and remove some changes like `impl Default`.
run-make: enforce `#[must_use]` and arm command wrappers with drop bombs

This PR is one in a series of cleanups to run-make tests and the run-make-support library.

### Summary

It's easy to forget to actually executed constructed command wrappers, e.g. `rustc().input("foo.rs")` but forget the `run()`, so to help catch these mistakes, we:

- Add `#[must_use]` annotations to functions where suitable and compile rmake.rs recipes with `-Dunused_must_use`.
- Arm command wrappers with drop bombs on construction to force them to be executed by test code.

### Details

Especially for command wrappers like `Rustc`, it's very easy to build up
a command invocation but forget to actually execute it, e.g. by using
`run()`. This commit adds "drop bombs" to command wrappers, which are
armed on command wrapper construction, and only defused if the command
is executed (through `run`, `run_fail` or `run_fail_assert_exit_code`).
If the test writer forgets to execute the command, the drop bomb will
"explode" and panic with an error message. This is so that tests don't
silently pass with constructed-but-not-executed command wrappers.

We don't add `#[must_use]` for command wrapper helper methods because
they return `&mut Self` and can be annoying e.g. if a helper method is
conditionally called, such as

```
if condition {
    cmd.arg("-Cprefer-dynamic"); // <- unused_must_use fires
}
cmd.run(); // <- even though cmd is eventually executed
```

This PR is best reviewed commit-by-commit.

Fixes rust-lang#125703.

Because `command_output()` doesn't defuse the drop bomb, it also fixes rust-lang#125617.
coverage: Optionally instrument the RHS of lazy logical operators

(This is an updated version of rust-lang#124644 and rust-lang#124402. Fixes rust-lang#124120.)

When `||` or `&&` is used outside of a branching context (such as the condition of an `if`), the rightmost value does not directly influence any branching decision, so branch coverage instrumentation does not treat it as its own true-or-false branch.

That is a correct and useful interpretation of “branch coverage”, but might be undesirable in some contexts, as described at rust-lang#124120. This PR therefore adds a new coverage level `-Zcoverage-options=condition` that behaves like branch coverage, but also adds additional branch instrumentation to the right-hand-side of lazy boolean operators.

---

As discussed at rust-lang#124120 (comment), this is mainly intended as an intermediate step towards fully-featured MC/DC instrumentation. It's likely that we'll eventually want to remove this coverage level (rather than stabilize it), either because it has been incorporated into MC/DC instrumentation, or because it's getting in the way of future MC/DC work. The main appeal of landing it now is so that work on tracking conditions can proceed concurrently with other MC/DC-related work.

``@rustbot`` label +A-code-coverage
…-obk

Also InstSimplify `&raw*`

We do this for `&*` and `&mut*` already; might as well do it for raw pointers too.

r? mir-opt
Don't build the `rust-demangler` binary for coverage tests

The coverage-run tests invoke `llvm-cov`, which requires us to specify a command-line demangler that it can use to demangle Rust symbol names.

Historically this used `src/tools/rust-demangler`, which means that we currently build two different command-line tools to help with the coverage tests (`rust-demangler` and `coverage-dump`).

However, it occurred to me that if we add a demangler mode to `coverage-dump` (which is only a handful of lines and no extra dependencies), then we only need to build one helper binary for the coverage tests, and there is no need for tests to build `rust-demangler` at all.

---

Note that the `rust-demangler` binary is separate from the `rustc-demangle` crate (which both `rust-demangler` and `coverage-dump` use as a dependency to do the actual demangling).

---

So the main benefits/motivations here are:
- Slightly faster builds after a fresh checkout or bootstrap bump.
- Making it clear that currently no tests actually need the `rust-demangler` binary, since the coverage tests can use their own tool instead.
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc 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. rollup A PR which is a rollup labels May 31, 2024
@jieyouxu
Copy link
Member Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented May 31, 2024

📌 Commit 7082681 has been approved by jieyouxu

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 May 31, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 31, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#125652 (Revert propagation of drop-live information from Polonius)
 - rust-lang#125730 (Apply `x clippy --fix` and `x fmt` on Rustc)
 - rust-lang#125752 (run-make: enforce `#[must_use]` and arm command wrappers with drop bombs)
 - rust-lang#125756 (coverage: Optionally instrument the RHS of lazy logical operators)
 - rust-lang#125796 (Also InstSimplify `&raw*`)
 - rust-lang#125816 (Don't build the `rust-demangler` binary for coverage tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented May 31, 2024

⌛ Testing commit 7082681 with merge 144b9c5...

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

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

@bors
Copy link
Contributor

bors commented May 31, 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 May 31, 2024
@jieyouxu
Copy link
Member Author

jieyouxu commented May 31, 2024

curl: (35) Recv failure: Software caused connection abort

@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 31, 2024
@jieyouxu
Copy link
Member Author

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 31, 2024
@jieyouxu jieyouxu closed this May 31, 2024
@jieyouxu jieyouxu deleted the rollup-ak8rrn1 branch May 31, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc rollup A PR which is a rollup S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

9 participants