-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Don't expose redundant information in rustc_public's LayoutShape
#151040
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
Conversation
Enum variant layouts don't need to store a full `LayoutShape`; just storing
the fields offsets is enough and all other information can be inferred from
the parent layout:
- size, align and ABI don't make much sense for individual variants and
should generally be taken from the parent layout instead;
- variants always have `fields: FieldsShape::Arbitrary { .. }` and
`variant: VariantShape::Single { .. }`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks good to me. Per past rustc_public development experience, we usually just chase the unstable rustc, whereas I think it's the first time we're changing rustc_public first :) But anyway it's a good cleanup, so
|
⌛ Testing commit 3777ebc with merge 7cfb9e0... Workflow: https://github.com/rust-lang/rust/actions/runs/21367599744 |
Don't expose redundant information in `rustc_public`'s `LayoutShape`
Enum variant layouts don't need to store a full `LayoutShape`; just storing the fields offsets is enough and all other information can be inferred from the parent layout:
- size, align and ABI don't make much sense for individual variants and should generally be taken from the parent layout instead;
- variants always have `fields: FieldsShape::Arbitrary { .. }` and `variant: VariantShape::Single { .. }`.
In principle, the same refactor could be done on `rustc_abi::Layout` (see [this comment](#113988 (comment))) but I prefer starting with this smaller change first.
|
Auto build cancelled. Cancelled workflows: The next pull request likely to be tested is #151040. |
…akai410
Don't expose redundant information in `rustc_public`'s `LayoutShape`
Enum variant layouts don't need to store a full `LayoutShape`; just storing the fields offsets is enough and all other information can be inferred from the parent layout:
- size, align and ABI don't make much sense for individual variants and should generally be taken from the parent layout instead;
- variants always have `fields: FieldsShape::Arbitrary { .. }` and `variant: VariantShape::Single { .. }`.
In principle, the same refactor could be done on `rustc_abi::Layout` (see [this comment](rust-lang#113988 (comment))) but I prefer starting with this smaller change first.
…akai410
Don't expose redundant information in `rustc_public`'s `LayoutShape`
Enum variant layouts don't need to store a full `LayoutShape`; just storing the fields offsets is enough and all other information can be inferred from the parent layout:
- size, align and ABI don't make much sense for individual variants and should generally be taken from the parent layout instead;
- variants always have `fields: FieldsShape::Arbitrary { .. }` and `variant: VariantShape::Single { .. }`.
In principle, the same refactor could be done on `rustc_abi::Layout` (see [this comment](rust-lang#113988 (comment))) but I prefer starting with this smaller change first.
…akai410
Don't expose redundant information in `rustc_public`'s `LayoutShape`
Enum variant layouts don't need to store a full `LayoutShape`; just storing the fields offsets is enough and all other information can be inferred from the parent layout:
- size, align and ABI don't make much sense for individual variants and should generally be taken from the parent layout instead;
- variants always have `fields: FieldsShape::Arbitrary { .. }` and `variant: VariantShape::Single { .. }`.
In principle, the same refactor could be done on `rustc_abi::Layout` (see [this comment](rust-lang#113988 (comment))) but I prefer starting with this smaller change first.
Rollup of 6 pull requests Successful merges: - #148718 (Do not mention `-Zmacro-backtrace` for std macros that are a wrapper around a compiler intrinsic) - #151137 (checksum-freshness: Fix invalid checksum calculation for binary files) - #150863 (Adds two new Tier 3 targets - `aarch64v8r-unknown-none{,-softfloat}`) - #151040 (Don't expose redundant information in `rustc_public`'s `LayoutShape`) - #151699 (Update books) - #151700 (os allow missing_docs)
Rollup of 12 pull requests Successful merges: - #147996 (Stabilize ppc inline assembly) - #148718 (Do not mention `-Zmacro-backtrace` for std macros that are a wrapper around a compiler intrinsic) - #151137 (checksum-freshness: Fix invalid checksum calculation for binary files) - #151680 (Update backtrace and windows-bindgen) - #150863 (Adds two new Tier 3 targets - `aarch64v8r-unknown-none{,-softfloat}`) - #151040 (Don't expose redundant information in `rustc_public`'s `LayoutShape`) - #151383 (remove `#[deprecated]` from unstable & internal `SipHasher13` and `24` types) - #151529 (lint: Use rustc_apfloat for `overflowing_literals`, add f16 and f128) - #151669 (rename uN::{gather,scatter}_bits to uN::{extract,deposit}_bits) - #151689 (Fix broken Xtensa installation link) - #151699 (Update books) - #151700 (os allow missing_docs)
Rollup merge of #151040 - moulins:public-variant-layout, r=makai410 Don't expose redundant information in `rustc_public`'s `LayoutShape` Enum variant layouts don't need to store a full `LayoutShape`; just storing the fields offsets is enough and all other information can be inferred from the parent layout: - size, align and ABI don't make much sense for individual variants and should generally be taken from the parent layout instead; - variants always have `fields: FieldsShape::Arbitrary { .. }` and `variant: VariantShape::Single { .. }`. In principle, the same refactor could be done on `rustc_abi::Layout` (see [this comment](#113988 (comment))) but I prefer starting with this smaller change first.
Rollup of 12 pull requests Successful merges: - rust-lang/rust#147996 (Stabilize ppc inline assembly) - rust-lang/rust#148718 (Do not mention `-Zmacro-backtrace` for std macros that are a wrapper around a compiler intrinsic) - rust-lang/rust#151137 (checksum-freshness: Fix invalid checksum calculation for binary files) - rust-lang/rust#151680 (Update backtrace and windows-bindgen) - rust-lang/rust#150863 (Adds two new Tier 3 targets - `aarch64v8r-unknown-none{,-softfloat}`) - rust-lang/rust#151040 (Don't expose redundant information in `rustc_public`'s `LayoutShape`) - rust-lang/rust#151383 (remove `#[deprecated]` from unstable & internal `SipHasher13` and `24` types) - rust-lang/rust#151529 (lint: Use rustc_apfloat for `overflowing_literals`, add f16 and f128) - rust-lang/rust#151669 (rename uN::{gather,scatter}_bits to uN::{extract,deposit}_bits) - rust-lang/rust#151689 (Fix broken Xtensa installation link) - rust-lang/rust#151699 (Update books) - rust-lang/rust#151700 (os allow missing_docs)
Enum variant layouts don't need to store a full
LayoutShape; just storing the fields offsets is enough and all other information can be inferred from the parent layout:fields: FieldsShape::Arbitrary { .. }andvariant: VariantShape::Single { .. }.In principle, the same refactor could be done on
rustc_abi::Layout(see this comment) but I prefer starting with this smaller change first.