Skip to content

Closures inherit #[optimize] from the enclosing function by default.#158901

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
veluca93:optimize-attr-closure-inherit
Jul 26, 2026
Merged

Closures inherit #[optimize] from the enclosing function by default.#158901
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
veluca93:optimize-attr-closure-inherit

Conversation

@veluca93

@veluca93 veluca93 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Tracking issue: #54882
Stabilization PR: #157273

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 7, 2026
@rustbot

rustbot commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 19 candidates

@fee1-dead fee1-dead added S-waiting-on-t-lang Status: Awaiting decision from T-lang and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2026
@fee1-dead

Copy link
Copy Markdown
Member

Waiting on t-lang per #157273 (comment)

@fee1-dead fee1-dead added I-lang-nominated Nominated for discussion during a lang team meeting. and removed I-lang-nominated Nominated for discussion during a lang team meeting. labels Jul 9, 2026
@tmandry

tmandry commented Jul 16, 2026

Copy link
Copy Markdown
Member

The lang team discussed this change today and was in favor: #157562 (comment)

@veluca93

Copy link
Copy Markdown
Contributor Author

I don't believe this is blocked on lang anymore, but I don't think I can remove the tag myself.

@mejrs mejrs added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-t-lang Status: Awaiting decision from T-lang labels Jul 21, 2026

@fee1-dead fee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

r? scottmcm
could you check if the codegen-llvm CHECK annotations here are idiomatic or is there a better way?

View changes since this review

Comment thread tests/codegen-llvm/optimize-closures-inheritance.rs
@rustbot rustbot assigned scottmcm and unassigned fee1-dead Jul 22, 2026
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 22, 2026
@rustbot

rustbot commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@veluca93
veluca93 force-pushed the optimize-attr-closure-inherit branch from 3c5ebd5 to f24fa6f Compare July 22, 2026 05:49
@veluca93

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 22, 2026
@wesleywiser

Copy link
Copy Markdown
Member

FileCheck tests look fine to me but maybe @scottmcm knows better 🙂

@bors rollup=never

Could have perf implications from changing effective optimization values for closures.

@scottmcm scottmcm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cg_ssa bit of this looks plausible to me; just a couple thoughts about the codegen test.

(Feel free to push back if they won't work or get ugly for some reason.)

View changes since this review

Comment thread tests/codegen-llvm/optimize-closures-inheritance.rs Outdated
#![crate_type = "lib"]

// CHECK-DAG: define{{.*}}void {{.*}}test_none{{.*}}call_once{{.*}}#[[ATTR_NONE:[0-9]+]]
// CHECK-DAG: define{{.*}}void {{.*}}test_none{{.*}}B{{[0-9]+}}_() {{.*}}#[[ATTR_NONE]]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Possibly a silly question: what's that B from?

(I get worried about anything too specific in name checks because we've been bitten before by people doing things like %[[FOO]].0 and depending on the exact internal LLVM naming of SRoA results and such. This is probably not that bad, but I'm confused what it is, so...)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I personally don't know, but $LLM seems to think it comes from "Rust's v0 symbol mangling specification (specifically compression back-references for path components, e.g. B3_)". However with no-prepopulate-passes this doesn't seem needed anymore, so ... :-)

(I don't really understand FileCheck)

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2026
@veluca93
veluca93 force-pushed the optimize-attr-closure-inherit branch from f24fa6f to cca3d28 Compare July 25, 2026 18:42
@scottmcm

Copy link
Copy Markdown
Member

@bors r=wesleywiser,scottmcm

@rust-bors

rust-bors Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

📌 Commit cca3d28 has been approved by wesleywiser,scottmcm

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

Reason for tree closure: spurious failures

@rust-bors rust-bors Bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 25, 2026
@scottmcm
scottmcm dismissed their stale review July 25, 2026 23:57

it's my review

@JonathanBrouwer

JonathanBrouwer commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

@bors p=101
Scheduling

@rust-bors

This comment has been minimized.

@rust-bors rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 26, 2026
@rust-bors

rust-bors Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

☀️ Test successful - CI
Approved by: wesleywiser,scottmcm
Duration: 3h 7m 14s
Pushing 5e2e873 to main...

@rust-bors
rust-bors Bot merged commit 5e2e873 into rust-lang:main Jul 26, 2026
14 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor
What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 9451e06 (parent) -> 5e2e873 (this PR)

Test differences

Show 5 test diffs

Stage 1

  • [codegen] tests/codegen-llvm/optimize-closures-inheritance.rs: [missing] -> pass (J1)

Stage 2

  • [codegen] tests/codegen-llvm/optimize-closures-inheritance.rs: [missing] -> pass (J0)

Additionally, 3 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 5e2e8734b6a10a88879b8732fec64dd524894793 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-rust-for-linux: 31m 38s -> 47m 25s (+49.9%)
  2. dist-i686-msvc: 1h 27m -> 2h 4m (+43.4%)
  3. test-various: 1h 28m -> 2h 4m (+40.1%)
  4. dist-powerpc64le-linux-gnu: 1h 15m -> 1h 40m (+33.0%)
  5. dist-x86_64-musl: 1h 45m -> 2h 16m (+29.8%)
  6. arm-android: 1h 19m -> 1h 42m (+29.6%)
  7. dist-powerpc64le-linux-musl: 1h 19m -> 1h 37m (+23.5%)
  8. x86_64-gnu-gcc-core-tests: 8m 10s -> 6m 16s (-23.4%)
  9. x86_64-gnu-distcheck: 2h 13m -> 1h 42m (-23.0%)
  10. dist-x86_64-freebsd: 1h 11m -> 1h 27m (+22.6%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (5e2e873): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.1%, secondary 0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.1% [2.1%, 2.1%] 1
Regressions ❌
(secondary)
1.7% [1.2%, 2.2%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) 2.1% [2.1%, 2.1%] 1

Cycles

Results (secondary -3.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.4% [-3.7%, -3.0%] 2
All ❌✅ (primary) - - 0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.176s -> 487.552s (-0.33%)
Artifact size: 387.73 MiB -> 387.70 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) merged-by-bors This PR was explicitly merged by bors. 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