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

Merged
merged 31 commits into from
Jul 20, 2024
Merged

Rollup of 9 pull requests #127998

merged 31 commits into from
Jul 20, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Zalathar and others added 30 commits July 10, 2024 20:25
This comment has two problems:

- It is very long, making the flow of the enclosing method hard to follow.
- It starts by talking about an `autoref` flag that hasn't existed since rust-lang#59114.

This PR therefore replaces the long inline comment with a revised doc comment
on `bind_matched_candidate_for_guard`, and some shorter inline comments.

For readers who want more historical context, we also link to the PR that added
the old comment, and the PR that removed the `autoref` flag.
Signed-off-by: Ayush Singh <[email protected]>
Use a custom simple_text_output protocol to capture output.

Signed-off-by: Ayush Singh <[email protected]>
Implement stderr support in similar fashion.

Signed-off-by: Ayush Singh <[email protected]>
Only tested in 2 levels right now. Need args support for 3 levels

Signed-off-by: Ayush Singh <[email protected]>
Also fix stdio inherit

Signed-off-by: Ayush Singh <[email protected]>
- Update system table crc32
- Fix unsound use of Box
- Free exit data
- Code improvements
- Introduce OwnedTable
- Update r-efi to latest version
- Use extended_varargs_abi_support for
  install_multiple_protocol_interfaces and
  uninstall_multiple_protocol_interfaces
- Fix comments
- Stub out args implementation

Signed-off-by: Ayush Singh <[email protected]>
This commit updates the support for the `wasm-component-ld` tool
from rust-lang#126967 to conditionally build it rather than unconditionally
building it when LLD is enabled. This support is disabled by default and
can be enabled by one of two means:

* the `extended` field in `config.toml` which dist builders use to build
  a complete set of tools for each host platform.
* a `"wasm-component-ld"` entry in the `tools` section of `config.toml`.

Neither of these are enabled by default meaning that most local builds
will likely not have this new tool built. Dist builders should still,
however, build the tool.
Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing).  Inlining format args prevents accidental `&` misuse.
When encountering `-> Trait`, suggest `-> Box<dyn Trait>` (instead of `-> Box<Trait>`.

If there's a single returned type within the `fn`, suggest `-> impl Trait`.
Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing).  Inlining format args prevents accidental `&` misuse.
Add Process support for UEFI

UEFI does not have an actual process. However, it does provide methods to launch and execute another UEFI image. Having process support is important since it is possible to run rust test suit using `Command::output` and is the first step towards being able to run it for UEFI.

Here is an overview of how the support is implemented.

- We create a copy of the SystemTable. This is required since at least OVMF seems to crash if the original system table is modified.
- Stdout and Stderr pipe works by registering a new `simple_text_output` Protocol and pointing the child system table to use those.
- `Stdio::Inherit` just points the console to the current running image console which seems to work with even 3 levels of process.
- `spawn` is left unimplemented since it does not make sense for UEFI architecture. Additionally, since rust-lang#105458 was merged, the `spawn` and `output` implementations are completely independent.
Replace a long inline "autoref" comment with method docs

This comment has two problems:

- It is very long, making the flow of the enclosing method hard to follow.
- It starts by talking about an `autoref` flag that hasn't existed since rust-lang#59114.
  - This makes it hard to trust that the information in the comment is accurate or relevant, even though much of it still seems to be true.

This PR therefore replaces the long inline comment with a revised doc comment on `bind_matched_candidate_for_guard`, and some shorter inline comments.

For readers who want more historical context, we also link to the PR that added the old comment, and the PR that removed the `autoref` flag.
…sion-rmake, r=jieyouxu

Migrate `crate-hash-rustc-version` to `rmake`

Part of rust-lang#121876.

r? ``@jieyouxu``

try-job: x86_64-gnu-llvm-18
try-job: dist-x86_64-linux
…t-ld-by-default, r=onur-ozkan

Conditionally build `wasm-component-ld`

This commit updates the support for the `wasm-component-ld` tool from rust-lang#126967 to conditionally build rather than unconditionally building it when LLD is enabled. This support is disabled by default and can be enabled by one of two means:

* the `extended` field in `config.toml` which dist builders use to build a complete set of tools for each host platform.
* a `"wasm-component-ld"` entry in the `tools` section of `config.toml`.

Neither of these are enabled by default meaning that most local builds will likely not have this new tool built. Dist builders should still, however, build the tool.
…joboet

Safely enforce thread name requirements

The requirements for the thread name to be both UTF-8 and null terminated are easily enforced by a wrapper type so lets do that. The fact this used to be just a bare `CString` has tripped me up before because it was entirely safe to use a non UTF-8 `CString`.
…rors

fixes panic error `index out of bounds` in conflicting error

fixes rust-lang#127915
Avoid ref when using format! in compiler

Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing).  Inlining format args prevents accidental `&` misuse.

See also rust-lang/rust-clippy#10851
Avoid ref when using format! in src

Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing).  Inlining format args prevents accidental `&` misuse.

See also rust-lang/rust-clippy#10851
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=9

@bors
Copy link
Contributor

bors commented Jul 20, 2024

📌 Commit 89798e9 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 Jul 20, 2024
@bors
Copy link
Contributor

bors commented Jul 20, 2024

⌛ Testing commit 89798e9 with merge c247b86...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#123196 (Add Process support for UEFI)
 - rust-lang#127556 (Replace a long inline "autoref" comment with method docs)
 - rust-lang#127693 (Migrate `crate-hash-rustc-version` to `rmake`)
 - rust-lang#127866 (Conditionally build `wasm-component-ld` )
 - rust-lang#127918 (Safely enforce thread name requirements)
 - rust-lang#127948 (fixes panic error `index out of bounds` in conflicting error)
 - rust-lang#127980 (Avoid ref when using format! in compiler)
 - rust-lang#127984 (Avoid ref when using format! in src)
 - rust-lang#127987 (More accurate suggestion for `-> Box<dyn Trait>` or `-> impl Trait`)

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

bors commented Jul 20, 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 Jul 20, 2024
@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)

@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 Jul 20, 2024
@bors
Copy link
Contributor

bors commented Jul 20, 2024

⌛ Testing commit 89798e9 with merge 1afc5fd...

@bors
Copy link
Contributor

bors commented Jul 20, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 20, 2024
@bors bors merged commit 1afc5fd into rust-lang:master Jul 20, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 20, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#123196 Add Process support for UEFI 1b3c26161c602218fc2bfb67ec1ca50f13e1ca89 (link)
#127556 Replace a long inline "autoref" comment with method docs f2e82b1c522c6e753ac9ae06972423e900af4741 (link)
#127693 Migrate crate-hash-rustc-version to rmake 94d4fdf69e24eacd47c5bdc91c930f19578f6a2d (link)
#127866 Conditionally build wasm-component-ld 5c35c07ca720441da2e0fc728aa7be4c4d44fd9d (link)
#127918 Safely enforce thread name requirements aeaddb0aff36c23de968487f36b0004fdd8e1750 (link)
#127948 fixes panic error index out of bounds in conflicting error 5c2870e1d77eb51733520f0173f38ecb028cce8d (link)
#127980 Avoid ref when using format! in compiler d1d08e43a9b36fba94e1dd6ed3760046fc1a413f (link)
#127984 Avoid ref when using format! in src 3e08940004f7e17562d9b80e789d4f9f00abfce8 (link)
#127987 More accurate suggestion for -> Box<dyn Trait> or `-> imp… 91399e3fda12a22bb738f5767adcd93a0c390eb7 (link)

previous master: 41ff460889

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 (1afc5fd): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.3%, 0.3%] 2
Regressions ❌
(secondary)
1.3% [1.3%, 1.3%] 1
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.7%, 0.3%] 3

Max RSS (memory usage)

Results (primary 0.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)
4.6% [3.9%, 5.3%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.7% [-3.2%, -2.2%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.9% [-3.2%, 5.3%] 4

Cycles

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

Binary size

Results (primary -0.1%, secondary 0.0%)

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.0% [0.0%, 0.2%] 8
Regressions ❌
(secondary)
0.0% [0.0%, 0.0%] 19
Improvements ✅
(primary)
-0.1% [-0.2%, -0.1%] 14
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.2%, 0.2%] 22

Bootstrap: 769.168s -> 769.548s (0.05%)
Artifact size: 328.88 MiB -> 328.86 MiB (-0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jul 20, 2024
@Mark-Simulacrum
Copy link
Member

@rust-timer build aeaddb0

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (aeaddb0): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.3%] 2
Regressions ❌
(secondary)
4.4% [0.3%, 11.7%] 3
Improvements ✅
(primary)
-0.8% [-0.8%, -0.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.8%, 0.3%] 3

Max RSS (memory usage)

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

Cycles

Results (primary -1.0%, secondary 4.7%)

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

Binary size

Results (primary -0.1%, secondary 0.0%)

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.0% [0.0%, 0.2%] 8
Regressions ❌
(secondary)
0.0% [0.0%, 0.0%] 19
Improvements ✅
(primary)
-0.1% [-0.2%, -0.1%] 14
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.2%, 0.2%] 22

Bootstrap: 769.168s -> 769.93s (0.10%)
Artifact size: 328.88 MiB -> 328.81 MiB (-0.02%)

@matthiaskrgr matthiaskrgr deleted the rollup-ykp0h5r branch September 1, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. PG-exploit-mitigations Project group: Exploit mitigations 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.