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 9 pull requests #129365

Merged
merged 24 commits into from
Aug 22, 2024
Merged

Rollup of 9 pull requests #129365

merged 24 commits into from
Aug 22, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

bvanjoi and others added 24 commits July 5, 2024 01:00
There are quite a few uses of escaped quotes. Turn these into raw
strings within documentation and tests to make things easier to read.
Add a `.finish_non_exhaustive()` method to `DebugTuple`, `DebugSet`,
`DebugList`, and `DebugMap`. This indicates that the structures have
remaining items with `..`.

This implements the ACP at
<rust-lang/libs-team#248>.
1. Decouple them.
2. Make logic around `diagnostic_outside_of_impl`'s early exits simpler.
3. Make `untranslatable_diagnostic` run one loop instead of two
   and not allocate an intermediate vec.
4. Overall, reduce the amount of code executed
   when the lints do not end up firing.
... by using `std::fs::remove_dir_all`, which handles a bunch of edge
cases including read-only files and symlinks which are extremely tricky
on Windows.
During config parsing, some bootstrap logic (e.g., `download-ci-llvm`) checks certain sources
and acts based on their state. This means that if path is a git submodule, bootstrap needs to
update it before checking its state. Otherwise it may make incorrect assumptions by relying on
outdated sources. To enable submodule updates during config parsing, we need to move the `update_submodule`
function from the `Build` to `Config` instance, so we can access to it during the parsing process.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Before the workspace split, the library was covered by the weekly `cargo
update` cron job. Now that the library has its own workspace, it doesn't
get these updates.

Add `library/Cargo.toml` to the job so updates happen again.
Signed-off-by: onur-ozkan <work@onurozkan.dev>
This approach is, roughly, based on how Discourse does it.
It came up while discussing some other possible sidebar changes,
as a design that made rapid scanning easier while avoiding the
inherent trade-offs in summarizing.
Given `trait Any: 'static` and a `struct` with a `Box<dyn Any + 'a>` field, point at the `'static` bound in `Any` to explain why `'a: 'static`.

```
error[E0478]: lifetime bound not satisfied
   --> f202.rs:2:12
    |
2   |     value: Box<dyn std::any::Any + 'a>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
note: lifetime parameter instantiated with the lifetime `'a` as defined here
   --> f202.rs:1:14
    |
1   | struct Hello<'a> {
    |              ^^
note: but lifetime parameter must outlive the static lifetime
   --> /home/gh-estebank/rust/library/core/src/any.rs:113:16
    |
113 | pub trait Any: 'static {
    |                ^^^^^^^
```

Partially address rust-lang#33652.
use old ctx if has same expand environment during decode span

Fixes rust-lang#112680

The root reason why rust-lang#112680 failed with incremental compilation on the second attempt is the difference in `opaque` between the span of the field [`ident`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_hir_typeck/src/expr.rs#L2348) and the span in the incremental cache at `tcx.def_ident_span(field.did)`.

-  Let's call the span of `ident` as `span_a`, which is generated by [`apply_mark_internal`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_span/src/hygiene.rs#L553-L554). Its content is similar to:

```rs
span_a_ctx -> SyntaxContextData {
      opaque: span_a_ctx,
      opaque_and_semitransparent: span_a_ctx,
      // ....
}
```

- And call the span of `tcx.def_ident_span` as `span_b`, which is generated by [`decode_syntax_context`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_span/src/hygiene.rs#L1390). Its content is:

```rs
span_b_ctx -> SyntaxContextData {
      opaque: span_b_ctx,
      // note `span_b_ctx` is not same as `span_a_ctx`
      opaque_and_semitransparent: span_b_ctx,
      // ....
}
```

Although they have the same `parent` (both refer to the root) and `outer_expn`, I cannot find the specific connection between them. Therefore, I chose a solution that may not be the best: give up the incremental compile cache to ensure we can use `span_a` in this case.

r?  `@petrochenkov` Do you have any advice on this? Or perhaps this solution is acceptable?
…, r=Noratrieb

Implement `debug_more_non_exhaustive`

This implements the ACP at rust-lang/libs-team#248, adding `.finish_non_exhaustive()` for `DebugTuple`, `DebugSet`, `DebugList`, and `DebugMap`.

Also used this as an opportunity to make some documentation and tests more readable by using raw strings instead of escaped quotes.

Tracking issue: rust-lang#127942
…ints, r=davidtwco

 Improve diagnostic-related lints: `untranslatable_diagnostic` & `diagnostic_outside_of_impl`

Summary:
- Made `untranslatable_diagnostic` point to problematic arguments instead of the function call
  (I found this misleading while working on some `A-translation` PRs: my first impression was that
  the methods themselves were not translation-aware and needed to be changed,
  while in reality the problem was with the hardcoded strings passed as arguments).
- Made the shared pass of `untranslatable_diagnostic` & `diagnostic_outside_of_impl` more efficient.

`@rustbot` label D-imprecise-spans A-translation
Point at explicit `'static` obligations on a trait

Given `trait Any: 'static` and a `struct` with a `Box<dyn Any + 'a>` field, point at the `'static` bound in `Any` to explain why `'a: 'static`.

```
error[E0478]: lifetime bound not satisfied
   --> f202.rs:2:12
    |
2   |     value: Box<dyn std::any::Any + 'a>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
note: lifetime parameter instantiated with the lifetime `'a` as defined here
   --> f202.rs:1:14
    |
1   | struct Hello<'a> {
    |              ^^
note: but lifetime parameter must outlive the static lifetime
   --> /home/gh-estebank/rust/library/core/src/any.rs:113:16
    |
113 | pub trait Any: 'static {
    |                ^^^^^^^
```

Partially address rust-lang#33652.
…links, r=Kobzol

bootstrap: fix clean's remove_dir_all implementation

It turns out bootstrap's `clean.rs`'s hand-rolled `rm_rf` (which probably comes before `std::fs::remove_dir_all` was stable) is very broken on native Windows around both read-only files/directories and especially symbolic links. So instead of rolling our own, just use `std::fs::remove_dir_all`.

This is a blocker for compiletest's own `rm_rf` implementation rust-lang#129155 which happens to be also buggy, which in turn is a blocker for the rmake.rs test port rust-lang#128562 that heavily exercises symlinks (I was reviewing rust-lang#128562 and testing it on native Windows which is how I found out).

I also left a FIXME for `detect_src_and_out` due to a failing assertion on native Windows (opened rust-lang#129188):

```
---- core::config::tests::detect_src_and_out stdout ----
thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:72:13:
assertion `left == right` failed
  left: "E:\\tmp"
 right: "C:\\tmp"
```

Fixes rust-lang#112544 (because now we handle Windows symlinks properly).

try-job: x86_64-msvc
try-job: i686-mingw
try-job: test-various
try-job: armhf-gnu
try-job: aarch64-apple
try-job: aarch64-gnu
…es, r=Mark-Simulacrum

improve submodule updates

During config parsing, some bootstrap logic (e.g., `download-ci-llvm`) checks certain sources (for `download-ci-llvm`, it's `src/llvm-project`) and acts based on their state. This means that if path is a git submodule, bootstrap needs to update it before checking its state. Otherwise it may make incorrect assumptions by relying on outdated sources. To enable submodule updates during config parsing, we need to move the `update_submodule` function from the `Build` to `Config`, so we can access to it during the parsing process.

Closes rust-lang#122787
…r=Kobzol

Update `library/Cargo.toml` in weekly job

Before the workspace split, the library was covered by the weekly `cargo update` cron job. Now that the library has its own workspace, it doesn't get these updates.

Add `library/Cargo.toml` to the job so updates happen again.
…=light, r=GuillaumeGomez

rustdoc: animate the `:target` highlight

This approach is, roughly, based on how Discourse does it. It came up while discussing [some other possible sidebar changes](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/Moving.20deprecated.20items.20out.20of.20the.20way), as a design that made rapid scanning easier while avoiding the inherent trade-offs in summarizing.

https://github.com/user-attachments/assets/f7a8fec3-70a5-40a1-92ea-bfdffbd61c22
…iler-errors

compiletest: use `std::fs::remove_dir_all` now that it is available

It turns out `aggressive_rm_rf` is not sufficiently aggressive (RAGEY) on Windows and obviously handles Windows symlinks incorrectly. Instead of rolling our own version, let's use `std::fs::remove_dir_all` now that it's available (well, it's been available for a good while, but probably wasn't available when this helper was written).

cc rust-lang#129187 since basically this is failing due to similar problems.

Blocker for rust-lang#128562.
Fixes rust-lang#129155.
Fixes rust-lang#126334.
@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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 21, 2024
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] miri test:false 4.447
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Thu, Aug 22, 2024  1:38:32 AM
  network time: Thu, 22 Aug 2024 01:38:33 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Contributor

bors commented Aug 22, 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 Aug 22, 2024
@matthiaskrgr
Copy link
Member Author

@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 Aug 22, 2024
@bors
Copy link
Contributor

bors commented Aug 22, 2024

⌛ Testing commit 33dc313 with merge 739b1fd...

@jieyouxu
Copy link
Member

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Pinging back #127883.

@bors
Copy link
Contributor

bors commented Aug 22, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 22, 2024
@bors bors merged commit 739b1fd into rust-lang:master Aug 22, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 22, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#127279 use old ctx if has same expand environment during decode sp… ff4d0214b98f67a324dfa1baacd12e9acff8cde9 (link)
#127945 Implement debug_more_non_exhaustive dbd21e736435ff67822b5f5d2a275695efaaaaa2 (link)
#128941 Improve diagnostic-related lints: `untranslatable_diagnost… c33147e9dc881cb6438141121866506e7ec4bbbf (link)
#129070 Point at explicit 'static obligations on a trait 5b3c73b689e744287c1d68878f546a54e2a4c660 (link)
#129187 bootstrap: fix clean's remove_dir_all implementation 726f362c204ca6c5f8ee2e3e5c6d85bfb832901e (link)
#129231 improve submodule updates c2dd28ac5aeab1472b63ab7e7226aba7d8befa16 (link)
#129264 Update library/Cargo.toml in weekly job c93ab77cff0c82a45734508057e60f17ba3e1ec8 (link)
#129284 rustdoc: animate the :target highlight 509761db6186471755c69a16e477a9666cd0e648 (link)
#129302 compiletest: use std::fs::remove_dir_all now that it is a… dc10160b0eeef592010b5d6e571242beda7802e1 (link)

previous master: a32d4a0e82

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 (739b1fd): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.3% [0.2%, 0.4%] 15
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [0.2%, 0.4%] 15

Max RSS (memory usage)

Results (primary -4.4%, secondary 2.4%)

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.4% [2.4%, 2.4%] 1
Improvements ✅
(primary)
-4.4% [-4.4%, -4.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -4.4% [-4.4%, -4.4%] 1

Cycles

Results (primary -2.3%, secondary -1.5%)

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)
-2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)
-1.5% [-2.1%, -0.9%] 2
All ❌✅ (primary) -2.3% [-2.3%, -2.3%] 1

Binary size

Results (primary 0.3%, secondary 0.2%)

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.3% [0.0%, 1.4%] 43
Regressions ❌
(secondary)
0.2% [0.0%, 0.4%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [0.0%, 1.4%] 43

Bootstrap: 750.761s -> 750.993s (0.03%)
Artifact size: 338.92 MiB -> 338.96 MiB (0.01%)

@pnkfelix
Copy link
Member

@rust-timer build ff4d021

@rust-timer

This comment has been minimized.

@pnkfelix
Copy link
Member

visiting for weekly rustc-perf triage.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ff4d021): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.3% [0.2%, 0.4%] 15
Regressions ❌
(secondary)
1.6% [1.6%, 1.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [0.2%, 0.4%] 15

Max RSS (memory usage)

Results (primary -2.3%, secondary 2.8%)

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.5% [0.5%, 0.5%] 1
Regressions ❌
(secondary)
2.8% [2.8%, 2.8%] 1
Improvements ✅
(primary)
-3.2% [-4.1%, -2.5%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.3% [-4.1%, 0.5%] 4

Cycles

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

Binary size

Results (primary 0.3%, secondary 0.2%)

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.3% [0.0%, 1.4%] 42
Regressions ❌
(secondary)
0.2% [0.0%, 0.4%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.0% [-0.0%, -0.0%] 1
All ❌✅ (primary) 0.3% [0.0%, 1.4%] 42

Bootstrap: 750.761s -> 750.844s (0.01%)
Artifact size: 338.92 MiB -> 338.96 MiB (0.01%)

@pnkfelix
Copy link
Member

The above seems to confirm that PR #127279 is the cause of the reported regression.

However, I do not think the performance hit is significant enough to warrant reverting that PR, compared to the impact that would have on the incremental compilation scenarios described in issue #112680. (We may nonetheless want to explore replacing that PR with a fix for the potential deeper problem alluded to by @cjgillot and @petrochenkov on PR #127279.)

@pnkfelix
Copy link
Member

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Aug 29, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-ebwx6ya branch September 1, 2024 17:35
@jieyouxu jieyouxu added the CI-spurious-fail-msvc CI spurious failure: target env msvc label Oct 15, 2024
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 CI-spurious-fail-msvc CI spurious failure: target env msvc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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-libs Relevant to the library 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.