Skip to content

Avoid ref when using format! in src #127984

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jul 20, 2024
Merged

Avoid ref when using format! in src #127984

merged 1 commit into from
Jul 20, 2024

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented Jul 19, 2024

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

@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2024

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@nyurik nyurik changed the title Avoid ref when using format! in compiler Avoid ref when using format! in src Jul 19, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

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.
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@onur-ozkan
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 20, 2024

📌 Commit 8bcf0b4 has been approved by onur-ozkan

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 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 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 bors merged commit 40cfc88 into rust-lang:master Jul 20, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
Rollup merge of rust-lang#127984 - nyurik:src-refs, r=onur-ozkan

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
@nyurik nyurik deleted the src-refs branch July 20, 2024 15:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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-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.

5 participants