Skip to content

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

Open
veluca93 wants to merge 1 commit into
rust-lang:mainfrom
veluca93:optimize-attr-closure-inherit
Open

Closures inherit #[optimize] from the enclosing function by default.#158901
veluca93 wants to merge 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

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

Requested changes must be addressed to merge this pull request.

Assignees

@scottmcm scottmcm

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@veluca93 @rustbot @fee1-dead @tmandry @wesleywiser @scottmcm @mejrs