repr(transparent): do not consider repr(C) types to be 1-ZST#147185
repr(transparent): do not consider repr(C) types to be 1-ZST#147185bors merged 1 commit intorust-lang:masterfrom
Conversation
|
rustbot has assigned @petrochenkov. Use |
|
@bors try |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
|
This found
EDIT: It later got confirmed that these crates indeed already trigger the lint before this PR. |
|
@bors try |
repr(transparent): do not consider repr(C) types to be 1-ZST
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
|
@petrochenkov @rustbot ready |
|
r=me with the outdated comment updated. |
091a9d5 to
b9b29c4
Compare
|
Thanks! @bors r=petrochenkov |
…enkov repr(transparent): do not consider repr(C) types to be 1-ZST Context: rust-lang/unsafe-code-guidelines#552 This experiments with a [suggestion](rust-lang/rfcs#3845 (comment)) by `@RustyYato` to stop considering repr(C) types as 1-ZST for the purpose of repr(transparent). If we go with rust-lang/rfcs#3845 (or another approach for fixing repr(C)), they will anyway not be ZST on all targets any more, so this removes a portability hazard. Furthermore, zero-sized repr(C) structs [may have to be treated](rust-lang/unsafe-code-guidelines#552 (comment)) as non-ZST for the win64 ABI (at least that's what gcc/clang do), so allowing them to be ignored in repr(transparent) types is not entirely coherent. Turns out we already have an FCW for repr(transparent), namely rust-lang#78586. This extends that lint to also check for repr(C).
…enkov repr(transparent): do not consider repr(C) types to be 1-ZST Context: rust-lang/unsafe-code-guidelines#552 This experiments with a [suggestion](rust-lang/rfcs#3845 (comment)) by ``@RustyYato`` to stop considering repr(C) types as 1-ZST for the purpose of repr(transparent). If we go with rust-lang/rfcs#3845 (or another approach for fixing repr(C)), they will anyway not be ZST on all targets any more, so this removes a portability hazard. Furthermore, zero-sized repr(C) structs [may have to be treated](rust-lang/unsafe-code-guidelines#552 (comment)) as non-ZST for the win64 ABI (at least that's what gcc/clang do), so allowing them to be ignored in repr(transparent) types is not entirely coherent. Turns out we already have an FCW for repr(transparent), namely rust-lang#78586. This extends that lint to also check for repr(C).
Rollup of 14 pull requests Successful merges: - #144936 (CFI: Fix types that implement Fn, FnMut, or FnOnce) - #147185 (repr(transparent): do not consider repr(C) types to be 1-ZST) - #147840 (Rework unsizing coercions in the new solver) - #147915 (Update target maintainers android.md) - #148013 (1.91.0 release notes) - #148044 (compiletest: show output in debug logging) - #148057 (tests/ui/sanitizer/hwaddress.rs: Run on aarch64 and remove cgu hack) - #148139 (Add `coverage` scope for controlling paths in code coverage) - #148154 (Add a mailmap entry) - #148158 (ci: loongarch64: use medium code model to avoid relocation overflows) - #148166 (Re-enable macro-stepping test for AArch64) - #148172 (rustc-dev-guide subtree update) - #148175 (Fix typos: duplicate words in comments) - #148186 (rustdoc-search: add an integration test for CCI) Failed merges: - #147935 (Add LLVM realtime sanitizer) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #147185 - RalfJung:repr-c-not-zst, r=petrochenkov repr(transparent): do not consider repr(C) types to be 1-ZST Context: rust-lang/unsafe-code-guidelines#552 This experiments with a [suggestion](rust-lang/rfcs#3845 (comment)) by ```@RustyYato``` to stop considering repr(C) types as 1-ZST for the purpose of repr(transparent). If we go with rust-lang/rfcs#3845 (or another approach for fixing repr(C)), they will anyway not be ZST on all targets any more, so this removes a portability hazard. Furthermore, zero-sized repr(C) structs [may have to be treated](rust-lang/unsafe-code-guidelines#552 (comment)) as non-ZST for the win64 ABI (at least that's what gcc/clang do), so allowing them to be ignored in repr(transparent) types is not entirely coherent. Turns out we already have an FCW for repr(transparent), namely #78586. This extends that lint to also check for repr(C).
|
This appears to be the cause of a few perf regressions: #148202 (comment) |
|
Are you sure? This code only affects the repr(transparent) check, which shouldn't run that often -- and it only affects it in fairly trivial ways, too; We do run that iterator multiple times, but that was already the case before this PR. |
perf experiment for #147185
|
That regression is caused by the lint rename, which triggers the "lint has been renamed" lint in syn, and it takes some work to render those nice diagnostics we have. |
Context: rust-lang/unsafe-code-guidelines#552
This experiments with a suggestion by @RustyYato to stop considering repr(C) types as 1-ZST for the purpose of repr(transparent). If we go with rust-lang/rfcs#3845 (or another approach for fixing repr(C)), they will anyway not be ZST on all targets any more, so this removes a portability hazard. Furthermore, zero-sized repr(C) structs may have to be treated as non-ZST for the win64 ABI (at least that's what gcc/clang do), so allowing them to be ignored in repr(transparent) types is not entirely coherent.
Turns out we already have an FCW for repr(transparent), namely #78586. This extends that lint to also check for repr(C).