make repr_transparent_non_zst_fields a hard error - #155299
Conversation
|
rustbot has assigned @jdonszelmann. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@bors try |
This comment has been minimized.
This comment has been minimized.
…try> make repr_transparent_non_zst_fields a hard error
This comment has been minimized.
This comment has been minimized.
5b074cf to
3bb6c15
Compare
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
This looks nice Ralf, indeed let's nominate for lang after this completes |
This comment has been minimized.
This comment has been minimized.
|
Note that this also forbids The custom phantom variance markers in See also CAD97/generativity#13 where this was first reported as an issue with my crate. I didn't say anything at that point as I presumed it wouldn't be made into a hard error without some way to opt-in to being a trivial type for |
|
Uh, that's interesting that you would assume that when the error very clearly states what our plans are and nothing in the tracking issue indicates other plans either... why did you wait years until the literal last moment before raising this concern upstream? You could have saved me a bunch of work by posting this any time in the last 3 years. :( |
use std::marker::PhantomData;
#[repr(transparent)]
pub struct Phantomish<T>(PhantomData<T>);
#[repr(transparent)]
pub struct Transparent<T>(u8, Phantomish<T>);Edit: ah, I see, you need two crates… |
|
FWIW I don't think it's necessary to block turning this into a hard error on some way to make 1ZST with private members. I just want to ensure that this knowingly effects It's ultimately quite minor. |
|
@CAD97 is there an issue tracking that? It'd be nice to have a good self-contained writeup for what you think is currently missing. |
|
FWIW I'd also be fine with only making the repr(C) part of this a hard error, and letting the part that is motivated by semver concerns bake for longer. But there hasn't been any movement on that semver side nor even any suggestions for how the rules should be relaxed so that's not very actionable. Let's see what crater says. Last time we tried this, we found only 2 regressions due to the semver rule; your crate did not show up. So either crater bugged out a bit (spurious failure in the baseline build?) or nothing covered by crater uses your crate with this pattern you mention. |
3bb6c15 to
3dfa7ea
Compare
This comment has been minimized.
This comment has been minimized.
|
Cc @obi1kenobi as this is about removing a currently-existing semver hazard 🎉 |
|
Another breakage: the FCW currently triggers when one tries to use a |
|
That's exactly #155925 isn't it? |
…fields, r=jdonszelmann make repr_transparent_non_zst_fields a hard error This lint is about which fields we consider "trivial" for `repr(transparent)`. For `repr(transparent)` to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of the `repr(transparent)` type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back: - Types might be 1-ZST today but that's not actually meant to be a semver guarantee, so it's bad for downstream code to rely on it. Therefore we should not accept types that have private fields or that are marked `#[non_exhaustive]` (except when we are in the same crate as that type). - Types might be 1-ZST but still be relevant for the ABI because they are `repr(C)` and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With rust-lang/rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets. This was a deny-by-default FCW since rust-lang#147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The [tracking issue](rust-lang#78586) has had no new relevant backrefs since that PR landed. So, I think it is time to make this a hard error. Fixes rust-lang#78586 (tracking issue) Fixes rust-lang/unsafe-code-guidelines#552 because this means the `repr(transparent)` ABI compatibility rule no longer ever "ignores" `repr(C)` fields. @rust-lang/lang What do you think? See [here](rust-lang#155299 (comment)) for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people: - The [`ghost`](https://github.com/dtolnay/ghost) crate offers a macro to define `PhantomData`-like types, and those types involve a `repr(C)`. If someone uses such a type as a marker inside a `repr(transparent)`, that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this. - rust-lang#155925 is unresolved: there is currently no way for a crate to say "yes this type has private field but I promise it will remain a 1-ZST" (except via the unstable `#[rustc_pub_transparent]`). That means it is not possible for a crate to expose a semantically relevant maker type (like `GhostToken`) that is "trivial" for `repr(transparent)` purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make the `repr(C)` and `#[non_exhaustive]` part of the check a hard error, and leave the "has private fields" part as a warning. Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do *not* check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?
…fields, r=jdonszelmann make repr_transparent_non_zst_fields a hard error This lint is about which fields we consider "trivial" for `repr(transparent)`. For `repr(transparent)` to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of the `repr(transparent)` type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back: - Types might be 1-ZST today but that's not actually meant to be a semver guarantee, so it's bad for downstream code to rely on it. Therefore we should not accept types that have private fields or that are marked `#[non_exhaustive]` (except when we are in the same crate as that type). - Types might be 1-ZST but still be relevant for the ABI because they are `repr(C)` and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With rust-lang/rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets. This was a deny-by-default FCW since rust-lang#147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The [tracking issue](rust-lang#78586) has had no new relevant backrefs since that PR landed. So, I think it is time to make this a hard error. Fixes rust-lang#78586 (tracking issue) Fixes rust-lang/unsafe-code-guidelines#552 because this means the `repr(transparent)` ABI compatibility rule no longer ever "ignores" `repr(C)` fields. @rust-lang/lang What do you think? See [here](rust-lang#155299 (comment)) for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people: - The [`ghost`](https://github.com/dtolnay/ghost) crate offers a macro to define `PhantomData`-like types, and those types involve a `repr(C)`. If someone uses such a type as a marker inside a `repr(transparent)`, that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this. - rust-lang#155925 is unresolved: there is currently no way for a crate to say "yes this type has private field but I promise it will remain a 1-ZST" (except via the unstable `#[rustc_pub_transparent]`). That means it is not possible for a crate to expose a semantically relevant maker type (like `GhostToken`) that is "trivial" for `repr(transparent)` purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make the `repr(C)` and `#[non_exhaustive]` part of the check a hard error, and leave the "has private fields" part as a warning. Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do *not* check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?
…fields, r=jdonszelmann make repr_transparent_non_zst_fields a hard error This lint is about which fields we consider "trivial" for `repr(transparent)`. For `repr(transparent)` to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of the `repr(transparent)` type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back: - Types might be 1-ZST today but that's not actually meant to be a semver guarantee, so it's bad for downstream code to rely on it. Therefore we should not accept types that have private fields or that are marked `#[non_exhaustive]` (except when we are in the same crate as that type). - Types might be 1-ZST but still be relevant for the ABI because they are `repr(C)` and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With rust-lang/rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets. This was a deny-by-default FCW since rust-lang#147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The [tracking issue](rust-lang#78586) has had no new relevant backrefs since that PR landed. So, I think it is time to make this a hard error. Fixes rust-lang#78586 (tracking issue) Fixes rust-lang/unsafe-code-guidelines#552 because this means the `repr(transparent)` ABI compatibility rule no longer ever "ignores" `repr(C)` fields. @rust-lang/lang What do you think? See [here](rust-lang#155299 (comment)) for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people: - The [`ghost`](https://github.com/dtolnay/ghost) crate offers a macro to define `PhantomData`-like types, and those types involve a `repr(C)`. If someone uses such a type as a marker inside a `repr(transparent)`, that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this. - rust-lang#155925 is unresolved: there is currently no way for a crate to say "yes this type has private field but I promise it will remain a 1-ZST" (except via the unstable `#[rustc_pub_transparent]`). That means it is not possible for a crate to expose a semantically relevant maker type (like `GhostToken`) that is "trivial" for `repr(transparent)` purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make the `repr(C)` and `#[non_exhaustive]` part of the check a hard error, and leave the "has private fields" part as a warning. Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do *not* check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?
…fields, r=jdonszelmann make repr_transparent_non_zst_fields a hard error This lint is about which fields we consider "trivial" for `repr(transparent)`. For `repr(transparent)` to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of the `repr(transparent)` type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back: - Types might be 1-ZST today but that's not actually meant to be a semver guarantee, so it's bad for downstream code to rely on it. Therefore we should not accept types that have private fields or that are marked `#[non_exhaustive]` (except when we are in the same crate as that type). - Types might be 1-ZST but still be relevant for the ABI because they are `repr(C)` and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With rust-lang/rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets. This was a deny-by-default FCW since rust-lang#147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The [tracking issue](rust-lang#78586) has had no new relevant backrefs since that PR landed. So, I think it is time to make this a hard error. Fixes rust-lang#78586 (tracking issue) Fixes rust-lang/unsafe-code-guidelines#552 because this means the `repr(transparent)` ABI compatibility rule no longer ever "ignores" `repr(C)` fields. @rust-lang/lang What do you think? See [here](rust-lang#155299 (comment)) for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people: - The [`ghost`](https://github.com/dtolnay/ghost) crate offers a macro to define `PhantomData`-like types, and those types involve a `repr(C)`. If someone uses such a type as a marker inside a `repr(transparent)`, that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this. - rust-lang#155925 is unresolved: there is currently no way for a crate to say "yes this type has private field but I promise it will remain a 1-ZST" (except via the unstable `#[rustc_pub_transparent]`). That means it is not possible for a crate to expose a semantically relevant maker type (like `GhostToken`) that is "trivial" for `repr(transparent)` purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make the `repr(C)` and `#[non_exhaustive]` part of the check a hard error, and leave the "has private fields" part as a warning. Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do *not* check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?
…uwer Rollup of 23 pull requests Successful merges: - #157716 (update Enzyme, June'26) - #149793 (Add inline asm support for amdgpu) - #152852 (Remove driver_lint_caps) - #155299 (make repr_transparent_non_zst_fields a hard error) - #155439 (Enable Cargo's new build-dir layout) - #157612 (Add a test where subtyping inhibits coercion.) - #157626 (Autogenerate unstable compiler flag stubs for unstable-book) - #157667 (Rename typing modes to better describe real usage) - #156212 (Additionally gate negative bounds behind new `-Zinternal-testing-features`) - #157342 (Reduce verbosity of cycle errors when possible) - #157366 (Add a regression test for an unconstrained TransmuteFrom ICE) - #157459 (rustc_target: callconv: powerpc64: Remove unreachable fallback code path) - #157658 (UnsafeCell: mention shared-ref-to-interior case, fix aliasing model inaccuracy) - #157698 (Remove an unnecessary cloning) - #157699 (Arg splat experiment - hir FnDecl impl) - #157713 (resolve: Remove exported imports from `maybe_unused_trait_imports`) - #157722 (Move create_scope_map to rustc_codegen_ssa.) - #157725 (Keep generic suggestion for macro-expanded missing-type items) - #157733 (Remove old FIXMEs about nocapture attribute) - #157737 (Reorganize `tests/ui/issues` [7/N]) - #157746 (supports_c_variadic_definitions: extend checklist for new targets) - #157763 (Move unused target expression error to appropriate place and rename it) - #157768 (codegen_ssa: peel trans. wrappers on scalable vecs)
…uwer Rollup of 23 pull requests Successful merges: - #157716 (update Enzyme, June'26) - #149793 (Add inline asm support for amdgpu) - #152852 (Remove driver_lint_caps) - #155299 (make repr_transparent_non_zst_fields a hard error) - #155439 (Enable Cargo's new build-dir layout) - #157612 (Add a test where subtyping inhibits coercion.) - #157626 (Autogenerate unstable compiler flag stubs for unstable-book) - #157667 (Rename typing modes to better describe real usage) - #156212 (Additionally gate negative bounds behind new `-Zinternal-testing-features`) - #157342 (Reduce verbosity of cycle errors when possible) - #157366 (Add a regression test for an unconstrained TransmuteFrom ICE) - #157459 (rustc_target: callconv: powerpc64: Remove unreachable fallback code path) - #157658 (UnsafeCell: mention shared-ref-to-interior case, fix aliasing model inaccuracy) - #157698 (Remove an unnecessary cloning) - #157699 (Arg splat experiment - hir FnDecl impl) - #157713 (resolve: Remove exported imports from `maybe_unused_trait_imports`) - #157722 (Move create_scope_map to rustc_codegen_ssa.) - #157725 (Keep generic suggestion for macro-expanded missing-type items) - #157733 (Remove old FIXMEs about nocapture attribute) - #157737 (Reorganize `tests/ui/issues` [7/N]) - #157746 (supports_c_variadic_definitions: extend checklist for new targets) - #157763 (Move unused target expression error to appropriate place and rename it) - #157768 (codegen_ssa: peel trans. wrappers on scalable vecs)
…fields, r=jdonszelmann make repr_transparent_non_zst_fields a hard error This lint is about which fields we consider "trivial" for `repr(transparent)`. For `repr(transparent)` to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of the `repr(transparent)` type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back: - Types might be 1-ZST today but that's not actually meant to be a semver guarantee, so it's bad for downstream code to rely on it. Therefore we should not accept types that have private fields or that are marked `#[non_exhaustive]` (except when we are in the same crate as that type). - Types might be 1-ZST but still be relevant for the ABI because they are `repr(C)` and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With rust-lang/rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets. This was a deny-by-default FCW since rust-lang#147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The [tracking issue](rust-lang#78586) has had no new relevant backrefs since that PR landed. So, I think it is time to make this a hard error. Fixes rust-lang#78586 (tracking issue) Fixes rust-lang/unsafe-code-guidelines#552 because this means the `repr(transparent)` ABI compatibility rule no longer ever "ignores" `repr(C)` fields. @rust-lang/lang What do you think? See [here](rust-lang#155299 (comment)) for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people: - The [`ghost`](https://github.com/dtolnay/ghost) crate offers a macro to define `PhantomData`-like types, and those types involve a `repr(C)`. If someone uses such a type as a marker inside a `repr(transparent)`, that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this. - rust-lang#155925 is unresolved: there is currently no way for a crate to say "yes this type has private field but I promise it will remain a 1-ZST" (except via the unstable `#[rustc_pub_transparent]`). That means it is not possible for a crate to expose a semantically relevant maker type (like `GhostToken`) that is "trivial" for `repr(transparent)` purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make the `repr(C)` and `#[non_exhaustive]` part of the check a hard error, and leave the "has private fields" part as a warning. Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do *not* check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?
…uwer Rollup of 23 pull requests Successful merges: - #157716 (update Enzyme, June'26) - #149793 (Add inline asm support for amdgpu) - #155299 (make repr_transparent_non_zst_fields a hard error) - #155439 (Enable Cargo's new build-dir layout) - #157612 (Add a test where subtyping inhibits coercion.) - #157626 (Autogenerate unstable compiler flag stubs for unstable-book) - #157667 (Rename typing modes to better describe real usage) - #149749 (Make `BorrowedBuf` and `BorrowedCursor` generic over the data) - #156212 (Additionally gate negative bounds behind new `-Zinternal-testing-features`) - #157342 (Reduce verbosity of cycle errors when possible) - #157366 (Add a regression test for an unconstrained TransmuteFrom ICE) - #157459 (rustc_target: callconv: powerpc64: Remove unreachable fallback code path) - #157658 (UnsafeCell: mention shared-ref-to-interior case, fix aliasing model inaccuracy) - #157698 (Remove an unnecessary cloning) - #157699 (Arg splat experiment - hir FnDecl impl) - #157713 (resolve: Remove exported imports from `maybe_unused_trait_imports`) - #157722 (Move create_scope_map to rustc_codegen_ssa.) - #157725 (Keep generic suggestion for macro-expanded missing-type items) - #157733 (Remove old FIXMEs about nocapture attribute) - #157737 (Reorganize `tests/ui/issues` [7/N]) - #157746 (supports_c_variadic_definitions: extend checklist for new targets) - #157763 (Move unused target expression error to appropriate place and rename it) - #157768 (codegen_ssa: peel trans. wrappers on scalable vecs)
Rollup of 24 pull requests Successful merges: - #157716 (update Enzyme, June'26) - #149793 (Add inline asm support for amdgpu) - #155299 (make repr_transparent_non_zst_fields a hard error) - #157612 (Add a test where subtyping inhibits coercion.) - #157626 (Autogenerate unstable compiler flag stubs for unstable-book) - #157667 (Rename typing modes to better describe real usage) - #149749 (Make `BorrowedBuf` and `BorrowedCursor` generic over the data) - #155113 (Ensure Send/Sync impl for std::process::CommandArgs) - #156212 (Additionally gate negative bounds behind new `-Zinternal-testing-features`) - #157342 (Reduce verbosity of cycle errors when possible) - #157366 (Add a regression test for an unconstrained TransmuteFrom ICE) - #157459 (rustc_target: callconv: powerpc64: Remove unreachable fallback code path) - #157658 (UnsafeCell: mention shared-ref-to-interior case, fix aliasing model inaccuracy) - #157698 (Remove an unnecessary cloning) - #157699 (Arg splat experiment - hir FnDecl impl) - #157713 (resolve: Remove exported imports from `maybe_unused_trait_imports`) - #157722 (Move create_scope_map to rustc_codegen_ssa.) - #157723 (Move uninhabited unreachable code lint to rustc_mir_transform) - #157725 (Keep generic suggestion for macro-expanded missing-type items) - #157733 (Remove old FIXMEs about nocapture attribute) - #157737 (Reorganize `tests/ui/issues` [7/N]) - #157746 (supports_c_variadic_definitions: extend checklist for new targets) - #157763 (Move unused target expression error to appropriate place and rename it) - #157768 (codegen_ssa: peel trans. wrappers on scalable vecs)
Rollup merge of #155299 - RalfJung:repr_transparent_non_zst_fields, r=jdonszelmann make repr_transparent_non_zst_fields a hard error This lint is about which fields we consider "trivial" for `repr(transparent)`. For `repr(transparent)` to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of the `repr(transparent)` type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back: - Types might be 1-ZST today but that's not actually meant to be a semver guarantee, so it's bad for downstream code to rely on it. Therefore we should not accept types that have private fields or that are marked `#[non_exhaustive]` (except when we are in the same crate as that type). - Types might be 1-ZST but still be relevant for the ABI because they are `repr(C)` and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With rust-lang/rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets. This was a deny-by-default FCW since #147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The [tracking issue](#78586) has had no new relevant backrefs since that PR landed. So, I think it is time to make this a hard error. Fixes #78586 (tracking issue) Fixes rust-lang/unsafe-code-guidelines#552 because this means the `repr(transparent)` ABI compatibility rule no longer ever "ignores" `repr(C)` fields. @rust-lang/lang What do you think? See [here](#155299 (comment)) for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people: - The [`ghost`](https://github.com/dtolnay/ghost) crate offers a macro to define `PhantomData`-like types, and those types involve a `repr(C)`. If someone uses such a type as a marker inside a `repr(transparent)`, that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this. - #155925 is unresolved: there is currently no way for a crate to say "yes this type has private field but I promise it will remain a 1-ZST" (except via the unstable `#[rustc_pub_transparent]`). That means it is not possible for a crate to expose a semantically relevant maker type (like `GhostToken`) that is "trivial" for `repr(transparent)` purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make the `repr(C)` and `#[non_exhaustive]` part of the check a hard error, and leave the "has private fields" part as a warning. Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do *not* check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?
…=jdonszelmann make repr_transparent_non_zst_fields a hard error This lint is about which fields we consider "trivial" for `repr(transparent)`. For `repr(transparent)` to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of the `repr(transparent)` type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back: - Types might be 1-ZST today but that's not actually meant to be a semver guarantee, so it's bad for downstream code to rely on it. Therefore we should not accept types that have private fields or that are marked `#[non_exhaustive]` (except when we are in the same crate as that type). - Types might be 1-ZST but still be relevant for the ABI because they are `repr(C)` and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With rust-lang/rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets. This was a deny-by-default FCW since rust-lang/rust#147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The [tracking issue](rust-lang/rust#78586) has had no new relevant backrefs since that PR landed. So, I think it is time to make this a hard error. Fixes rust-lang/rust#78586 (tracking issue) Fixes rust-lang/unsafe-code-guidelines#552 because this means the `repr(transparent)` ABI compatibility rule no longer ever "ignores" `repr(C)` fields. @rust-lang/lang What do you think? See [here](rust-lang/rust#155299 (comment)) for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people: - The [`ghost`](https://github.com/dtolnay/ghost) crate offers a macro to define `PhantomData`-like types, and those types involve a `repr(C)`. If someone uses such a type as a marker inside a `repr(transparent)`, that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this. - rust-lang/rust#155925 is unresolved: there is currently no way for a crate to say "yes this type has private field but I promise it will remain a 1-ZST" (except via the unstable `#[rustc_pub_transparent]`). That means it is not possible for a crate to expose a semantically relevant maker type (like `GhostToken`) that is "trivial" for `repr(transparent)` purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make the `repr(C)` and `#[non_exhaustive]` part of the check a hard error, and leave the "has private fields" part as a warning. Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do *not* check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?
Rollup of 24 pull requests Successful merges: - rust-lang/rust#157716 (update Enzyme, June'26) - rust-lang/rust#149793 (Add inline asm support for amdgpu) - rust-lang/rust#155299 (make repr_transparent_non_zst_fields a hard error) - rust-lang/rust#157612 (Add a test where subtyping inhibits coercion.) - rust-lang/rust#157626 (Autogenerate unstable compiler flag stubs for unstable-book) - rust-lang/rust#157667 (Rename typing modes to better describe real usage) - rust-lang/rust#149749 (Make `BorrowedBuf` and `BorrowedCursor` generic over the data) - rust-lang/rust#155113 (Ensure Send/Sync impl for std::process::CommandArgs) - rust-lang/rust#156212 (Additionally gate negative bounds behind new `-Zinternal-testing-features`) - rust-lang/rust#157342 (Reduce verbosity of cycle errors when possible) - rust-lang/rust#157366 (Add a regression test for an unconstrained TransmuteFrom ICE) - rust-lang/rust#157459 (rustc_target: callconv: powerpc64: Remove unreachable fallback code path) - rust-lang/rust#157658 (UnsafeCell: mention shared-ref-to-interior case, fix aliasing model inaccuracy) - rust-lang/rust#157698 (Remove an unnecessary cloning) - rust-lang/rust#157699 (Arg splat experiment - hir FnDecl impl) - rust-lang/rust#157713 (resolve: Remove exported imports from `maybe_unused_trait_imports`) - rust-lang/rust#157722 (Move create_scope_map to rustc_codegen_ssa.) - rust-lang/rust#157723 (Move uninhabited unreachable code lint to rustc_mir_transform) - rust-lang/rust#157725 (Keep generic suggestion for macro-expanded missing-type items) - rust-lang/rust#157733 (Remove old FIXMEs about nocapture attribute) - rust-lang/rust#157737 (Reorganize `tests/ui/issues` [7/N]) - rust-lang/rust#157746 (supports_c_variadic_definitions: extend checklist for new targets) - rust-lang/rust#157763 (Move unused target expression error to appropriate place and rename it) - rust-lang/rust#157768 (codegen_ssa: peel trans. wrappers on scalable vecs)
|
The 1.98 beta crater run found another regression that seems to be due to this PR. I assume this is acceptable? |
|
Last updated 4 years ago, failure in code generated by an outdated |
|
And the rustdoc crater run found one more regression due to this PR. Again, I assume this is acceptable? |
|
Looks like another unused crate that wasn't updated in 5 years. |
This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [rust](https://github.com/rust-lang/rust) | tools | minor | `1.97.1` → `1.98.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>rust-lang/rust (rust)</summary> ### [`v1.98.0`](https://github.com/rust-lang/rust/blob/HEAD/RELEASES.md#Version-1980-2026-08-20) [Compare Source](rust-lang/rust@1.97.1...1.98.0) \========================== <a id="1.98.0-Language"></a> ## Language - [Allow shortening lifetime of `&mut` when unsize-coercing, even in an invariant position.](rust-lang/rust#149219) For example, you can now coerce a `Cell<&'long mut i32>` to a `Cell<&'short mut dyn Send>`. Such shortenings were already previously allowed when coercing a `&mut` to a `&`, or coercing a `&` to a `&`. - [Add deny-by-default `invalid_runtime_symbol_definitions` lint and warn-by-default `suspicious_runtime_symbol_definitions` lint](rust-lang/rust#155521) - The lints currently specifically targets `core` runtime symbols like `memcmp`, `memset`, `strlen`, ... and is planned to be expanded in the next few releases. - [Add warn-by-default `c_void_returns` lint to check `core::ffi::c_void` as a return type](rust-lang/rust#156379) <a id="1.98.0-Platform-Support"></a> ## Platform Support - [Add `powerpc64-unknown-linux-gnuelfv2` as Tier 3](rust-lang/rust#144220) - [Add `aarch64-unknown-linux-pauthtest` as Tier 3 target](rust-lang/rust#155722) - [Promote `thumbv7a-none-eabi` to Tier 2](rust-lang/rust#155763) - [Promote `thumbv7a-none-eabihf` to Tier 2](rust-lang/rust#155763) - [Promote `thumbv7r-none-eabi` to Tier 2](rust-lang/rust#155763) - [Promote `thumbv7r-none-eabihf` to Tier 2](rust-lang/rust#155763) - [Promote `thumbv8r-none-eabihf` to Tier 2](rust-lang/rust#155763) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. [platform-support-doc]: https://doc.rust-lang.org/rustc/platform-support.html <a id="1.98.0-Libraries"></a> ## Libraries - [Change `Location<'_>` lifetime to `'static` in `Panic[Hook]Info`](rust-lang/rust#146561) - [Document panic in `RangeInclusive::from(legacy::RangeInclusive)`](rust-lang/rust#155421) - [Document that `ManuallyDrop`'s `Box` interaction has been fixed](rust-lang/rust#155750) - [Stabilize LoongArch CRC Intrinsics](rust-lang/rust#156908) - [The `derive` macro is available at `{core,std}::derive`.](rust-lang/rust#154645) This was previously [unintentionally stabilized in 1.96](rust-lang/rust#159856), but is now [explicitly accepted](rust-lang/rust#154645) as a stabilized API. - Please note that the MSRV for `{core,std}::derive` will be 1.96, and not 1.98. <a id="1.98.0-Stabilized-APIs"></a> ## Stabilized APIs - [`str::substr_range`](https://doc.rust-lang.org/stable/std/primitive.str.html#method.substr_range) - [`[T]::subslice_range`](https://doc.rust-lang.org/stable/std/primitive.slice.html#method.subslice_range) - [`core::fmt::NumBuffer`](https://doc.rust-lang.org/stable/core/fmt/struct.NumBuffer.html) - [`<{integer}>::format_into`](https://doc.rust-lang.org/stable/core/primitive.usize.html#method.format_into) - [`Send/Sync for std::process::CommandArgs`](https://doc.rust-lang.org/stable/std/process/struct.CommandArgs.html#impl-Send-for-CommandArgs%3C'a%3E) - [`{fN}::algebraic_add`](https://doc.rust-lang.org/stable/core/primitive.f32.html#method.algebraic_add) - [`{fN}::algebraic_sub`](https://doc.rust-lang.org/stable/core/primitive.f32.html#method.algebraic_sub) - [`{fN}::algebraic_mul`](https://doc.rust-lang.org/stable/core/primitive.f32.html#method.algebraic_mul) - [`{fN}::algebraic_div`](https://doc.rust-lang.org/stable/core/primitive.f32.html#method.algebraic_div) - [`{fN}::algebraic_rem`](https://doc.rust-lang.org/stable/core/primitive.f32.html#method.algebraic_rem) - [`NonZero<{integer}>::from_str_radix`](https://doc.rust-lang.org/stable/core/num/struct.NonZero.html#method.from_str_radix-4) - [`String::from_utf16le`](https://doc.rust-lang.org/stable/std/string/struct.String.html#method.from_utf16le) - [`String::from_utf16le_lossy`](https://doc.rust-lang.org/stable/std/string/struct.String.html#method.from_utf16le_lossy) - [`String::from_utf16be`](https://doc.rust-lang.org/stable/std/string/struct.String.html#method.from_utf16be) - [`String::from_utf16be_lossy`](https://doc.rust-lang.org/stable/std/string/struct.String.html#method.from_utf16be_lossy) - [`[T]::strip_circumfix`](https://doc.rust-lang.org/stable/core/primitive.slice.html#method.strip_circumfix) - [`str::strip_circumfix`](https://doc.rust-lang.org/stable/core/primitive.str.html#method.strip_circumfix) - [`Atomic<T>::from_mut`](https://doc.rust-lang.org/stable/core/sync/atomic/struct.Atomic.html#method.from_mut) - [`Atomic<T>::get_mut_slice`](https://doc.rust-lang.org/stable/core/sync/atomic/struct.Atomic.html#method.get_mut_slice) - [`Atomic<T>::from_mut_slice`](https://doc.rust-lang.org/stable/core/sync/atomic/struct.Atomic.html#method.from_mut_slice) - [`std::range::legacy`](https://doc.rust-lang.org/stable/std/range/legacy/index.html) <a id="1.98.0-Compatibility-Notes"></a> ## Compatibility Notes - [If fully elided, lifetime bounds of trait object types may now resolve differently or even get rejected in very specific niche scenarios](rust-lang/rust#129543) - [Error in more cases of ambiguous imports](rust-lang/rust#145108) - [Switch the destructors implementation for thread locals on Windows to use Fiber Local Storage (FLS)](rust-lang/rust#148799) - [Convert some cases of the `ambiguous_glob_imports` lint into a hard error](rust-lang/rust#149195) - [Where-bounds of the form `Type = Type` and `Type == Type` are no longer syntactically allowed](rust-lang/rust#153513) - [Ensure Send/Sync is not implemented for std::env::Vars{,Os}](rust-lang/rust#155153) - [Fix that in some attributes, arguments were not properly rejected](rust-lang/rust#155193) - [`repr(transparent)` is now more strict about which fields have "trivial" layout and hence can be ignored: `repr(C)` types, types with private fields, and `#[non_exhaustive]` types are no longer considered "trivial"](rust-lang/rust#155299) - [Correctly check whether types have equal size in `transmute()` when some `repr` attributes are involved.](rust-lang/rust#155418) - [More characters are escaped when printing strings and chars](rust-lang/rust#155527) - [Implement fast path for `derive(PartialOrd)` when deriving `Ord`](rust-lang/rust#155598) This can break crates in practice where a type's PartialOrd and Ord impls were inconsistent with each other. - [Add temporary scope to `assert_eq` and `assert_ne`](rust-lang/rust#155739) - Closed a hole in the pattern matching [structural equality](https://doc.rust-lang.org/reference/patterns.html#constant-patterns) check, preventing cases where a match of a constant would be allowed, despite disagreeing with a manually written `PartialEq` implementation, when a `derive(PartialEq)` implementation for that type also exists. - [On Emscripten the WASM exception handling ABI is now unconditionally used](rust-lang/rust#156928) The `-Zemscripten-wasm-eh=false` flag to switch back to JS exceptions has been removed. - [The UNSAFE\_CODE lint is now consistently emitted for all unsafe attributes](rust-lang/rust#157201) - [Solaris: remove `File::lock` implementation, it has the wrong semantics (return "unsupported" instead)](rust-lang/rust#157509) - [Windows-gnu targets now specify baseline tools versions](rust-lang/rust#158020) - [rustfmt now discovers module files that are defined in `cfg_select!`](rust-lang/rust#158372) This may cause more code to be formatted which was previously ignored. </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODguMCIsInVwZGF0ZWRJblZlciI6IjQzLjI4OC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
Audit RUST_GUIDELINES.md against Rust/Cargo/Clippy 1.98.0 (2026-08-20). Three 1.98 changes conflicted with rules the document already gave: - derive(PartialOrd) fast path (rust-lang/rust#155598) plus the closed pattern-matching structural-equality hole make a manual PartialEq alongside a derived Ord observably inconsistent. The existing "destructure structs in trait impls" example did exactly this (ignoring `timestamp`), so it shipped the trap with the technique. New rule: comparison traits are all-manual or all-derived, `a == b` iff `cmp == Equal`, and manual-PartialEq types must not appear in constant patterns. - UNSAFE_CODE now fires on unsafe attributes (rust-lang/rust#157201). `unsafe_code = "forbid"` cannot be locally overridden, so crates using #[unsafe(no_mangle)] / #[unsafe(link_section)] / #[unsafe(export_name)] / #[unsafe(naked)] break on upgrade. Firmware targets must use "deny" plus a justified per-item #[allow]. - repr(transparent) is stricter (rust-lang/rust#155299): repr(C), private-field, and #[non_exhaustive] types are no longer "trivial" layout, which collides with the newtype and exhaustive_structs guidance. Also added: algebraic float non-determinism rule; Clippy 1.98 lint tables (5 lints auto-covered by all = "deny"; with_capacity_zero and unused_async_trait_impl are pedantic and need a decision -- documented as a per-impl #[expect(reason)] matching 80c3586, not a crate-wide allow); rustc runtime-symbol lint table with warnings-group membership; rustfmt now discovering cfg_select! modules and the fmt-gate churn that implies; Debug-is-not-a-wire-format and assert_eq! temporary-scope test rules; and 1.98 API idioms (bool::ok_or, NonZero::from_str_radix, format_into + NumBuffer, substr_range/subslice_range, Atomic::from_mut, strip_circumfix, String::from_utf16{le,be}). Corrected drift against the enforced config: expect_used and clone_on_ref_ptr to "deny", removed the deprecated string_to_string lint, added the mandatory priority = -1 on [lints.clippy] group entries, documented the clippy.toml thresholds, marked the pointer lints as moot under unsafe_code = "forbid", and made the Miri requirement conditional on the crate actually containing unsafe. Fixed a GOOD example that violated the document's own unwrap_used and indexing_slicing rules. Cargo 1.98 needs no changes: its stable Added and Changed sections are empty. ESP32/embassy guidance is retained and refreshed in place rather than removed, including NumBuffer as the core-not-alloc integer formatting path and algebraic_* fenced off from values reported over MQTT or persisted to NVS.
View all comments
This lint is about which fields we consider "trivial" for
repr(transparent). Forrepr(transparent)to be valid, there can be at most one non-trivial field. In other words, trivial fields are those that we promise do not affect the layout or ABI of therepr(transparent)type. Historically we considered all types with size 0 and alignment 1 (i.e., all 1-ZST) to be trivial. However we'd like to take some of that back:#[non_exhaustive](except when we are in the same crate as that type).repr(C)and who knows what the C ABI does. In particular on MSVC a struct whose only field is a 0-length array has size 1. Rust incorrectly gives it size 0. With repr(ordered_fields) rfcs#3845 we can hopefully fix the layout, which would make "is that type a 1-ZST" target-dependent, and we ideally should reject such code on all targets.This was a deny-by-default FCW since #147185, which landed almost 6 months ago and shipped with Rust 1.93. (If this PR lands now it will ship with 1.97.) Already back then we found hardly any crater impact. The tracking issue has had no new relevant backrefs since that PR landed.
So, I think it is time to make this a hard error.
Fixes #78586 (tracking issue)
Fixes rust-lang/unsafe-code-guidelines#552 because this means the
repr(transparent)ABI compatibility rule no longer ever "ignores"repr(C)fields.@rust-lang/lang What do you think? See here for the crater analysis; the summary is that there's no relevant regressions found in the wild. But some points have been raised by people:
ghostcrate offers a macro to definePhantomData-like types, and those types involve arepr(C). If someone uses such a type as a marker inside arepr(transparent), that will no longer work. Apparently nobody does that in the code checked by crater. A new version of the crate has been released that fixes this.#[rustc_pub_transparent]). That means it is not possible for a crate to expose a semantically relevant maker type (likeGhostToken) that is "trivial" forrepr(transparent)purposes. Apparently currently nobody does this in the ecosystem (the parts crater can see, anyway), but it seems like a sensible thing to do. If we are concerned about this, we could limit this PR to only make therepr(C)and#[non_exhaustive]part of the check a hard error, and leave the "has private fields" part as a warning.Also note that the private field check is technically a bit odd: we literally check "is the type defined in this crate or are all fields public". We do not check if the current module can access those fields. So if a type has private fields then one can rely on it being "trivial" everywhere in the current crate, even outside the module that defined the type. This is not how field privacy usually works. If we want to restrict this to "only modules that can 'see' the fields are allowed to rely on the type being a 1-ZST", that's technically a breaking change. I don't know if this was a deliberate choice or just the easiest thing to implement. @scottmcm do you remember?