Skip to content

Commit b1e5a58

Browse files
borsbors
committedFeb 6, 2024
Auto merge of #11812 - Jarcho:issue_11786, r=Alexendoo
Return `Some` from `walk_to_expr_usage` more fixes #11786 supersedes #11097 The code removed in the first commit would have needed changes due to the second commit. Since it's useless it just gets removed instead. changelog: `needless_borrow`: Fix linting in tuple and array expressions.
2 parents d910f77 + cdc4c69 commit b1e5a58

11 files changed

+154
-200
lines changed
 

‎clippy_utils/src/lib.rs

+78-147
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![feature(array_chunks)]
22
#![feature(box_patterns)]
3+
#![feature(control_flow_enum)]
34
#![feature(if_let_guard)]
45
#![feature(let_chains)]
56
#![feature(lint_reasons)]
@@ -116,10 +117,7 @@ use visitors::Visitable;
116117

117118
use crate::consts::{constant, mir_to_const, Constant};
118119
use crate::higher::Range;
119-
use crate::ty::{
120-
adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type,
121-
ty_is_fn_once_param,
122-
};
120+
use crate::ty::{adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type};
123121
use crate::visitors::for_each_expr;
124122

125123
use rustc_middle::hir::nested_filter;
@@ -1355,46 +1353,12 @@ pub fn get_enclosing_loop_or_multi_call_closure<'tcx>(
13551353
for (_, node) in cx.tcx.hir().parent_iter(expr.hir_id) {
13561354
match node {
13571355
Node::Expr(e) => match e.kind {
1358-
ExprKind::Closure { .. } => {
1356+
ExprKind::Closure { .. }
13591357
if let rustc_ty::Closure(_, subs) = cx.typeck_results().expr_ty(e).kind()
1360-
&& subs.as_closure().kind() == ClosureKind::FnOnce
1361-
{
1362-
continue;
1363-
}
1364-
let is_once = walk_to_expr_usage(cx, e, |node, id| {
1365-
let Node::Expr(e) = node else {
1366-
return None;
1367-
};
1368-
match e.kind {
1369-
ExprKind::Call(f, _) if f.hir_id == id => Some(()),
1370-
ExprKind::Call(f, args) => {
1371-
let i = args.iter().position(|arg| arg.hir_id == id)?;
1372-
let sig = expr_sig(cx, f)?;
1373-
let predicates = sig
1374-
.predicates_id()
1375-
.map_or(cx.param_env, |id| cx.tcx.param_env(id))
1376-
.caller_bounds();
1377-
sig.input(i).and_then(|ty| {
1378-
ty_is_fn_once_param(cx.tcx, ty.skip_binder(), predicates).then_some(())
1379-
})
1380-
},
1381-
ExprKind::MethodCall(_, receiver, args, _) => {
1382-
let i = std::iter::once(receiver)
1383-
.chain(args.iter())
1384-
.position(|arg| arg.hir_id == id)?;
1385-
let id = cx.typeck_results().type_dependent_def_id(e.hir_id)?;
1386-
let ty = cx.tcx.fn_sig(id).instantiate_identity().skip_binder().inputs()[i];
1387-
ty_is_fn_once_param(cx.tcx, ty, cx.tcx.param_env(id).caller_bounds()).then_some(())
1388-
},
1389-
_ => None,
1390-
}
1391-
})
1392-
.is_some();
1393-
if !is_once {
1394-
return Some(e);
1395-
}
1396-
},
1397-
ExprKind::Loop(..) => return Some(e),
1358+
&& subs.as_closure().kind() == ClosureKind::FnOnce => {},
1359+
1360+
// Note: A closure's kind is determined by how it's used, not it's captures.
1361+
ExprKind::Closure { .. } | ExprKind::Loop(..) => return Some(e),
13981362
_ => (),
13991363
},
14001364
Node::Stmt(_) | Node::Block(_) | Node::Local(_) | Node::Arm(_) => (),
@@ -2592,26 +2556,30 @@ pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
25922556
&& item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
25932557
}
25942558

2595-
/// Walks the HIR tree from the given expression, up to the node where the value produced by the
2596-
/// expression is consumed. Calls the function for every node encountered this way until it returns
2597-
/// `Some`.
2559+
/// Walks up the HIR tree from the given expression in an attempt to find where the value is
2560+
/// consumed.
2561+
///
2562+
/// Termination has three conditions:
2563+
/// - The given function returns `Break`. This function will return the value.
2564+
/// - The consuming node is found. This function will return `Continue(use_node, child_id)`.
2565+
/// - No further parent nodes are found. This will trigger a debug assert or return `None`.
25982566
///
2599-
/// This allows walking through `if`, `match`, `break`, block expressions to find where the value
2600-
/// produced by the expression is consumed.
2567+
/// This allows walking through `if`, `match`, `break`, and block expressions to find where the
2568+
/// value produced by the expression is consumed.
26012569
pub fn walk_to_expr_usage<'tcx, T>(
26022570
cx: &LateContext<'tcx>,
26032571
e: &Expr<'tcx>,
2604-
mut f: impl FnMut(Node<'tcx>, HirId) -> Option<T>,
2605-
) -> Option<T> {
2572+
mut f: impl FnMut(HirId, Node<'tcx>, HirId) -> ControlFlow<T>,
2573+
) -> Option<ControlFlow<T, (Node<'tcx>, HirId)>> {
26062574
let map = cx.tcx.hir();
26072575
let mut iter = map.parent_iter(e.hir_id);
26082576
let mut child_id = e.hir_id;
26092577

26102578
while let Some((parent_id, parent)) = iter.next() {
2611-
if let Some(x) = f(parent, child_id) {
2612-
return Some(x);
2579+
if let ControlFlow::Break(x) = f(parent_id, parent, child_id) {
2580+
return Some(ControlFlow::Break(x));
26132581
}
2614-
let parent = match parent {
2582+
let parent_expr = match parent {
26152583
Node::Expr(e) => e,
26162584
Node::Block(Block { expr: Some(body), .. }) | Node::Arm(Arm { body, .. }) if body.hir_id == child_id => {
26172585
child_id = parent_id;
@@ -2621,18 +2589,19 @@ pub fn walk_to_expr_usage<'tcx, T>(
26212589
child_id = parent_id;
26222590
continue;
26232591
},
2624-
_ => return None,
2592+
_ => return Some(ControlFlow::Continue((parent, child_id))),
26252593
};
2626-
match parent.kind {
2594+
match parent_expr.kind {
26272595
ExprKind::If(child, ..) | ExprKind::Match(child, ..) if child.hir_id != child_id => child_id = parent_id,
26282596
ExprKind::Break(Destination { target_id: Ok(id), .. }, _) => {
26292597
child_id = id;
26302598
iter = map.parent_iter(id);
26312599
},
2632-
ExprKind::Block(..) => child_id = parent_id,
2633-
_ => return None,
2600+
ExprKind::Block(..) | ExprKind::DropTemps(_) => child_id = parent_id,
2601+
_ => return Some(ControlFlow::Continue((parent, child_id))),
26342602
}
26352603
}
2604+
debug_assert!(false, "no parent node found for `{child_id:?}`");
26362605
None
26372606
}
26382607

@@ -2676,6 +2645,8 @@ pub enum ExprUseNode<'tcx> {
26762645
Callee,
26772646
/// Access of a field.
26782647
FieldAccess(Ident),
2648+
Expr,
2649+
Other,
26792650
}
26802651
impl<'tcx> ExprUseNode<'tcx> {
26812652
/// Checks if the value is returned from the function.
@@ -2752,144 +2723,104 @@ impl<'tcx> ExprUseNode<'tcx> {
27522723
let sig = cx.tcx.fn_sig(id).skip_binder();
27532724
Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i))))
27542725
},
2755-
Self::Local(_) | Self::FieldAccess(..) | Self::Callee => None,
2726+
Self::Local(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None,
27562727
}
27572728
}
27582729
}
27592730

27602731
/// Gets the context an expression's value is used in.
2761-
#[expect(clippy::too_many_lines)]
27622732
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> {
27632733
let mut adjustments = [].as_slice();
27642734
let mut is_ty_unified = false;
27652735
let mut moved_before_use = false;
27662736
let ctxt = e.span.ctxt();
2767-
walk_to_expr_usage(cx, e, &mut |parent, child_id| {
2768-
// LocalTableInContext returns the wrong lifetime, so go use `expr_adjustments` instead.
2737+
walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| {
27692738
if adjustments.is_empty()
27702739
&& let Node::Expr(e) = cx.tcx.hir_node(child_id)
27712740
{
27722741
adjustments = cx.typeck_results().expr_adjustments(e);
27732742
}
2774-
match parent {
2775-
Node::Local(l) if l.span.ctxt() == ctxt => Some(ExprUseCtxt {
2776-
node: ExprUseNode::Local(l),
2777-
adjustments,
2778-
is_ty_unified,
2779-
moved_before_use,
2780-
}),
2743+
if !cx.tcx.hir().opt_span(parent_id).is_some_and(|x| x.ctxt() == ctxt) {
2744+
return ControlFlow::Break(());
2745+
}
2746+
if let Node::Expr(e) = parent {
2747+
match e.kind {
2748+
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
2749+
is_ty_unified = true;
2750+
moved_before_use = true;
2751+
},
2752+
ExprKind::Block(_, Some(_)) | ExprKind::Break(..) => {
2753+
is_ty_unified = true;
2754+
moved_before_use = true;
2755+
},
2756+
ExprKind::Block(..) => moved_before_use = true,
2757+
_ => {},
2758+
}
2759+
}
2760+
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+
27812768
Node::Item(&Item {
27822769
kind: ItemKind::Static(..) | ItemKind::Const(..),
27832770
owner_id,
2784-
span,
27852771
..
27862772
})
27872773
| Node::TraitItem(&TraitItem {
27882774
kind: TraitItemKind::Const(..),
27892775
owner_id,
2790-
span,
27912776
..
27922777
})
27932778
| Node::ImplItem(&ImplItem {
27942779
kind: ImplItemKind::Const(..),
27952780
owner_id,
2796-
span,
27972781
..
2798-
}) if span.ctxt() == ctxt => Some(ExprUseCtxt {
2799-
node: ExprUseNode::ConstStatic(owner_id),
2800-
adjustments,
2801-
is_ty_unified,
2802-
moved_before_use,
2803-
}),
2782+
}) => ExprUseNode::ConstStatic(owner_id),
28042783

28052784
Node::Item(&Item {
28062785
kind: ItemKind::Fn(..),
28072786
owner_id,
2808-
span,
28092787
..
28102788
})
28112789
| Node::TraitItem(&TraitItem {
28122790
kind: TraitItemKind::Fn(..),
28132791
owner_id,
2814-
span,
28152792
..
28162793
})
28172794
| Node::ImplItem(&ImplItem {
28182795
kind: ImplItemKind::Fn(..),
28192796
owner_id,
2820-
span,
28212797
..
2822-
}) if span.ctxt() == ctxt => Some(ExprUseCtxt {
2823-
node: ExprUseNode::Return(owner_id),
2824-
adjustments,
2825-
is_ty_unified,
2826-
moved_before_use,
2827-
}),
2828-
2829-
Node::ExprField(field) if field.span.ctxt() == ctxt => Some(ExprUseCtxt {
2830-
node: ExprUseNode::Field(field),
2831-
adjustments,
2832-
is_ty_unified,
2833-
moved_before_use,
2834-
}),
2798+
}) => ExprUseNode::Return(owner_id),
28352799

2836-
Node::Expr(parent) if parent.span.ctxt() == ctxt => match parent.kind {
2837-
ExprKind::Ret(_) => Some(ExprUseCtxt {
2838-
node: ExprUseNode::Return(OwnerId {
2839-
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
2840-
}),
2841-
adjustments,
2842-
is_ty_unified,
2843-
moved_before_use,
2844-
}),
2845-
ExprKind::Closure(closure) => Some(ExprUseCtxt {
2846-
node: ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
2847-
adjustments,
2848-
is_ty_unified,
2849-
moved_before_use,
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()),
28502803
}),
2851-
ExprKind::Call(func, args) => Some(ExprUseCtxt {
2852-
node: match args.iter().position(|arg| arg.hir_id == child_id) {
2853-
Some(i) => ExprUseNode::FnArg(func, i),
2854-
None => ExprUseNode::Callee,
2855-
},
2856-
adjustments,
2857-
is_ty_unified,
2858-
moved_before_use,
2859-
}),
2860-
ExprKind::MethodCall(name, _, args, _) => Some(ExprUseCtxt {
2861-
node: ExprUseNode::MethodArg(
2862-
parent.hir_id,
2863-
name.args,
2864-
args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
2865-
),
2866-
adjustments,
2867-
is_ty_unified,
2868-
moved_before_use,
2869-
}),
2870-
ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(ExprUseCtxt {
2871-
node: ExprUseNode::FieldAccess(name),
2872-
adjustments,
2873-
is_ty_unified,
2874-
moved_before_use,
2875-
}),
2876-
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
2877-
is_ty_unified = true;
2878-
moved_before_use = true;
2879-
None
2880-
},
2881-
ExprKind::Block(_, Some(_)) | ExprKind::Break(..) => {
2882-
is_ty_unified = true;
2883-
moved_before_use = true;
2884-
None
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,
28852808
},
2886-
ExprKind::Block(..) => {
2887-
moved_before_use = true;
2888-
None
2889-
},
2890-
_ => None,
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,
28912816
},
2892-
_ => None,
2817+
_ => ExprUseNode::Other,
2818+
};
2819+
ExprUseCtxt {
2820+
node,
2821+
adjustments,
2822+
is_ty_unified,
2823+
moved_before_use,
28932824
}
28942825
})
28952826
}

‎clippy_utils/src/ty.rs

-29
Original file line numberDiff line numberDiff line change
@@ -1000,35 +1000,6 @@ pub fn adt_and_variant_of_res<'tcx>(cx: &LateContext<'tcx>, res: Res) -> Option<
10001000
}
10011001
}
10021002

1003-
/// Checks if the type is a type parameter implementing `FnOnce`, but not `FnMut`.
1004-
pub fn ty_is_fn_once_param<'tcx>(tcx: TyCtxt<'_>, ty: Ty<'tcx>, predicates: &'tcx [ty::Clause<'_>]) -> bool {
1005-
let ty::Param(ty) = *ty.kind() else {
1006-
return false;
1007-
};
1008-
let lang = tcx.lang_items();
1009-
let (Some(fn_once_id), Some(fn_mut_id), Some(fn_id)) = (lang.fn_once_trait(), lang.fn_mut_trait(), lang.fn_trait())
1010-
else {
1011-
return false;
1012-
};
1013-
predicates
1014-
.iter()
1015-
.try_fold(false, |found, p| {
1016-
if let ty::ClauseKind::Trait(p) = p.kind().skip_binder()
1017-
&& let ty::Param(self_ty) = p.trait_ref.self_ty().kind()
1018-
&& ty.index == self_ty.index
1019-
{
1020-
// This should use `super_traits_of`, but that's a private function.
1021-
if p.trait_ref.def_id == fn_once_id {
1022-
return Some(true);
1023-
} else if p.trait_ref.def_id == fn_mut_id || p.trait_ref.def_id == fn_id {
1024-
return None;
1025-
}
1026-
}
1027-
Some(found)
1028-
})
1029-
.unwrap_or(false)
1030-
}
1031-
10321003
/// Comes up with an "at least" guesstimate for the type's size, not taking into
10331004
/// account the layout of type parameters.
10341005
pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {

‎tests/ui/ignored_unit_patterns.fixed

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
//@aux-build:proc_macro_derive.rs
22
#![warn(clippy::ignored_unit_patterns)]
3-
#![allow(clippy::let_unit_value, clippy::redundant_pattern_matching, clippy::single_match)]
3+
#![allow(
4+
clippy::let_unit_value,
5+
clippy::redundant_pattern_matching,
6+
clippy::single_match,
7+
clippy::needless_borrow
8+
)]
49

510
fn foo() -> Result<(), ()> {
611
unimplemented!()

0 commit comments

Comments
 (0)