-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(mut_mutex_lock): FP in projection chains #16261
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
base: master
Are you sure you want to change the base?
Conversation
|
rustbot has assigned @samueltardieu. Use |
138c735 to
e95dbbf
Compare
|
No changes for 37d8ef4 |
|
So this is still wrong. The thing this needs to check for is that each adjustment to the place could have been a mutable adjustment rather than an immutable one. Would be something like (not quite real code and please write it better): let mut e = recv;
'outer: loop {
if !expr_adjustments(e)
.take_while(|a| match a.kind {
Adjust::Deref(x) => Some((a.target, x)),
_ => None,
}).try_fold(expr_ty(e), |ty, (target, deref)| match deref {
None => (!ty.is_imm_ref()).then(target),
Some(_) => impls_deref_mut(ty).then(target),
}).is_some()
{
return;
}
loop {
match e.kind {
ExprKind::Field(base, _) => e = base,
ExprKind::Index(base, idx) if impls_idx_mut(expr_ty_adjusted(base), expr_ty_adjusted(idx)) => e = base,
// `base` here can't actually have adjustments
ExprKind::UnOp(UnOp::Deref, base) if impls_deref_mut(expr_ty_adjusted(base)) => {
e = base;
continue;
},
_ => break 'outer,
}
continue 'outer;
}
} |
|
Okay so I took your code, fixed some small bugs/typos, but importantly also removed the inner loop, because, as you implied yourself, it made the code a lot more confusing for imo (?) little benefit. I did keep each step as a separate commit though, so I can easily remove some of the changes if you prefer. Looking at the first couple of dogfood results, they do seem to be legit -- apart from the one in |
since there won't be any adjustments after an explicit deref, the adjustment checking step should finish pretty fast
48ef46e to
767e668
Compare
|
I implemented a pretty basic check for statics, but I think there ought to be a more robust approach -- especially given that this statics thing didn't happen before |
767e668 to
37d8ef4
Compare
fixes #16253
changelog: [
mut_mutex_lock]: fix FP in projection chains