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

Merged
merged 22 commits into from
Dec 9, 2023
Merged

Rollup of 7 pull requests #118771

merged 22 commits into from
Dec 9, 2023

Conversation

workingjubilee
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

nnethercote and others added 22 commits December 8, 2023 21:02
… MIR

There are cases where coverage instrumentation wants to show a span for some
syntax element, but there is no MIR node that naturally carries that span, so
the instrumentor can't see it.

MIR building can now use this new kind of coverage statement to deliberately
include those spans in MIR, attached to a dummy statement that has no other
effect.
This replaces the previous workaround, which was to inject a dummy `Assign`
statement.
When MIR is built for an if-not expression, the `!` part of the condition
doesn't correspond to any MIR statement, so coverage instrumentation normally
can't see it.

We can fix that by deliberately injecting a dummy statement whose sole purpose
is to associate that span with its enclosing block.
This should make it easier to investigate unwrap failures in bug reports.
LLVM commit llvm/llvm-project@e817966
renamed the `unaligned-scalar-mem` target feature to `fast-unaligned-access`.
coverage: Use `SpanMarker` to improve coverage spans for `if !` expressions

Coverage instrumentation works by extracting source code spans from MIR. However, some kinds of syntax are effectively erased during MIR building, so their spans don't necessarily exist anywhere in MIR, making them invisible to the coverage instrumentor (unless we resort to various heuristics and hacks to recover them).

This PR introduces `CoverageKind::SpanMarker`, which is a new variant of `StatementKind::Coverage`. Its sole purpose is to represent spans that would otherwise not appear in MIR, so that the coverage instrumentor can extract them.

When coverage is enabled, the MIR builder can insert these dummy statements as needed, to improve the accuracy of spans used by coverage mappings.

Fixes rust-lang#115468.

---

```@rustbot``` label +A-code-coverage
…ted-tests, r=jackh726

Add tests related to normalization in implied bounds

Getting ```@aliemjay's``` tests from rust-lang#109763, so we can better track what's going on in every different example.

r? ```@jackh726```
update target feature following LLVM API change

LLVM commit llvm/llvm-project@e817966 renamed* the `unaligned-scalar-mem` target feature to `fast-unaligned-access`.

(*) technically the commit folded two previous features into one, but there are no references to the other one in rust.
coverage: Simplify the heuristic for ignoring `async fn` return spans

The code for extracting coverage spans from MIR has a special heuristic for dealing with `async fn`, so that the function's closing brace does not have a confusing double count.

The code implementing that heuristic is currently mixed in with the code for flushing remaining spans after the main refinement loop, making the refinement code harder to understand.

We can solve that by hoisting the heuristic to an earlier stage, after the spans have been extracted and sorted but before they have been processed by the refinement loop.

The coverage tests verify that the heuristic is still effective, so coverage mappings/reports for `async fn` have not changed.

---

This PR also has the side-effect of fixing the `None some_prev` panic that started appearing after rust-lang#118525.

The old code assumed that `prev` would always be present after the refinement loop. That was only true if the list of collected spans was non-empty, but prior to rust-lang#118525 that didn't seem to come up in practice. After that change, the list of collected spans could be empty in some specific circumstances, leading to panics.

The new code uses an `if let` to inspect `prev`, which correctly does nothing if there is no span present.
…estsa, r=Nilstrieb

Extend tidy alphabetical checking to `tests/`.

This is desired for rust-lang#118702.

r? ```@Nilstrieb```
Some more minor `async gen`-related nits

Tiny tweaks found after `async gen` pr landed

r? eholk
…or, r=eholk

Make async generators fused by default

I actually changed my mind about this since the implementation PR landed. I think it's beneficial for `async gen` blocks to be "fused" by default -- i.e., for them to repeatedly return `Poll::Ready(None)` -- rather than panic.

We have [`FusedStream`](https://docs.rs/futures/latest/futures/stream/trait.FusedStream.html) in futures-rs to represent streams with this capability already anyways.

r? eholk
cc ```@rust-lang/wg-async,``` would like to know if anyone else has opinions about this.
@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 Dec 9, 2023
@workingjubilee
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Dec 9, 2023

📌 Commit 61dfb1f has been approved by workingjubilee

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 9, 2023
@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 Dec 9, 2023
@bors
Copy link
Contributor

bors commented Dec 9, 2023

⌛ Testing commit 61dfb1f with merge ce67033...

@bors
Copy link
Contributor

bors commented Dec 9, 2023

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing ce67033 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 9, 2023
@bors bors merged commit ce67033 into rust-lang:master Dec 9, 2023
12 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 9, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#118198 coverage: Use SpanMarker to improve coverage spans for `i… 8ae42fb756916d1bfac81a6fe11a9031460dcbb0 (link)
#118512 Add tests related to normalization in implied bounds 87282e05f67f4f5363b46c922fd7ff0d5c48f2e5 (link)
#118610 update target feature following LLVM API change 6ca70ac231fedfe67e0b6b4d95525ed6730883b0 (link)
#118666 coverage: Simplify the heuristic for ignoring async fn re… a10d55635ac8a160bdf3756b677e9ca9dc9b02f2 (link)
#118737 Extend tidy alphabetical checking to tests/. f9ad14cf2baa2de4b8a7640d9820d9ebc9bc3985 (link)
#118762 Some more minor async gen-related nits d70551080f4f187315806de0ae67a67f1226b526 (link)
#118764 Make async generators fused by default 47f91fdf513fa0c0a0e9908ae76891995330512c (link)

previous master: c41669970a

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 (ce67033): 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

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.8% [0.6%, 1.1%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.0% [-1.4%, -0.5%] 2
Improvements ✅
(secondary)
-1.9% [-1.9%, -1.9%] 1
All ❌✅ (primary) -0.1% [-1.4%, 1.1%] 4

Cycles

Results

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)
- - 0
Improvements ✅
(primary)
-0.5% [-0.5%, -0.4%] 4
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.5% [-0.5%, -0.4%] 4

Binary size

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

Bootstrap: 669.317s -> 670.475s (0.17%)
Artifact size: 314.04 MiB -> 314.11 MiB (0.02%)

@workingjubilee workingjubilee deleted the rollup-q1p3riz branch December 9, 2023 21:16
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