Skip to content

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented Sep 28, 2025

Much of the compiler calls functions on Align projected from AbiAlign. AbiAlign impls Deref to its inner Align, so we can simplify these away. Also, it will minimize disruption when AbiAlign is removed.

For now, preserve usages that might resolve to PartialOrd or PartialEq, as those have odd inference.

Same sentiment as #147116 in that this seemed the most reviewable chunk to submit.

@rustbot
Copy link
Collaborator

rustbot commented Sep 28, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

HIR ty lowering was modified

cc @fmease

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Sep 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 28, 2025

r? @lcnr

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

@workingjubilee workingjubilee force-pushed the remove-explicit-abialign-deref branch from f798c9d to 9f404e6 Compare September 28, 2025 21:52
@rust-log-analyzer

This comment has been minimized.

Much of the compiler calls functions on Align projected from AbiAlign.
AbiAlign impls Deref to its inner Align, so we can simplify these away.
Also, it will minimize disruption when AbiAlign is removed.

For now, preserve usages that might resolve to PartialOrd or PartialEq,
as those have odd inference.
@workingjubilee workingjubilee force-pushed the remove-explicit-abialign-deref branch from 9f404e6 to 0c9d0df Compare September 28, 2025 22:02
(i128_type, u128_type)
} else {
/*let layout = tcx.layout_of(ParamEnv::reveal_all().and(tcx.types.i128)).unwrap();
let i128_align = layout.align.abi.bytes();
Copy link
Contributor

Choose a reason for hiding this comment

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

Remark: This code is commented out, but I guess including it in a find-replace isn't wrong.

) {
let l = eval_goal(ra_fixture, minicore).unwrap();
assert_eq!(l.size.bytes(), size, "size mismatch");
assert_eq!(l.align.abi.bytes(), align, "align mismatch");
Copy link
Contributor

Choose a reason for hiding this comment

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

Remark: From a review perspective it's unclear whether these rust-analyzer changes are intended/accurate (and not just a find-replace mishap), but they seem plausible and I guess CI would complain if they were broken, so it seems fine to rubber-stamp them.

Copy link
Member

Choose a reason for hiding this comment

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

most of r-a's test suite currently does not run on CI #136779

The changes should be proper though.

@Zalathar
Copy link
Contributor

My remarks don't require any further action, so LGTM.

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 29, 2025

📌 Commit 0c9d0df has been approved by Zalathar

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 Sep 29, 2025
bors added a commit that referenced this pull request Sep 29, 2025
Rollup of 3 pull requests

Successful merges:

 - #147100 (tests: Remove ignore-android directive for fixed issue)
 - #147116 (compiler: remove AbiAlign inside TargetDataLayout)
 - #147134 (remove explicit deref of AbiAlign for most methods)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cd6f32a into rust-lang:master Sep 29, 2025
10 checks passed
rust-timer added a commit that referenced this pull request Sep 29, 2025
Rollup merge of #147134 - workingjubilee:remove-explicit-abialign-deref, r=Zalathar

remove explicit deref of AbiAlign for most methods

Much of the compiler calls functions on Align projected from AbiAlign. AbiAlign impls Deref to its inner Align, so we can simplify these away. Also, it will minimize disruption when AbiAlign is removed.

For now, preserve usages that might resolve to PartialOrd or PartialEq, as those have odd inference.
@rustbot rustbot added this to the 1.92.0 milestone Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants