repr(transparent) check: do not compute check_unsuited more than once#148281
repr(transparent) check: do not compute check_unsuited more than once#148281bors merged 1 commit intorust-lang:mainfrom
Conversation
|
r? @spastorino rustbot has assigned @spastorino. Use |
| @@ -1532,60 +1520,7 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, adt: ty::AdtDef<'tcx>) | |||
| // We are currently checking the type this field came from, so it must be local | |||
| let span = tcx.hir_span_if_local(field.did).unwrap(); | |||
| let trivial = layout.is_ok_and(|layout| layout.is_1zst()); | |||
There was a problem hiding this comment.
There is still some code here we are running twice, in particular the layout_of query. The alternative would be to simply collect the results into a Vec, which will cost us an allocation but then avoids running the iterator twice.
|
@rustbot reroll |
| } | ||
|
|
||
| FieldInfo { span, trivial, unsuited: check_unsuited(tcx, typing_env, ty).break_value() } | ||
| return FieldInfo { span, trivial, ty }; |
There was a problem hiding this comment.
| return FieldInfo { span, trivial, ty }; | |
| FieldInfo { span, trivial, ty } |
|
r=me with the nit fixed. |
928cde5 to
c23182e
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r=nnethercote rollup |
Rollup of 5 pull requests Successful merges: - #147887 (Improve the documentation of atomic::fence) - #148281 (repr(transparent) check: do not compute check_unsuited more than once) - #148484 (Fix suggestion for the `cfg!` macro) - #149057 (`rust-analyzer` subtree update) - #149061 (debug-assert FixedSizeEncoding invariant) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 5 pull requests Successful merges: - #147887 (Improve the documentation of atomic::fence) - #148281 (repr(transparent) check: do not compute check_unsuited more than once) - #148484 (Fix suggestion for the `cfg!` macro) - #149057 (`rust-analyzer` subtree update) - #149061 (debug-assert FixedSizeEncoding invariant) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #148281 - RalfJung:repr-transparent-check, r=nnethercote repr(transparent) check: do not compute check_unsuited more than once `field_infos` is an iterator that we execute multiple times. However, we usually ignore the `unsuited` field -- we only need it in the last iteration. So move the computation of that field to that iteration to avoid computing it multiple times. Computing `unsuited` involves a recursive traversal over the types of all non-trivial fields, so there can be non-trivial amounts of work here. (I benchmarked this in #148243 and saw no changes, probably because we don't have a benchmark with many repr(transparent) types. But still, computing this each time just seemed silly.)
Rollup of 5 pull requests Successful merges: - rust-lang/rust#147887 (Improve the documentation of atomic::fence) - rust-lang/rust#148281 (repr(transparent) check: do not compute check_unsuited more than once) - rust-lang/rust#148484 (Fix suggestion for the `cfg!` macro) - rust-lang/rust#149057 (`rust-analyzer` subtree update) - rust-lang/rust#149061 (debug-assert FixedSizeEncoding invariant) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#147887 (Improve the documentation of atomic::fence) - rust-lang#148281 (repr(transparent) check: do not compute check_unsuited more than once) - rust-lang#148484 (Fix suggestion for the `cfg!` macro) - rust-lang#149057 (`rust-analyzer` subtree update) - rust-lang#149061 (debug-assert FixedSizeEncoding invariant) r? `@ghost` `@rustbot` modify labels: rollup
field_infosis an iterator that we execute multiple times. However, we usually ignore theunsuitedfield -- we only need it in the last iteration. So move the computation of that field to that iteration to avoid computing it multiple times. Computingunsuitedinvolves a recursive traversal over the types of all non-trivial fields, so there can be non-trivial amounts of work here.(I benchmarked this in #148243 and saw no changes, probably because we don't have a benchmark with many repr(transparent) types. But still, computing this each time just seemed silly.)