Fix false positive of unused_braces with Mutex lock().unwrap() chains
#143574
Conversation
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
…ains Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
rustbot
added
S-waiting-on-review
| match &expr.kind { | ||
| // Check for patterns like `test.lock().unwrap().field` that might cause deadlocks | ||
| ast::ExprKind::Field(base, _) => { | ||
| Self::matches_method_chain(base, vec!["lock", "unwrap"]) |
There was a problem hiding this comment.
This misses every other function that could possibly have this problem and theoretically stops linting on functions that don't have this problem.
the root problem is lifetime extension, not locks. Any kind of guard that gets dropped at the end of the main expression unless wrapped in a block expression will want this. I'm not sure how to check this on the HIR, but I expect this has been a problem elsewhere before, likely in match scrutinees, so maybe you can find some logic there that handles this generally.
There was a problem hiding this comment.
Yeah, this is a hack method, I will investigate it later.
I forgot to link the #143562 to this PR.
There was a problem hiding this comment.
Maybe we could reuse the same logic about significant drops as the tail_expr_drop_order lint: extract_component_with_significant_dtor
There was a problem hiding this comment.
In any case the .unwrap() shouldn't have any effect on if this lint triggers or not.
There was a problem hiding this comment.
This Lint appears to require total refactoring. To obtain this information, we need tcx in LateContext, but the current UnusedBraces implementation is EarlyLintPass.
Fixes #143562
Add side effect detection to prevent linting braces around expressions that contain method call chains like
test.lock().unwrap().field, which require braces for proper scoping to avoid deadlocks. The current method is to match the call chain, and I am not sure if a more precise match is needed.r? compiler