Skip to content

comments for deduce_param_attrs #147873

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 19, 2025
Merged

comments for deduce_param_attrs #147873

merged 1 commit into from
Oct 19, 2025

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Oct 19, 2025 •
edited
Loading

Cc @saethlin since IIRC you experimented with codegen doing post-mono MIR ops? That seems to be in conflict with this pass.
Cc @tmiasko

r? @scottmcm

@rustbot
Copy link
Collaborator

rustbot commented Oct 19, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added 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 Oct 19, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 19, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 19, 2025

Lol, #147695 recently changed this quite a bit and I thought my 24h old branch was new enough...^^ That PR subsumes the one logic change I had here; now this just adds some comments (most of which are minor, but the one about MIR opts during codegen seems crucial).

Cc @cjgillot

@RalfJung RalfJung changed the title comments and tweaks for deduce_param_attrs comments for deduce_param_attrs Oct 19, 2025
@tmiasko
Copy link
Contributor

tmiasko commented Oct 19, 2025

Could you also update the pull request description (it references code changes which are no longer there)?

Thanks. r=me

RalfJung
RalfJung commented Oct 19, 2025
View reviewed changes
compiler/rustc_mir_transform/src/deduce_param_attrs.rs
Copy link
Member Author

@RalfJung RalfJung Oct 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Codegen actually uses instance_mir so using optimized_mir here seems a bit inconsistent?
Things could go quite wrong if they end up using different MIR...

@rustbot
Copy link
Collaborator

rustbot commented Oct 19, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@RalfJung
Copy link
Member Author

RalfJung commented Oct 19, 2025

Could you also update the pull request description (it references code changes which are no longer there)?

Done. I've also added comments in codegen itself.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 19, 2025

@bors r=tmiasko rollup

@bors
Copy link
Collaborator

bors commented Oct 19, 2025

📌 Commit 82f6c60 has been approved by tmiasko

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 Oct 19, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 19, 2025
Merged
@saethlin
Copy link
Member

saethlin commented Oct 19, 2025

you experimented with codegen doing post-mono MIR ops? That seems to be in conflict with this pass.

It sure is, this pass is the primary reason post-mono opts are stuck.

@tmiasko
Copy link
Contributor

tmiasko commented Oct 19, 2025

If we decide on post mono MIR passes I think it would be fine to remove attribute deduction.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 19, 2025 via email

bors added a commit that referenced this pull request Oct 19, 2025
@bors
Auto merge of #147884 - matthiaskrgr:rollup-pjqcccz, r=matthiaskrgr
b4f28c6 
Rollup of 4 pull requests

Successful merges:

 - #147575 (Refactor move analysis subpath representation)
 - #147864 (Parse `const unsafe trait` properly)
 - #147868 (MirPatch: Simplify new_local.)
 - #147873 (comments for deduce_param_attrs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Oct 19, 2025
@bors
Auto merge of #147884 - matthiaskrgr:rollup-pjqcccz, r=matthiaskrgr
4ddbb60 
Rollup of 4 pull requests

Successful merges:

 - #147575 (Refactor move analysis subpath representation)
 - #147864 (Parse `const unsafe trait` properly)
 - #147868 (MirPatch: Simplify new_local.)
 - #147873 (comments for deduce_param_attrs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 362c9e2 into rust-lang:master Oct 19, 2025
11 checks passed
rust-timer added a commit that referenced this pull request Oct 19, 2025
@rust-timer
Unrolled build for #147873
1be004b 
Rollup merge of #147873 - RalfJung:deduce_param_attrs, r=tmiasko

comments for deduce_param_attrs

Cc `@saethlin` since IIRC you experimented with codegen doing post-mono MIR ops? That seems to be in conflict with this pass.
Cc `@tmiasko`

r? `@scottmcm`
@rustbot rustbot added this to the 1.92.0 milestone Oct 19, 2025
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Oct 20, 2025
@bors
Auto merge of #147884 - matthiaskrgr:rollup-pjqcccz, r=matthiaskrgr
9e3b484 
Rollup of 4 pull requests

Successful merges:

 - rust-lang/rust#147575 (Refactor move analysis subpath representation)
 - rust-lang/rust#147864 (Parse `const unsafe trait` properly)
 - rust-lang/rust#147868 (MirPatch: Simplify new_local.)
 - rust-lang/rust#147873 (comments for deduce_param_attrs)

r? `@ghost`
`@rustbot` modify labels: rollup
@RalfJung RalfJung deleted the deduce_param_attrs branch October 20, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tmiasko tmiasko tmiasko approved these changes

Assignees

@scottmcm scottmcm

Labels

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

1.92.0

Development

Successfully merging this pull request may close these issues.

6 participants

@RalfJung @rustbot @tmiasko @bors @saethlin @scottmcm