Skip to content

Commit 07f0e2c

Browse files
committed
expr_use_ctxt changes:
* Delay the parsing of the use node * Mark when the `SyntaxContext` changes rather than return `None` * Return a default value if the HIR tree is broken rather than `None`
1 parent 547ad39 commit 07f0e2c

File tree

3 files changed

+119
-99
lines changed

3 files changed

+119
-99
lines changed

clippy_lints/src/dereference.rs

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -260,18 +260,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
260260
(None, kind) => {
261261
let expr_ty = typeck.expr_ty(expr);
262262
let use_cx = expr_use_ctxt(cx, expr);
263-
let adjusted_ty = match &use_cx {
264-
Some(use_cx) => match use_cx.adjustments {
265-
[.., a] => a.target,
266-
_ => expr_ty,
267-
},
268-
_ => typeck.expr_ty_adjusted(expr),
269-
};
263+
let adjusted_ty = use_cx.adjustments.last().map_or(expr_ty, |a| a.target);
270264

271-
match (use_cx, kind) {
272-
(Some(use_cx), RefOp::Deref) => {
265+
match kind {
266+
RefOp::Deref if use_cx.same_ctxt => {
267+
let use_node = use_cx.use_node(cx);
273268
let sub_ty = typeck.expr_ty(sub_expr);
274-
if let ExprUseNode::FieldAccess(name) = use_cx.node
269+
if let ExprUseNode::FieldAccess(name) = use_node
275270
&& !use_cx.moved_before_use
276271
&& !ty_contains_field(sub_ty, name.name)
277272
{
@@ -288,9 +283,9 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
288283
} else if sub_ty.is_ref()
289284
// Linting method receivers would require verifying that name lookup
290285
// would resolve the same way. This is complicated by trait methods.
291-
&& !use_cx.node.is_recv()
292-
&& let Some(ty) = use_cx.node.defined_ty(cx)
293-
&& TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()).is_deref_stable()
286+
&& !use_node.is_recv()
287+
&& let Some(ty) = use_node.defined_ty(cx)
288+
&& TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()).is_deref_stable()
294289
{
295290
self.state = Some((
296291
State::ExplicitDeref { mutability: None },
@@ -301,7 +296,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
301296
));
302297
}
303298
},
304-
(_, RefOp::Method { mutbl, is_ufcs })
299+
RefOp::Method { mutbl, is_ufcs }
305300
if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
306301
// Allow explicit deref in method chains. e.g. `foo.deref().bar()`
307302
&& (is_ufcs || !in_postfix_position(cx, expr)) =>
@@ -319,7 +314,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
319314
},
320315
));
321316
},
322-
(Some(use_cx), RefOp::AddrOf(mutability)) => {
317+
RefOp::AddrOf(mutability) if use_cx.same_ctxt => {
323318
// Find the number of times the borrow is auto-derefed.
324319
let mut iter = use_cx.adjustments.iter();
325320
let mut deref_count = 0usize;
@@ -338,10 +333,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
338333
};
339334
};
340335

341-
let stability = use_cx.node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
342-
TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return())
336+
let use_node = use_cx.use_node(cx);
337+
let stability = use_node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
338+
TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return())
343339
});
344-
let can_auto_borrow = match use_cx.node {
340+
let can_auto_borrow = match use_node {
345341
ExprUseNode::FieldAccess(_)
346342
if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) =>
347343
{
@@ -353,7 +349,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
353349
// deref through `ManuallyDrop<_>` will not compile.
354350
!adjust_derefs_manually_drop(use_cx.adjustments, expr_ty)
355351
},
356-
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) => true,
352+
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) if !use_cx.moved_before_use => true,
357353
ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => {
358354
// Check for calls to trait methods where the trait is implemented
359355
// on a reference.
@@ -363,9 +359,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
363359
// priority.
364360
if let Some(fn_id) = typeck.type_dependent_def_id(hir_id)
365361
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
366-
&& let arg_ty = cx
367-
.tcx
368-
.erase_regions(use_cx.adjustments.last().map_or(expr_ty, |a| a.target))
362+
&& let arg_ty = cx.tcx.erase_regions(adjusted_ty)
369363
&& let ty::Ref(_, sub_ty, _) = *arg_ty.kind()
370364
&& let args =
371365
typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default()
@@ -443,7 +437,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
443437
count: deref_count - required_refs,
444438
msg,
445439
stability,
446-
for_field_access: if let ExprUseNode::FieldAccess(name) = use_cx.node
440+
for_field_access: if let ExprUseNode::FieldAccess(name) = use_node
447441
&& !use_cx.moved_before_use
448442
{
449443
Some(name.name)
@@ -453,7 +447,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
453447
}),
454448
StateData {
455449
first_expr: expr,
456-
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
450+
adjusted_ty,
457451
},
458452
));
459453
} else if stability.is_deref_stable()
@@ -465,12 +459,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
465459
State::Borrow { mutability },
466460
StateData {
467461
first_expr: expr,
468-
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
462+
adjusted_ty,
469463
},
470464
));
471465
}
472466
},
473-
(None, _) | (_, RefOp::Method { .. }) => (),
467+
_ => {},
474468
}
475469
},
476470
(

clippy_lints/src/needless_borrows_for_generic_args.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> {
8181
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
8282
if matches!(expr.kind, ExprKind::AddrOf(..))
8383
&& !expr.span.from_expansion()
84-
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
84+
&& let use_cx = expr_use_ctxt(cx, expr)
85+
&& use_cx.same_ctxt
8586
&& !use_cx.is_ty_unified
86-
&& let Some(DefinedTy::Mir(ty)) = use_cx.node.defined_ty(cx)
87+
&& let use_node = use_cx.use_node(cx)
88+
&& let Some(DefinedTy::Mir(ty)) = use_node.defined_ty(cx)
8789
&& let ty::Param(ty) = *ty.value.skip_binder().kind()
88-
&& let Some((hir_id, fn_id, i)) = match use_cx.node {
90+
&& let Some((hir_id, fn_id, i)) = match use_node {
8991
ExprUseNode::MethodArg(_, _, 0) => None,
9092
ExprUseNode::MethodArg(hir_id, None, i) => cx
9193
.typeck_results()

clippy_utils/src/lib.rs

Lines changed: 94 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#![feature(array_chunks)]
22
#![feature(box_patterns)]
33
#![feature(control_flow_enum)]
4+
#![feature(exhaustive_patterns)]
45
#![feature(if_let_guard)]
56
#![feature(let_chains)]
67
#![feature(lint_reasons)]
@@ -2618,13 +2619,80 @@ pub enum DefinedTy<'tcx> {
26182619
/// The context an expressions value is used in.
26192620
pub struct ExprUseCtxt<'tcx> {
26202621
/// The parent node which consumes the value.
2621-
pub node: ExprUseNode<'tcx>,
2622+
pub node: Node<'tcx>,
2623+
/// The child id of the node the value came from.
2624+
pub child_id: HirId,
26222625
/// Any adjustments applied to the type.
26232626
pub adjustments: &'tcx [Adjustment<'tcx>],
2624-
/// Whether or not the type must unify with another code path.
2627+
/// Whether the type must unify with another code path.
26252628
pub is_ty_unified: bool,
2626-
/// Whether or not the value will be moved before it's used.
2629+
/// Whether the value will be moved before it's used.
26272630
pub moved_before_use: bool,
2631+
/// Whether the use site has the same `SyntaxContext` as the value.
2632+
pub same_ctxt: bool,
2633+
}
2634+
impl<'tcx> ExprUseCtxt<'tcx> {
2635+
pub fn use_node(&self, cx: &LateContext<'tcx>) -> ExprUseNode<'tcx> {
2636+
match self.node {
2637+
Node::Local(l) => ExprUseNode::Local(l),
2638+
Node::ExprField(field) => ExprUseNode::Field(field),
2639+
2640+
Node::Item(&Item {
2641+
kind: ItemKind::Static(..) | ItemKind::Const(..),
2642+
owner_id,
2643+
..
2644+
})
2645+
| Node::TraitItem(&TraitItem {
2646+
kind: TraitItemKind::Const(..),
2647+
owner_id,
2648+
..
2649+
})
2650+
| Node::ImplItem(&ImplItem {
2651+
kind: ImplItemKind::Const(..),
2652+
owner_id,
2653+
..
2654+
}) => ExprUseNode::ConstStatic(owner_id),
2655+
2656+
Node::Item(&Item {
2657+
kind: ItemKind::Fn(..),
2658+
owner_id,
2659+
..
2660+
})
2661+
| Node::TraitItem(&TraitItem {
2662+
kind: TraitItemKind::Fn(..),
2663+
owner_id,
2664+
..
2665+
})
2666+
| Node::ImplItem(&ImplItem {
2667+
kind: ImplItemKind::Fn(..),
2668+
owner_id,
2669+
..
2670+
}) => ExprUseNode::Return(owner_id),
2671+
2672+
Node::Expr(use_expr) => match use_expr.kind {
2673+
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
2674+
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
2675+
}),
2676+
2677+
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
2678+
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == self.child_id) {
2679+
Some(i) => ExprUseNode::FnArg(func, i),
2680+
None => ExprUseNode::Callee,
2681+
},
2682+
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
2683+
use_expr.hir_id,
2684+
name.args,
2685+
args.iter()
2686+
.position(|arg| arg.hir_id == self.child_id)
2687+
.map_or(0, |i| i + 1),
2688+
),
2689+
ExprKind::Field(_, name) => ExprUseNode::FieldAccess(name),
2690+
ExprKind::AddrOf(kind, mutbl, _) => ExprUseNode::AddrOf(kind, mutbl),
2691+
_ => ExprUseNode::Other,
2692+
},
2693+
_ => ExprUseNode::Other,
2694+
}
2695+
}
26282696
}
26292697

26302698
/// The node which consumes a value.
@@ -2645,7 +2713,8 @@ pub enum ExprUseNode<'tcx> {
26452713
Callee,
26462714
/// Access of a field.
26472715
FieldAccess(Ident),
2648-
Expr,
2716+
/// Borrow expression.
2717+
AddrOf(ast::BorrowKind, Mutability),
26492718
Other,
26502719
}
26512720
impl<'tcx> ExprUseNode<'tcx> {
@@ -2723,25 +2792,26 @@ impl<'tcx> ExprUseNode<'tcx> {
27232792
let sig = cx.tcx.fn_sig(id).skip_binder();
27242793
Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i))))
27252794
},
2726-
Self::Local(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None,
2795+
Self::Local(_) | Self::FieldAccess(..) | Self::Callee | Self::Other | Self::AddrOf(..) => None,
27272796
}
27282797
}
27292798
}
27302799

27312800
/// Gets the context an expression's value is used in.
2732-
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> {
2801+
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> ExprUseCtxt<'tcx> {
27332802
let mut adjustments = [].as_slice();
27342803
let mut is_ty_unified = false;
27352804
let mut moved_before_use = false;
2805+
let mut same_ctxt = true;
27362806
let ctxt = e.span.ctxt();
2737-
walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| {
2807+
let node = walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| -> ControlFlow<!> {
27382808
if adjustments.is_empty()
27392809
&& let Node::Expr(e) = cx.tcx.hir_node(child_id)
27402810
{
27412811
adjustments = cx.typeck_results().expr_adjustments(e);
27422812
}
2743-
if !cx.tcx.hir().opt_span(parent_id).is_some_and(|x| x.ctxt() == ctxt) {
2744-
return ControlFlow::Break(());
2813+
if let Some(s) = cx.tcx.hir().opt_span(parent_id) {
2814+
same_ctxt &= s.ctxt() == ctxt;
27452815
}
27462816
if let Node::Expr(e) = parent {
27472817
match e.kind {
@@ -2758,71 +2828,25 @@ pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Optio
27582828
}
27592829
}
27602830
ControlFlow::Continue(())
2761-
})?
2762-
.continue_value()
2763-
.map(|(use_node, child_id)| {
2764-
let node = match use_node {
2765-
Node::Local(l) => ExprUseNode::Local(l),
2766-
Node::ExprField(field) => ExprUseNode::Field(field),
2767-
2768-
Node::Item(&Item {
2769-
kind: ItemKind::Static(..) | ItemKind::Const(..),
2770-
owner_id,
2771-
..
2772-
})
2773-
| Node::TraitItem(&TraitItem {
2774-
kind: TraitItemKind::Const(..),
2775-
owner_id,
2776-
..
2777-
})
2778-
| Node::ImplItem(&ImplItem {
2779-
kind: ImplItemKind::Const(..),
2780-
owner_id,
2781-
..
2782-
}) => ExprUseNode::ConstStatic(owner_id),
2783-
2784-
Node::Item(&Item {
2785-
kind: ItemKind::Fn(..),
2786-
owner_id,
2787-
..
2788-
})
2789-
| Node::TraitItem(&TraitItem {
2790-
kind: TraitItemKind::Fn(..),
2791-
owner_id,
2792-
..
2793-
})
2794-
| Node::ImplItem(&ImplItem {
2795-
kind: ImplItemKind::Fn(..),
2796-
owner_id,
2797-
..
2798-
}) => ExprUseNode::Return(owner_id),
2799-
2800-
Node::Expr(use_expr) => match use_expr.kind {
2801-
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
2802-
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
2803-
}),
2804-
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
2805-
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == child_id) {
2806-
Some(i) => ExprUseNode::FnArg(func, i),
2807-
None => ExprUseNode::Callee,
2808-
},
2809-
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
2810-
use_expr.hir_id,
2811-
name.args,
2812-
args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
2813-
),
2814-
ExprKind::Field(child, name) if child.hir_id == e.hir_id => ExprUseNode::FieldAccess(name),
2815-
_ => ExprUseNode::Expr,
2816-
},
2817-
_ => ExprUseNode::Other,
2818-
};
2819-
ExprUseCtxt {
2831+
});
2832+
match node {
2833+
Some(ControlFlow::Continue((node, child_id))) => ExprUseCtxt {
28202834
node,
2835+
child_id,
28212836
adjustments,
28222837
is_ty_unified,
28232838
moved_before_use,
2824-
}
2825-
})
2839+
same_ctxt,
2840+
},
2841+
None => ExprUseCtxt {
2842+
node: Node::Crate(cx.tcx.hir().root_module()),
2843+
child_id: HirId::INVALID,
2844+
adjustments: &[],
2845+
is_ty_unified: true,
2846+
moved_before_use: true,
2847+
same_ctxt: false,
2848+
},
2849+
}
28262850
}
28272851

28282852
/// Tokenizes the input while keeping the text associated with each token.

0 commit comments

Comments
 (0)