repr(transparent) check: do not compute check_unsuited more than once #148281
Conversation
rustbot
added
S-waiting-on-review
|
r? @spastorino rustbot has assigned @spastorino. Use |
| let layout = tcx.layout_of(typing_env.as_query_input(ty)); | ||
| // 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.
| if let Some(unsuited) = field.unsuited { | ||
| assert!(field.trivial); | ||
| if field.trivial | ||
| && let Some(unsuited) = check_unsuited(tcx, typing_env, field.ty).break_value() |
There was a problem hiding this comment.
Pre-existing, but why does the function return a ControlFlow if its only call immediately calls break_value, and answering my own question: because it is used internally in recursive calls.
Perhaps this implementation detail should still be pushed inside with check_unsuited returning Option, while calling an internal function that returns ControlFlow?
There was a problem hiding this comment.
This is already a purely internal helper. I don't see the point of having even more nested functions here.
4 participants
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.)