nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-23 23:34:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #11812 - Jarcho:issue_11786, r=Alexendoo

Browse Source 
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.
This commit is contained in:
bors 2024-02-06 15:20:07 +00:00
commit b1e5a58427
11 changed files with 154 additions and 200 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

225
clippy_utils/src/lib.rs
View File

@ -1,5 +1,6 @@
#![feature(array_chunks)]
#![feature(box_patterns)]
#![feature(control_flow_enum)]
#![feature(if_let_guard)]
#![feature(let_chains)]
#![feature(lint_reasons)]
@ -116,10 +117,7 @@ use visitors::Visitable;
use crate::consts::{constant, mir_to_const, Constant};
use crate::higher::Range;
use crate::ty::{
adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type,
ty_is_fn_once_param,
};
use crate::ty::{adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type};
use crate::visitors::for_each_expr;
use rustc_middle::hir::nested_filter;
@ -1355,46 +1353,12 @@ pub fn get_enclosing_loop_or_multi_call_closure<'tcx>(
for (_, node) in cx.tcx.hir().parent_iter(expr.hir_id) {
match node {
Node::Expr(e) => match e.kind {
ExprKind::Closure { .. } => {
ExprKind::Closure { .. }
if let rustc_ty::Closure(_, subs) = cx.typeck_results().expr_ty(e).kind()
&& subs.as_closure().kind() == ClosureKind::FnOnce
{
continue;
}
let is_once = walk_to_expr_usage(cx, e, |node, id| {
let Node::Expr(e) = node else {
return None;
};
match e.kind {
ExprKind::Call(f, _) if f.hir_id == id => Some(()),
ExprKind::Call(f, args) => {
let i = args.iter().position(|arg| arg.hir_id == id)?;
let sig = expr_sig(cx, f)?;
let predicates = sig
.predicates_id()
.map_or(cx.param_env, |id| cx.tcx.param_env(id))
.caller_bounds();
sig.input(i).and_then(|ty| {
ty_is_fn_once_param(cx.tcx, ty.skip_binder(), predicates).then_some(())
})
},
ExprKind::MethodCall(_, receiver, args, _) => {
let i = std::iter::once(receiver)
.chain(args.iter())
.position(|arg| arg.hir_id == id)?;
let id = cx.typeck_results().type_dependent_def_id(e.hir_id)?;
let ty = cx.tcx.fn_sig(id).instantiate_identity().skip_binder().inputs()[i];
ty_is_fn_once_param(cx.tcx, ty, cx.tcx.param_env(id).caller_bounds()).then_some(())
},
_ => None,
}
})
.is_some();
if !is_once {
return Some(e);
}
},
ExprKind::Loop(..) => return Some(e),
&& subs.as_closure().kind() == ClosureKind::FnOnce => {},
// Note: A closure's kind is determined by how it's used, not it's captures.
ExprKind::Closure { .. } | ExprKind::Loop(..) => return Some(e),
_ => (),
},
Node::Stmt(_) | Node::Block(_) | Node::Local(_) | Node::Arm(_) => (),
@ -2592,26 +2556,30 @@ pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
&& item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
}
/// Walks the HIR tree from the given expression, up to the node where the value produced by the
/// expression is consumed. Calls the function for every node encountered this way until it returns
/// `Some`.
/// Walks up the HIR tree from the given expression in an attempt to find where the value is
/// consumed.
///
/// This allows walking through `if`, `match`, `break`, block expressions to find where the value
/// produced by the expression is consumed.
/// Termination has three conditions:
/// - The given function returns `Break`. This function will return the value.
/// - The consuming node is found. This function will return `Continue(use_node, child_id)`.
/// - No further parent nodes are found. This will trigger a debug assert or return `None`.
///
/// This allows walking through `if`, `match`, `break`, and block expressions to find where the
/// value produced by the expression is consumed.
pub fn walk_to_expr_usage<'tcx, T>(
cx: &LateContext<'tcx>,
e: &Expr<'tcx>,
mut f: impl FnMut(Node<'tcx>, HirId) -> Option<T>,
) -> Option<T> {
mut f: impl FnMut(HirId, Node<'tcx>, HirId) -> ControlFlow<T>,
) -> Option<ControlFlow<T, (Node<'tcx>, HirId)>> {
let map = cx.tcx.hir();
let mut iter = map.parent_iter(e.hir_id);
let mut child_id = e.hir_id;
while let Some((parent_id, parent)) = iter.next() {
if let Some(x) = f(parent, child_id) {
return Some(x);
if let ControlFlow::Break(x) = f(parent_id, parent, child_id) {
return Some(ControlFlow::Break(x));
}
let parent = match parent {
let parent_expr = match parent {
Node::Expr(e) => e,
Node::Block(Block { expr: Some(body), .. }) | Node::Arm(Arm { body, .. }) if body.hir_id == child_id => {
child_id = parent_id;
@ -2621,18 +2589,19 @@ pub fn walk_to_expr_usage<'tcx, T>(
child_id = parent_id;
continue;
},
_ => return None,
_ => return Some(ControlFlow::Continue((parent, child_id))),
};
match parent.kind {
match parent_expr.kind {
ExprKind::If(child, ..) | ExprKind::Match(child, ..) if child.hir_id != child_id => child_id = parent_id,
ExprKind::Break(Destination { target_id: Ok(id), .. }, _) => {
child_id = id;
iter = map.parent_iter(id);
},
ExprKind::Block(..) => child_id = parent_id,
_ => return None,
ExprKind::Block(..) | ExprKind::DropTemps(_) => child_id = parent_id,
_ => return Some(ControlFlow::Continue((parent, child_id))),
}
}
debug_assert!(false, "no parent node found for `{child_id:?}`");
None
}
@ -2676,6 +2645,8 @@ pub enum ExprUseNode<'tcx> {
Callee,
/// Access of a field.
FieldAccess(Ident),
Expr,
Other,
}
impl<'tcx> ExprUseNode<'tcx> {
/// Checks if the value is returned from the function.
@ -2752,144 +2723,104 @@ impl<'tcx> ExprUseNode<'tcx> {
let sig = cx.tcx.fn_sig(id).skip_binder();
Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i))))
},
Self::Local(_) | Self::FieldAccess(..) | Self::Callee => None,
Self::Local(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None,
}
}
}
/// Gets the context an expression's value is used in.
#[expect(clippy::too_many_lines)]
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> {
let mut adjustments = [].as_slice();
let mut is_ty_unified = false;
let mut moved_before_use = false;
let ctxt = e.span.ctxt();
walk_to_expr_usage(cx, e, &mut |parent, child_id| {
// LocalTableInContext returns the wrong lifetime, so go use `expr_adjustments` instead.
walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| {
if adjustments.is_empty()
&& let Node::Expr(e) = cx.tcx.hir_node(child_id)
{
adjustments = cx.typeck_results().expr_adjustments(e);
}
match parent {
Node::Local(l) if l.span.ctxt() == ctxt => Some(ExprUseCtxt {
node: ExprUseNode::Local(l),
adjustments,
is_ty_unified,
moved_before_use,
}),
if !cx.tcx.hir().opt_span(parent_id).is_some_and(|x| x.ctxt() == ctxt) {
return ControlFlow::Break(());
}
if let Node::Expr(e) = parent {
match e.kind {
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
is_ty_unified = true;
moved_before_use = true;
},
ExprKind::Block(_, Some(_)) | ExprKind::Break(..) => {
is_ty_unified = true;
moved_before_use = true;
},
ExprKind::Block(..) => moved_before_use = true,
_ => {},
}
}
ControlFlow::Continue(())
})?
.continue_value()
.map(|(use_node, child_id)| {
let node = match use_node {
Node::Local(l) => ExprUseNode::Local(l),
Node::ExprField(field) => ExprUseNode::Field(field),
Node::Item(&Item {
kind: ItemKind::Static(..) | ItemKind::Const(..),
owner_id,
span,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Const(..),
owner_id,
span,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Const(..),
owner_id,
span,
..
}) if span.ctxt() == ctxt => Some(ExprUseCtxt {
node: ExprUseNode::ConstStatic(owner_id),
adjustments,
is_ty_unified,
moved_before_use,
}),
}) => ExprUseNode::ConstStatic(owner_id),
Node::Item(&Item {
kind: ItemKind::Fn(..),
owner_id,
span,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Fn(..),
owner_id,
span,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Fn(..),
owner_id,
span,
..
}) if span.ctxt() == ctxt => Some(ExprUseCtxt {
node: ExprUseNode::Return(owner_id),
adjustments,
is_ty_unified,
moved_before_use,
}),
}) => ExprUseNode::Return(owner_id),
Node::ExprField(field) if field.span.ctxt() == ctxt => Some(ExprUseCtxt {
node: ExprUseNode::Field(field),
adjustments,
is_ty_unified,
moved_before_use,
}),
Node::Expr(parent) if parent.span.ctxt() == ctxt => match parent.kind {
ExprKind::Ret(_) => Some(ExprUseCtxt {
node: ExprUseNode::Return(OwnerId {
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
}),
adjustments,
is_ty_unified,
moved_before_use,
Node::Expr(use_expr) => match use_expr.kind {
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
}),
ExprKind::Closure(closure) => Some(ExprUseCtxt {
node: ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
adjustments,
is_ty_unified,
moved_before_use,
}),
ExprKind::Call(func, args) => Some(ExprUseCtxt {
node: match args.iter().position(|arg| arg.hir_id == child_id) {
Some(i) => ExprUseNode::FnArg(func, i),
None => ExprUseNode::Callee,
},
adjustments,
is_ty_unified,
moved_before_use,
}),
ExprKind::MethodCall(name, _, args, _) => Some(ExprUseCtxt {
node: ExprUseNode::MethodArg(
parent.hir_id,
name.args,
args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
),
adjustments,
is_ty_unified,
moved_before_use,
}),
ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(ExprUseCtxt {
node: ExprUseNode::FieldAccess(name),
adjustments,
is_ty_unified,
moved_before_use,
}),
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
is_ty_unified = true;
moved_before_use = true;
None
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == child_id) {
Some(i) => ExprUseNode::FnArg(func, i),
None => ExprUseNode::Callee,
},
ExprKind::Block(_, Some(_)) | ExprKind::Break(..) => {
is_ty_unified = true;
moved_before_use = true;
None
},
ExprKind::Block(..) => {
moved_before_use = true;
None
},
_ => None,
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
use_expr.hir_id,
name.args,
args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
),
ExprKind::Field(child, name) if child.hir_id == e.hir_id => ExprUseNode::FieldAccess(name),
_ => ExprUseNode::Expr,
},
_ => None,
_ => ExprUseNode::Other,
};
ExprUseCtxt {
node,
adjustments,
is_ty_unified,
moved_before_use,
}
})
}

29
clippy_utils/src/ty.rs
View File

@ -1000,35 +1000,6 @@ pub fn adt_and_variant_of_res<'tcx>(cx: &LateContext<'tcx>, res: Res) -> Option<
}
}
/// Checks if the type is a type parameter implementing `FnOnce`, but not `FnMut`.
pub fn ty_is_fn_once_param<'tcx>(tcx: TyCtxt<'_>, ty: Ty<'tcx>, predicates: &'tcx [ty::Clause<'_>]) -> bool {
let ty::Param(ty) = *ty.kind() else {
return false;
};
let lang = tcx.lang_items();
let (Some(fn_once_id), Some(fn_mut_id), Some(fn_id)) = (lang.fn_once_trait(), lang.fn_mut_trait(), lang.fn_trait())
else {
return false;
};
predicates
.iter()
.try_fold(false, |found, p| {
if let ty::ClauseKind::Trait(p) = p.kind().skip_binder()
&& let ty::Param(self_ty) = p.trait_ref.self_ty().kind()
&& ty.index == self_ty.index
{
// This should use `super_traits_of`, but that's a private function.
if p.trait_ref.def_id == fn_once_id {
return Some(true);
} else if p.trait_ref.def_id == fn_mut_id || p.trait_ref.def_id == fn_id {
return None;
}
}
Some(found)
})
.unwrap_or(false)
}
/// Comes up with an "at least" guesstimate for the type's size, not taking into
/// account the layout of type parameters.
pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {

7
tests/ui/ignored_unit_patterns.fixed
View File

@ -1,6 +1,11 @@
//@aux-build:proc_macro_derive.rs
#![warn(clippy::ignored_unit_patterns)]
#![allow(clippy::let_unit_value, clippy::redundant_pattern_matching, clippy::single_match)]
#![allow(
clippy::let_unit_value,
clippy::redundant_pattern_matching,
clippy::single_match,
clippy::needless_borrow
)]
fn foo() -> Result<(), ()> {
unimplemented!()

7
tests/ui/ignored_unit_patterns.rs
View File

@ -1,6 +1,11 @@
//@aux-build:proc_macro_derive.rs
#![warn(clippy::ignored_unit_patterns)]
#![allow(clippy::let_unit_value, clippy::redundant_pattern_matching, clippy::single_match)]
#![allow(
clippy::let_unit_value,
clippy::redundant_pattern_matching,
clippy::single_match,
clippy::needless_borrow
)]
fn foo() -> Result<(), ()> {
unimplemented!()

18
tests/ui/ignored_unit_patterns.stderr
View File

@ -1,5 +1,5 @@
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:11:12
--> $DIR/ignored_unit_patterns.rs:16:12
|
LL | Ok(_) => {},
| ^ help: use `()` instead of `_`: `()`
@ -8,49 +8,49 @@ LL | Ok(_) => {},
= help: to override `-D warnings` add `#[allow(clippy::ignored_unit_patterns)]`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:12:13
--> $DIR/ignored_unit_patterns.rs:17:13
|
LL | Err(_) => {},
| ^ help: use `()` instead of `_`: `()`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:14:15
--> $DIR/ignored_unit_patterns.rs:19:15
|
LL | if let Ok(_) = foo() {}
| ^ help: use `()` instead of `_`: `()`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:16:28
--> $DIR/ignored_unit_patterns.rs:21:28
|
LL | let _ = foo().map_err(|_| todo!());
| ^ help: use `()` instead of `_`: `()`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:22:16
--> $DIR/ignored_unit_patterns.rs:27:16
|
LL | Ok(_) => {},
| ^ help: use `()` instead of `_`: `()`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:24:17
--> $DIR/ignored_unit_patterns.rs:29:17
|
LL | Err(_) => {},
| ^ help: use `()` instead of `_`: `()`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:36:9
--> $DIR/ignored_unit_patterns.rs:41:9
|
LL | let _ = foo().unwrap();
| ^ help: use `()` instead of `_`: `()`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:45:13
--> $DIR/ignored_unit_patterns.rs:50:13
|
LL | (1, _) => unimplemented!(),
| ^ help: use `()` instead of `_`: `()`
error: matching over `()` is more explicit
--> $DIR/ignored_unit_patterns.rs:52:13
--> $DIR/ignored_unit_patterns.rs:57:13
|
LL | for (x, _) in v {
| ^ help: use `()` instead of `_`: `()`

3
tests/ui/needless_borrow.fixed
View File

@ -131,6 +131,9 @@ fn main() {
0
}
}
// issue #11786
let x: (&str,) = ("",);
}
#[allow(clippy::needless_borrowed_reference)]

3
tests/ui/needless_borrow.rs
View File

@ -131,6 +131,9 @@ fn main() {
0
}
}
// issue #11786
let x: (&str,) = (&"",);
}
#[allow(clippy::needless_borrowed_reference)]

20
tests/ui/needless_borrow.stderr
View File

@ -121,41 +121,47 @@ error: this expression creates a reference which is immediately dereferenced by
LL | (&&5).foo();
| ^^^^^ help: change this to: `(&5)`
error: this expression creates a reference which is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:136:23
|
LL | let x: (&str,) = (&"",);
| ^^^ help: change this to: `""`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:175:13
--> $DIR/needless_borrow.rs:178:13
|
LL | (&self.f)()
| ^^^^^^^^^ help: change this to: `(self.f)`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:184:13
--> $DIR/needless_borrow.rs:187:13
|
LL | (&mut self.f)()
| ^^^^^^^^^^^^^ help: change this to: `(self.f)`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:221:22
--> $DIR/needless_borrow.rs:224:22
|
LL | let _ = &mut (&mut { x.u }).x;
| ^^^^^^^^^^^^^^ help: change this to: `{ x.u }`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:228:22
--> $DIR/needless_borrow.rs:231:22
|
LL | let _ = &mut (&mut { x.u }).x;
| ^^^^^^^^^^^^^^ help: change this to: `{ x.u }`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:232:22
--> $DIR/needless_borrow.rs:235:22
|
LL | let _ = &mut (&mut x.u).x;
| ^^^^^^^^^^ help: change this to: `x.u`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:233:22
--> $DIR/needless_borrow.rs:236:22
|
LL | let _ = &mut (&mut { x.u }).x;
| ^^^^^^^^^^^^^^ help: change this to: `{ x.u }`
error: aborting due to 26 previous errors
error: aborting due to 27 previous errors

16
tests/ui/while_let_on_iterator.fixed
View File

@ -406,7 +406,7 @@ fn issue_8113() {
fn fn_once_closure() {
let mut it = 0..10;
(|| {
for x in it {
for x in it.by_ref() {
if x % 2 == 0 {
break;
}
@ -441,7 +441,19 @@ fn fn_once_closure() {
break;
}
}
})
});
trait MySpecialFnMut: FnOnce() {}
impl<T: FnOnce()> MySpecialFnMut for T {}
fn f4(_: impl MySpecialFnMut) {}
let mut it = 0..10;
f4(|| {
for x in it {
if x % 2 == 0 {
break;
}
}
});
}
fn main() {

14
tests/ui/while_let_on_iterator.rs
View File

@ -441,7 +441,19 @@ fn fn_once_closure() {
break;
}
}
})
});
trait MySpecialFnMut: FnOnce() {}
impl<T: FnOnce()> MySpecialFnMut for T {}
fn f4(_: impl MySpecialFnMut) {}
let mut it = 0..10;
f4(|| {
while let Some(x) = it.next() {
if x % 2 == 0 {
break;
}
}
});
}
fn main() {

12
tests/ui/while_let_on_iterator.stderr
View File

@ -131,7 +131,7 @@ error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:409:9
|
LL | while let Some(x) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:419:9
@ -152,10 +152,16 @@ LL | while let Some(x) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:449:5
--> $DIR/while_let_on_iterator.rs:451:9
|
LL | while let Some(x) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:461:5
|
LL | while let Some(..) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it`
error: aborting due to 26 previous errors
error: aborting due to 27 previous errors