Skip to content

uses_power_alignment: wording tweaks #139059

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 2 commits into from
Apr 30, 2025
Merged

uses_power_alignment: wording tweaks #139059

merged 2 commits into from
Apr 30, 2025
+14 −21

Conversation

RalfJung
Copy link
Member

Slightly improves the wording introduced with #135552.

@rustbot
Copy link
Collaborator

rustbot commented Mar 28, 2025

r? @estebank

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

@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 Mar 28, 2025
@RalfJung RalfJung force-pushed the uses_power_alignment branch from 661de9f to 83a3329 Compare March 28, 2025 17:34
@RalfJung
Copy link
Member Author

RalfJung commented Mar 29, 2025 •
edited
Loading

r? @workingjubilee
Seems like there's new developments, I'll update the PR once we figure out what we should say here.

@rustbot rustbot assigned workingjubilee and unassigned estebank Mar 29, 2025
@workingjubilee workingjubilee 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 Mar 29, 2025
@RalfJung RalfJung force-pushed the uses_power_alignment branch from 83a3329 to 8fca246 Compare April 1, 2025 14:31
@RalfJung RalfJung force-pushed the uses_power_alignment branch from 8fca246 to 87ff60c Compare April 1, 2025 14:33
@RalfJung
Copy link
Member Author

RalfJung commented Apr 1, 2025 •
edited
Loading

I've updated the PR based on this comment. Obviously we'll want more updates to eventually fix the alignment of f64, but that's a bigger change.

I also noticed an odd variable name, first_field_def for what is clearly not the first field but the "current" field, so I fixed that. And then I realized the code can be simplified quite a bit so I did that instead. ;)

@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 Apr 1, 2025
@RalfJung RalfJung mentioned this pull request Apr 1, 2025
Merged
RalfJung
RalfJung commented Apr 1, 2025
View reviewed changes
compiler/rustc_lint/src/types.rs
@@ -1621,15 +1623,13 @@ impl ImproperCTypesDefinitions {
cx: &LateContext<'tcx>,
ty: Ty<'tcx>,
) -> bool {
assert!(cx.tcx.sess.target.os == "aix");
Copy link
Member Author

Choose a reason for hiding this comment

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

This function should be dead code on non-AIX targets so I was confused why it would check the target again here.

@RalfJung
Copy link
Member Author

It's been 4 weeks without a review... @nikic @nagisa @bjorn3 , could you take a look? It's mostly just wording, though some fact-checking is probably a good idea.

@nagisa
Copy link
Member

nagisa commented Apr 29, 2025

@bors r+

This looks good to me. The code changes all seem equivalent and wording is clearer as to what are the implications of this warning.

@bors
Copy link
Collaborator

bors commented Apr 29, 2025

📌 Commit 87ff60c has been approved by nagisa

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 Apr 29, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 30, 2025
@Zalathar
Rollup merge of rust-lang#139059 - RalfJung:uses_power_alignment, r=n…
f52fde8 
…agisa

uses_power_alignment: wording tweaks

Slightly improves the wording introduced with rust-lang#135552.
@Zalathar Zalathar mentioned this pull request Apr 30, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 30, 2025
@bors
Auto merge of rust-lang#140499 - Zalathar:rollup-8qpbuw0, r=Zalathar
c9d3ed8 
Rollup of 12 pull requests

Successful merges:

 - rust-lang#136160 (Remove backticks from `ShouldPanic::YesWithMessage`'s `TrFailedMsg`)
 - rust-lang#138087 (Initial implementation of `core_float_math`)
 - rust-lang#139059 (uses_power_alignment: wording tweaks)
 - rust-lang#139192 (mention provenance in the pointer::wrapping_offset docs)
 - rust-lang#140312 (Improve pretty-printing of braces)
 - rust-lang#140404 (rm `TypeVistable` impls for `Canonical`)
 - rust-lang#140437 (enable msa feature for mips in codegen tests)
 - rust-lang#140438 (Add `rust.debug-assertions-tools` option)
 - rust-lang#140439 (miri: algebraic intrinsics: bring back float non-determinism)
 - rust-lang#140445 (Treat ManuallyDrop as ~const Destruct)
 - rust-lang#140446 (chore: fix some tests)
 - rust-lang#140448 (Rename `rustc_query_append!` to `rustc_with_all_queries!`)

Failed merges:

 - rust-lang#140456 (Fix test simd/extract-insert-dyn on s390x)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 30, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 30, 2025
@bors
Auto merge of rust-lang#140503 - matthiaskrgr:rollup-n7zigts, r=matth…
7188f45 
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#136160 (Remove backticks from `ShouldPanic::YesWithMessage`'s `TrFailedMsg`)
 - rust-lang#139059 (uses_power_alignment: wording tweaks)
 - rust-lang#139192 (mention provenance in the pointer::wrapping_offset docs)
 - rust-lang#140312 (Improve pretty-printing of braces)
 - rust-lang#140404 (rm `TypeVistable` impls for `Canonical`)
 - rust-lang#140437 (enable msa feature for mips in codegen tests)
 - rust-lang#140438 (Add `rust.debug-assertions-tools` option)
 - rust-lang#140439 (miri: algebraic intrinsics: bring back float non-determinism)
 - rust-lang#140445 (Treat ManuallyDrop as ~const Destruct)
 - rust-lang#140446 (chore: fix some tests)
 - rust-lang#140448 (Rename `rustc_query_append!` to `rustc_with_all_queries!`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e897144 into rust-lang:master Apr 30, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 30, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 30, 2025
@rust-timer
Unrolled build for rust-lang#139059
92528cf 
Rollup merge of rust-lang#139059 - RalfJung:uses_power_alignment, r=nagisa

uses_power_alignment: wording tweaks

Slightly improves the wording introduced with rust-lang#135552.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@workingjubilee workingjubilee

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.88.0
Development

Successfully merging this pull request may close these issues.
6 participants
@RalfJung @rustbot @nagisa @bors @estebank @workingjubilee