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

Merged
merged 21 commits into from
May 29, 2024
Merged

Rollup of 7 pull requests #125732

merged 21 commits into from
May 29, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Darksonn and others added 21 commits May 3, 2024 14:32
Signed-off-by: Alice Ryhl <[email protected]>
For coverage tests, splitting code across multiple lines often makes the
resulting coverage report easier to interpret, so we force rustfmt to retain
line breaks by adding dummy line comments with `//`.
Currently we can't automatically enforce formatting on tests (see rust-lang#125637), but
we can at least keep things relatively tidy by occasionally running the
formatter manually.

This was done by temporarily commenting out the `"/tests/"` exclusion in
`rustfmt.toml`, and then running `x fmt tests/coverage` and
`x test coverage --bless`.
If we perform this subtraction and then add 1, the subtraction can sometimes
overflow to -1 before the addition can bring its value back to 0. That
behaviour seems to be benign, but it nevertheless causes test failures in
compiler configurations that check for overflow.

We can avoid the overflow by instead subtracting (N - 1), which is
algebraically equivalent, and more closely matches what the code is actually
trying to do.
This moves a few hundred lines of coverage-specific code out of the main
module, making navigation a bit easier.
Add `-Zfixed-x18`

This PR is a follow-up to rust-lang#124323 that proposes a different implementation. Please read the description of that PR for motivation.

See the equivalent flag in [the clang docs](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-ffixed-x18).

MCP: rust-lang/compiler-team#748
Fixes rust-lang#121970
r? rust-lang/compiler
Format all source files in `tests/coverage/`

Currently we can't automatically enforce formatting on tests (see rust-lang#125637), but we can at least keep things relatively tidy by occasionally running the formatter manually.

This was done by temporarily commenting out the `"/tests/"` exclusion in `rustfmt.toml`, and then running:
- `x fmt tests/coverage`
- `x test coverage --bless`

(This PR also includes a few cosmetic tweaks to some of the affected files, to convince rustfmt to format them in the way we want.)

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

coverage: Avoid overflow when the MC/DC condition limit is exceeded

Fix for the test failure seen in rust-lang#124571 (comment).

If we perform this subtraction first, it can sometimes overflow to -1 before the addition can bring its value back to 0.

That behaviour seems to be benign, but it nevertheless causes test failures in compiler configurations that check for overflow.

``@rustbot`` label +A-code-coverage
…r-errors

Reintroduce name resolution check for trying to access locals from an inline const

fixes rust-lang#125676

I removed this without replacement in rust-lang#124650 without considering the consequences
tier 3 target policy: clarify the point about producing assembly

I think that is already the intended meaning of the policy, but I am not sure.

Cc ``@rust-lang/compiler``
remove unneeded extern crate in rmake test

Cleanup requested in rust-lang#125493 (comment)

r? jieyouxu
``@bors`` rollup=always
Extract coverage-specific code out of `compiletest::runtest`

I had been vaguely intending to do this for a while, but seeing rust-lang#89475 on the compiletest dashboard inspired me to actually go and do it.

This moves a few hundred lines of coverage-specific code out of the main module, making navigation a bit easier. There is still a small amount of coverage-specific logic in broader functions in that module, since it can't easily be moved.

This is just cut-and-paste plus fixing visibility and imports, so no functional changes.

I also removed the unit test for anonymizing line numbers in MC/DC reports, as foreshadowed by the comment I wrote when adding it. That functionality is now adequately exercised by the actual snapshot tests for MC/DC coverage.

(Removing the test now avoids the need to move it, or to make the function it calls visible.)
@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. rollup A PR which is a rollup labels May 29, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented May 29, 2024

📌 Commit 6ef3dd0 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 May 29, 2024
@bors
Copy link
Contributor

bors commented May 29, 2024

⌛ Testing commit 6ef3dd0 with merge debd22d...

@bors
Copy link
Contributor

bors commented May 29, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing debd22d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 29, 2024
@bors bors merged commit debd22d into rust-lang:master May 29, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 29, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#124655 Add -Zfixed-x18 d0297fa6c730c9deb68a3e63a1f0f9b5cb5922d4 (link)
#125693 Format all source files in tests/coverage/ d1d8b3b6aa395e0d5369b7455a04b086817fd1e1 (link)
#125700 coverage: Avoid overflow when the MC/DC condition limit is … 18562419a1b4e9ca70ad75ef672fbe2e30f844de (link)
#125705 Reintroduce name resolution check for trying to access loca… cfab26cea8fd8c19f9ec72f167e77b05b8403304 (link)
#125708 tier 3 target policy: clarify the point about producing ass… a6c1372aaaf2aa51c039fce6e4d5073ab2c490b4 (link)
#125715 remove unneeded extern crate in rmake test 6d42444af8e9c92ac349392620ed8eee29bdb350 (link)
#125719 Extract coverage-specific code out of compiletest::runtest 4fecf0ee528a3e417e55dfa816a029ef758009df (link)

previous master: e9b7aa08f7

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (debd22d): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary 2.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.9% [2.9%, 2.9%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 667.139s -> 666.699s (-0.07%)
Artifact size: 318.84 MiB -> 318.90 MiB (0.02%)

@matthiaskrgr matthiaskrgr deleted the rollup-bozbtk3 branch September 1, 2024 17:35
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 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. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants