-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Don't capture &[T; N]
when contents isn't read
#112636
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
@@ -443,7 +443,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { | |||
if matches!((lhs, wild, rhs), (&[], Some(_), &[])) | |||
// Arrays have a statically known size, so | |||
// there is no need to read their length | |||
|| discr_place.place.base_ty.is_array() | |||
|| discr_place.place.base_ty.peel_refs().is_array() |
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.
should we be doing discr_place.place.ty()
insead of checking base_ty
? Idk if that's right, will need checking...
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.
I wasn't able to produce any incorrect behaviour with this but ty()
sounds like the better choice so I'll switch to that
Does this make matching like: |
The closure capture analysis of this struct Foo<T>(T);
fn main() {
let f = &Foo(&[10; 3]);
let c = #[rustc_capture_analysis]
|| {
match f {
Foo([_, _, _]) => ()
}
};
c();
} is the same between current nightly and this PR ( EDIT: Actually, this check is run specifically for |
119c2a2
to
446a89a
Compare
Actually, I think this should be using |
My mistake 😓 This example does not capture |
446a89a
to
25d1ee0
Compare
25d1ee0
to
e72618a
Compare
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (9397862): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 656.279s -> 656.699s (0.06%) |
Fixes the check in #111831
Fixes #112607, although I decided to test the root cause rather than including the example in the issue as a test.
cc @BoxyUwU