Skip to content

inherit #[align] from trait method prototypes #143156

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

folkertdev
Copy link
Contributor

@workingjubilee this seems straightforward enough. Now that we're planning to make -Cmin-function-alignment a target modifier, I don't think there are any cross-crate complications here?

@Jules-Bertholet is this the behavior you had in mind? In particular the inheritance of the attribute of a default impl is maybe a bit unintuitive at first? (but I think it's ok if that behavior is explicitly documented).

r? ghost

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 28, 2025
@Jules-Bertholet
Copy link
Contributor

Maybe add a few tests for multiple align attributes on the same item, combined with traits? Other than that and my 1 comment, LGTM

@folkertdev folkertdev force-pushed the fn-align-inherit-from-trait branch from 923f7fc to 3bc59cd Compare June 28, 2025 20:57
@workingjubilee
Copy link
Member

@workingjubilee this seems straightforward enough. Now that we're planning to make -Cmin-function-alignment a target modifier, I don't think there are any cross-crate complications here?

@folkertdev I can't promise this won't somehow flow back into concerns for the implementation of the feature we're stabilizing in that PR, but I agree this should be enough for here.

@workingjubilee
Copy link
Member

In particular, "there is one universal rule for how this trait's code should be aligned" reduces the number of things we need to worry about when trying to correctly instantiate the alignment of that code's functions.

@folkertdev
Copy link
Contributor Author

Sure, we'll see how that develops.

At least we've thought about this now, and have something concrete to create tests against if the need arises.

r? @workingjubilee

@folkertdev folkertdev marked this pull request as ready for review June 28, 2025 21:05
@rustbot
Copy link
Collaborator

rustbot commented Jun 28, 2025

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 28, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs

cc @jdonszelmann

@folkertdev folkertdev force-pushed the fn-align-inherit-from-trait branch from 3bc59cd to bcf5105 Compare June 29, 2025 15:23
@workingjubilee
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 29, 2025

📌 Commit bcf5105 has been approved by workingjubilee

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 Jun 29, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 29, 2025
…rait, r=workingjubilee

inherit `#[align]` from trait method prototypes

`@workingjubilee` this seems straightforward enough. Now that we're planning to make `-Cmin-function-alignment` a target modifier, I don't think there are any cross-crate complications here?

`@Jules-Bertholet` is this the behavior you had in mind? In particular the inheritance of the attribute of a default impl is maybe a bit unintuitive at first? (but I think it's ok if that behavior is explicitly documented).

r? ghost
bors added a commit that referenced this pull request Jun 29, 2025
Rollup of 7 pull requests

Successful merges:

 - #142429 (`tests/ui`: A New Order [13/N])
 - #143066 (Use let chains in the new solver)
 - #143156 (inherit `#[align]` from trait method prototypes)
 - #143176 (fix typos and improve clarity in documentation)
 - #143187 (Add my work email to mailmap)
 - #143194 (fix bitcast of single-element SIMD vectors)
 - #143199 (Re-disable `tests/run-make/short-ice` on Windows MSVC again)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 30, 2025
…rait, r=workingjubilee

inherit `#[align]` from trait method prototypes

``@workingjubilee`` this seems straightforward enough. Now that we're planning to make `-Cmin-function-alignment` a target modifier, I don't think there are any cross-crate complications here?

``@Jules-Bertholet`` is this the behavior you had in mind? In particular the inheritance of the attribute of a default impl is maybe a bit unintuitive at first? (but I think it's ok if that behavior is explicitly documented).

r? ghost
bors added a commit that referenced this pull request Jun 30, 2025
Rollup of 7 pull requests

Successful merges:

 - #142429 (`tests/ui`: A New Order [13/N])
 - #143066 (Use let chains in the new solver)
 - #143156 (inherit `#[align]` from trait method prototypes)
 - #143176 (fix typos and improve clarity in documentation)
 - #143187 (Add my work email to mailmap)
 - #143199 (Re-disable `tests/run-make/short-ice` on Windows MSVC again)
 - #143219 (Show auto trait and blanket impls for `!`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 30, 2025
…rait, r=workingjubilee

inherit `#[align]` from trait method prototypes

```@workingjubilee``` this seems straightforward enough. Now that we're planning to make `-Cmin-function-alignment` a target modifier, I don't think there are any cross-crate complications here?

```@Jules-Bertholet``` is this the behavior you had in mind? In particular the inheritance of the attribute of a default impl is maybe a bit unintuitive at first? (but I think it's ok if that behavior is explicitly documented).

r? ghost
bors added a commit that referenced this pull request Jun 30, 2025
Rollup of 5 pull requests

Successful merges:

 - #143156 (inherit `#[align]` from trait method prototypes)
 - #143223 (Improve macro stats printing)
 - #143229 ([COMPILETEST-UNTANGLE 1/N] Move some some early config checks to the lib and move the compiletest binary)
 - #143239 (GCC backend subtree update)
 - #143246 (Subtree update of `rust-analyzer`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants