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

@veluca93 veluca93 mentioned this pull request Jul 7, 2026
Open
@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

fee1-dead commented Jul 9, 2026

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

veluca93 commented Jul 18, 2026

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 requested changes Jul 22, 2026
View reviewed changes

@fee1-dead fee1-dead left a comment •
edited by rustbot
Loading

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

veluca93 commented Jul 22, 2026

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

wesleywiser commented Jul 23, 2026

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 previously requested changes Jul 24, 2026
View reviewed changes

@scottmcm scottmcm left a comment •
edited by rustbot
Loading

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
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]]

@scottmcm scottmcm Jul 24, 2026

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...)

@veluca93 veluca93 Jul 25, 2026

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

scottmcm commented Jul 25, 2026

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 •
edited
Loading

Copy link
Copy Markdown
Contributor

@bors p=101
Scheduling

@rust-bors

This comment has been minimized.

Sign in to view
@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

github-actions Bot commented Jul 26, 2026

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

rust-timer commented Jul 26, 2026

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

Reviewers

@fee1-dead fee1-dead fee1-dead requested changes
@wesleywiser wesleywiser wesleywiser approved these changes
@scottmcm scottmcm scottmcm left review comments

Assignees

@scottmcm scottmcm

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

Milestone

1.99.0

Development

Successfully merging this pull request may close these issues.

9 participants

@veluca93 @rustbot @fee1-dead @tmandry @wesleywiser @scottmcm @JonathanBrouwer @rust-timer @mejrs