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 8 pull requests #115952

Merged
merged 26 commits into from
Sep 19, 2023
Merged

Rollup of 8 pull requests #115952

merged 26 commits into from
Sep 19, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

pietroalbini and others added 26 commits September 15, 2023 16:22
This code was calling `sort_unstable_by`, but failed to impose a total order on
the initial spans. That resulted in unpredictable handling of closure spans,
producing inconsistencies in the coverage maps and in user-visible coverage
reports.

This patch fixes the problem by always sorting closure spans before
otherwise-identical non-closure spans, and also switches to a stable sort in
case the ordering is still not total.
Switching to `Ordering::then_with` makes control-flow less complicated, and
there is no need to use `partial_cmp` here.
…, r=compiler-errors

Avoid blessing cargo deps's source code in ui tests

Before this PR, the source code of dependencies was included in UI test error messages whenever possible. Unfortunately, "whenever possible" means in some cases the source code wouldn't be injected, resulting in a test failure.

One such case is when `$CARGO_HOME` is remapped to something that is not present on disk [^1]. As the remapped path doesn't exist on disk, the source code wouldn't be showed in `tests/ui/issues/issue-21763.rs`:

```diff
    = note: required for `hashbrown::raw::RawTable<(Rc<()>, Rc<()>)>` to implement `Send`
 note: required because it appears within the type `HashMap<Rc<()>, Rc<()>, RandomState>`
   --> $HASHBROWN_SRC_LOCATION
-   |
-LL | pub struct HashMap<K, V, S = DefaultHashBuilder, A: Allocator + Clone = Global> {
-   |            ^^^^^^^
 note: required because it appears within the type `HashMap<Rc<()>, Rc<()>>`
   --> $SRC_DIR/std/src/collections/hash/map.rs:LL:COL
 note: required by a bound in `foo`
```

This PR fixes the problem by always hiding dependencies source code in the error messages generated during UI tests. This is implemented with a new internal flag, `-Z ignore-directory-in-diagnostics-source-blocks=$path`, which compiletest passes during UI tests. Once this is merged, remapping the Cargo home will be supported.

This PR is best reviewed commit-by-commit.

[^1]: After being puzzled for a bit, I discovered why this never impacted `rust-lang/rust`: we don't remap `$CARGO_HOME` :sweat_smile:. Instead, we set `$CARGO_HOME` to `/cargo` in CI, which sort-of-but-not-really achieves the same effect.
Make `TyKind::Adt`'s `Debug` impl be more pretty

Currently `{:?}` on `Ty` for a `TyKind::Adt` would print as `Adt(Foo, [])`. This PR changes it to be `Foo` when there are no generics or `Foo<T>`/`Foo<T, U>` when there _are_ generics. Example from debug log:
`├─0ms DEBUG rustc_hir_analysis::astconv return=Bar<T/#0, U/#1>`

I should have done this in my initial PR for a prettier TyKind: Debug impl but I thought I would need to be accessing generics_of to figure out where in the "path" the generics would have to go??? but no, adts literally only have a single place the generics can go (on the end). Feel a bit silly about this :)

r? `@oli-obk`
… r=compiler-errors

Migrate diagnostics in `hir_typeck/src/cast.rs`
coverage: Fix an unstable-sort inconsistency in coverage spans

This code was calling `sort_unstable_by`, but failed to impose a total order on the initial spans. That resulted in unpredictable handling of closure spans, producing inconsistencies in the coverage maps and in user-visible coverage reports.

This PR fixes the problem by always sorting closure spans before otherwise-identical non-closure spans, and also switches to a stable sort in case the ordering is still not total.

---

In addition to the fix itself, this PR also contains a cleanup to the comparison function that I was working on when I discovered the bug.
…notriddle

Move mobile topbar title creation entirely into JS

I was looking at potential size improvements and saw that we had an empty `h2` tag for the mobile topbar title that was filled with JS. So at this point, I think it's fine to just completely generate it from JS, like that the w3c HTML validator will emit one less warning.

r? `@notriddle`
…er-errors

compiletest: Don't swallow some error messages.

This updates some error handling in compiletest to display the underlying error rather than discarding it. There have been cases where the lack of error information makes it difficult to understand what went wrong.
…est, r=notriddle

Update browser-ui-test version

It includes the fix from `@notriddle` (GuillaumeGomez/browser-UI-test#537).

r? `@notriddle`
@rustbot rustbot added A-meta Area: Issues about the rust-lang/rust repository. 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) labels Sep 18, 2023
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Sep 18, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Sep 18, 2023

📌 Commit aa55d7d 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 Sep 18, 2023
@bors
Copy link
Contributor

bors commented Sep 19, 2023

⌛ Testing commit aa55d7d with merge f3984ce...

@bors
Copy link
Contributor

bors commented Sep 19, 2023

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

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#115869 Avoid blessing cargo deps's source code in ui tests d34486f895d11b14432c30f8925a78faf7b0d7a9 (link)
#115873 Make TyKind::Adt's Debug impl be more pretty 347f2fd63182e6a7cf772d3aff77f6f44bf93141 (link)
#115879 Migrate diagnostics in hir_typeck/src/cast.rs 5dad02c7dcd51d72c4e4b65e53522990f4e4767b (link)
#115930 coverage: Fix an unstable-sort inconsistency in coverage sp… 57cadbba6c076b82341448085594ab5d8429821a (link)
#115931 Move mobile topbar title creation entirely into JS 520305f5bed19e134191cf0d2d9eea4e149880eb (link)
#115941 Add myself to .mailmap b7c2a89b6abbda9f773adca8dcaffa3b5e991fca (link)
#115943 compiletest: Don't swallow some error messages. 8ec8ddb6f338c80cee7046a6512878b74131efce (link)
#115949 Update browser-ui-test version bd327bd4430ede16471384a1ed8dea53fe69141a (link)

previous master: 65ea825f40

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 (f3984ce): 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)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
6.7% [4.3%, 10.5%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.7% [-6.0%, -1.1%] 15
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

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)
1.6% [1.6%, 1.6%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.6% [1.6%, 1.6%] 1

Binary size

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

Bootstrap: 632.986s -> 631.066s (-0.30%)
Artifact size: 317.88 MiB -> 317.89 MiB (0.00%)

@matthiaskrgr matthiaskrgr deleted the rollup-qzk8t4e branch March 16, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues about the rust-lang/rust repository. 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. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.