Skip to content

Commit a002f93

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 9f5d60f commit a002f93

File tree

4 files changed

+120
-102
lines changed

4 files changed

+120
-102
lines changed

clippy_lints/src/casts/ref_as_ptr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ pub(super) fn check<'tcx>(
2222

2323
if matches!(cast_from.kind(), ty::Ref(..))
2424
&& let ty::RawPtr(_, to_mutbl) = cast_to.kind()
25-
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
25+
&& let use_cx = expr_use_ctxt(cx, expr)
2626
// TODO: only block the lint if `cast_expr` is a temporary
27-
&& !matches!(use_cx.node, ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
27+
&& !matches!(use_cx.use_node(cx), ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
2828
{
2929
let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" };
3030
let fn_name = match to_mutbl {

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
@@ -80,11 +80,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> {
8080
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
8181
if matches!(expr.kind, ExprKind::AddrOf(..))
8282
&& !expr.span.from_expansion()
83-
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
83+
&& let use_cx = expr_use_ctxt(cx, expr)
84+
&& use_cx.same_ctxt
8485
&& !use_cx.is_ty_unified
85-
&& let Some(DefinedTy::Mir(ty)) = use_cx.node.defined_ty(cx)
86+
&& let use_node = use_cx.use_node(cx)
87+
&& let Some(DefinedTy::Mir(ty)) = use_node.defined_ty(cx)
8688
&& let ty::Param(ty) = *ty.value.skip_binder().kind()
87-
&& let Some((hir_id, fn_id, i)) = match use_cx.node {
89+
&& let Some((hir_id, fn_id, i)) = match use_node {
8890
ExprUseNode::MethodArg(_, _, 0) => None,
8991
ExprUseNode::MethodArg(hir_id, None, i) => cx
9092
.typeck_results()

clippy_utils/src/lib.rs

Lines changed: 93 additions & 71 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)]
@@ -2664,13 +2665,80 @@ pub enum DefinedTy<'tcx> {
26642665
/// The context an expressions value is used in.
26652666
pub struct ExprUseCtxt<'tcx> {
26662667
/// The parent node which consumes the value.
2667-
pub node: ExprUseNode<'tcx>,
2668+
pub node: Node<'tcx>,
2669+
/// The child id of the node the value came from.
2670+
pub child_id: HirId,
26682671
/// Any adjustments applied to the type.
26692672
pub adjustments: &'tcx [Adjustment<'tcx>],
2670-
/// Whether or not the type must unify with another code path.
2673+
/// Whether the type must unify with another code path.
26712674
pub is_ty_unified: bool,
2672-
/// Whether or not the value will be moved before it's used.
2675+
/// Whether the value will be moved before it's used.
26732676
pub moved_before_use: bool,
2677+
/// Whether the use site has the same `SyntaxContext` as the value.
2678+
pub same_ctxt: bool,
2679+
}
2680+
impl<'tcx> ExprUseCtxt<'tcx> {
2681+
pub fn use_node(&self, cx: &LateContext<'tcx>) -> ExprUseNode<'tcx> {
2682+
match self.node {
2683+
Node::LetStmt(l) => ExprUseNode::LetStmt(l),
2684+
Node::ExprField(field) => ExprUseNode::Field(field),
2685+
2686+
Node::Item(&Item {
2687+
kind: ItemKind::Static(..) | ItemKind::Const(..),
2688+
owner_id,
2689+
..
2690+
})
2691+
| Node::TraitItem(&TraitItem {
2692+
kind: TraitItemKind::Const(..),
2693+
owner_id,
2694+
..
2695+
})
2696+
| Node::ImplItem(&ImplItem {
2697+
kind: ImplItemKind::Const(..),
2698+
owner_id,
2699+
..
2700+
}) => ExprUseNode::ConstStatic(owner_id),
2701+
2702+
Node::Item(&Item {
2703+
kind: ItemKind::Fn(..),
2704+
owner_id,
2705+
..
2706+
})
2707+
| Node::TraitItem(&TraitItem {
2708+
kind: TraitItemKind::Fn(..),
2709+
owner_id,
2710+
..
2711+
})
2712+
| Node::ImplItem(&ImplItem {
2713+
kind: ImplItemKind::Fn(..),
2714+
owner_id,
2715+
..
2716+
}) => ExprUseNode::Return(owner_id),
2717+
2718+
Node::Expr(use_expr) => match use_expr.kind {
2719+
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
2720+
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
2721+
}),
2722+
2723+
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
2724+
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == self.child_id) {
2725+
Some(i) => ExprUseNode::FnArg(func, i),
2726+
None => ExprUseNode::Callee,
2727+
},
2728+
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
2729+
use_expr.hir_id,
2730+
name.args,
2731+
args.iter()
2732+
.position(|arg| arg.hir_id == self.child_id)
2733+
.map_or(0, |i| i + 1),
2734+
),
2735+
ExprKind::Field(_, name) => ExprUseNode::FieldAccess(name),
2736+
ExprKind::AddrOf(kind, mutbl, _) => ExprUseNode::AddrOf(kind, mutbl),
2737+
_ => ExprUseNode::Other,
2738+
},
2739+
_ => ExprUseNode::Other,
2740+
}
2741+
}
26742742
}
26752743

26762744
/// The node which consumes a value.
@@ -2691,7 +2759,8 @@ pub enum ExprUseNode<'tcx> {
26912759
Callee,
26922760
/// Access of a field.
26932761
FieldAccess(Ident),
2694-
Expr,
2762+
/// Borrow expression.
2763+
AddrOf(ast::BorrowKind, Mutability),
26952764
Other,
26962765
}
26972766
impl<'tcx> ExprUseNode<'tcx> {
@@ -2768,26 +2837,25 @@ impl<'tcx> ExprUseNode<'tcx> {
27682837
let sig = cx.tcx.fn_sig(id).skip_binder();
27692838
Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i))))
27702839
},
2771-
Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None,
2840+
Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Other | Self::AddrOf(..) => None,
27722841
}
27732842
}
27742843
}
27752844

27762845
/// Gets the context an expression's value is used in.
2777-
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> {
2846+
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> ExprUseCtxt<'tcx> {
27782847
let mut adjustments = [].as_slice();
27792848
let mut is_ty_unified = false;
27802849
let mut moved_before_use = false;
2850+
let mut same_ctxt = true;
27812851
let ctxt = e.span.ctxt();
2782-
walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| {
2852+
let node = walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| -> ControlFlow<!> {
27832853
if adjustments.is_empty()
27842854
&& let Node::Expr(e) = cx.tcx.hir_node(child_id)
27852855
{
27862856
adjustments = cx.typeck_results().expr_adjustments(e);
27872857
}
2788-
if cx.tcx.hir().span(parent_id).ctxt() != ctxt {
2789-
return ControlFlow::Break(());
2790-
}
2858+
same_ctxt &= cx.tcx.hir().span(parent_id).ctxt() == ctxt;
27912859
if let Node::Expr(e) = parent {
27922860
match e.kind {
27932861
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
@@ -2803,71 +2871,25 @@ pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Optio
28032871
}
28042872
}
28052873
ControlFlow::Continue(())
2806-
})?
2807-
.continue_value()
2808-
.map(|(use_node, child_id)| {
2809-
let node = match use_node {
2810-
Node::LetStmt(l) => ExprUseNode::LetStmt(l),
2811-
Node::ExprField(field) => ExprUseNode::Field(field),
2812-
2813-
Node::Item(&Item {
2814-
kind: ItemKind::Static(..) | ItemKind::Const(..),
2815-
owner_id,
2816-
..
2817-
})
2818-
| Node::TraitItem(&TraitItem {
2819-
kind: TraitItemKind::Const(..),
2820-
owner_id,
2821-
..
2822-
})
2823-
| Node::ImplItem(&ImplItem {
2824-
kind: ImplItemKind::Const(..),
2825-
owner_id,
2826-
..
2827-
}) => ExprUseNode::ConstStatic(owner_id),
2828-
2829-
Node::Item(&Item {
2830-
kind: ItemKind::Fn(..),
2831-
owner_id,
2832-
..
2833-
})
2834-
| Node::TraitItem(&TraitItem {
2835-
kind: TraitItemKind::Fn(..),
2836-
owner_id,
2837-
..
2838-
})
2839-
| Node::ImplItem(&ImplItem {
2840-
kind: ImplItemKind::Fn(..),
2841-
owner_id,
2842-
..
2843-
}) => ExprUseNode::Return(owner_id),
2844-
2845-
Node::Expr(use_expr) => match use_expr.kind {
2846-
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
2847-
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
2848-
}),
2849-
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
2850-
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == child_id) {
2851-
Some(i) => ExprUseNode::FnArg(func, i),
2852-
None => ExprUseNode::Callee,
2853-
},
2854-
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
2855-
use_expr.hir_id,
2856-
name.args,
2857-
args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
2858-
),
2859-
ExprKind::Field(child, name) if child.hir_id == e.hir_id => ExprUseNode::FieldAccess(name),
2860-
_ => ExprUseNode::Expr,
2861-
},
2862-
_ => ExprUseNode::Other,
2863-
};
2864-
ExprUseCtxt {
2874+
});
2875+
match node {
2876+
Some(ControlFlow::Continue((node, child_id))) => ExprUseCtxt {
28652877
node,
2878+
child_id,
28662879
adjustments,
28672880
is_ty_unified,
28682881
moved_before_use,
2869-
}
2870-
})
2882+
same_ctxt,
2883+
},
2884+
None => ExprUseCtxt {
2885+
node: Node::Crate(cx.tcx.hir().root_module()),
2886+
child_id: HirId::INVALID,
2887+
adjustments: &[],
2888+
is_ty_unified: true,
2889+
moved_before_use: true,
2890+
same_ctxt: false,
2891+
},
2892+
}
28712893
}
28722894

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

0 commit comments

Comments
 (0)