Auto merge of #9791 - smoelius:issues-9739-9782, r=Jarcho

Address issues 9739 and 9782

This PR fixes #9739 in the manner I suggested in https://github.com/rust-lang/rust-clippy/issues/9739#issuecomment-1296802376.

This PR also fixes the compilation failures in #9782 (but doesn't address `@e00E's` other objections).

Fixes #9739

r? `@Jarcho`

changelog: Fix two `needless_borrow` false positives, one involving borrows in `if`-`else`s, the other involving qualified function calls
This commit is contained in:
bors 2022-11-08 14:15:40 +00:00
commit b9ca3195ab
4 changed files with 233 additions and 24 deletions

View file

@ -805,30 +805,39 @@ fn walk_parents<'tcx>(
.position(|arg| arg.hir_id == child_id) .position(|arg| arg.hir_id == child_id)
.zip(expr_sig(cx, func)) .zip(expr_sig(cx, func))
.and_then(|(i, sig)| { .and_then(|(i, sig)| {
sig.input_with_hir(i).map(|(hir_ty, ty)| match hir_ty { sig.input_with_hir(i).map(|(hir_ty, ty)| {
// Type inference for closures can depend on how they're called. Only go by the explicit match hir_ty {
// types here. // Type inference for closures can depend on how they're called. Only go by the explicit
Some(hir_ty) => binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars()), // types here.
None => { Some(hir_ty) => {
if let ty::Param(param_ty) = ty.skip_binder().kind() { binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars())
needless_borrow_impl_arg_position( },
cx, None => {
possible_borrowers, // `e.hir_id == child_id` for https://github.com/rust-lang/rust-clippy/issues/9739
parent, // `!call_is_qualified(func)` for https://github.com/rust-lang/rust-clippy/issues/9782
i, if e.hir_id == child_id
*param_ty, && !call_is_qualified(func)
e, && let ty::Param(param_ty) = ty.skip_binder().kind()
precedence, {
msrv, needless_borrow_impl_arg_position(
) cx,
} else { possible_borrowers,
ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence) parent,
.position_for_arg() i,
} *param_ty,
}, e,
precedence,
msrv,
)
} else {
ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence)
.position_for_arg()
}
},
}
}) })
}), }),
ExprKind::MethodCall(_, receiver, args, _) => { ExprKind::MethodCall(method, receiver, args, _) => {
let id = cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap(); let id = cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap();
if receiver.hir_id == child_id { if receiver.hir_id == child_id {
// Check for calls to trait methods where the trait is implemented on a reference. // Check for calls to trait methods where the trait is implemented on a reference.
@ -866,7 +875,9 @@ fn walk_parents<'tcx>(
} }
args.iter().position(|arg| arg.hir_id == child_id).map(|i| { args.iter().position(|arg| arg.hir_id == child_id).map(|i| {
let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i + 1]; let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i + 1];
if let ty::Param(param_ty) = ty.kind() { // `e.hir_id == child_id` for https://github.com/rust-lang/rust-clippy/issues/9739
// `method.args.is_none()` for https://github.com/rust-lang/rust-clippy/issues/9782
if e.hir_id == child_id && method.args.is_none() && let ty::Param(param_ty) = ty.kind() {
needless_borrow_impl_arg_position( needless_borrow_impl_arg_position(
cx, cx,
possible_borrowers, possible_borrowers,
@ -1044,6 +1055,18 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
v.0 v.0
} }
fn call_is_qualified(expr: &Expr<'_>) -> bool {
if let ExprKind::Path(path) = &expr.kind {
match path {
QPath::Resolved(_, path) => path.segments.last().map_or(false, |segment| segment.args.is_some()),
QPath::TypeRelative(_, segment) => segment.args.is_some(),
QPath::LangItem(..) => false,
}
} else {
false
}
}
// Checks whether: // Checks whether:
// * child is an expression of the form `&e` in an argument position requiring an `impl Trait` // * child is an expression of the form `&e` in an argument position requiring an `impl Trait`
// * `e`'s type implements `Trait` and is copyable // * `e`'s type implements `Trait` and is copyable

View file

@ -420,3 +420,93 @@ mod issue_9710 {
fn f<T: AsRef<str>>(_: T) {} fn f<T: AsRef<str>>(_: T) {}
} }
#[allow(dead_code)]
mod issue_9739 {
fn foo<D: std::fmt::Display>(_it: impl IntoIterator<Item = D>) {}
fn main() {
foo(if std::env::var_os("HI").is_some() {
&[0]
} else {
&[] as &[u32]
});
}
}
#[allow(dead_code)]
mod issue_9739_method_variant {
struct S;
impl S {
fn foo<D: std::fmt::Display>(&self, _it: impl IntoIterator<Item = D>) {}
}
fn main() {
S.foo(if std::env::var_os("HI").is_some() {
&[0]
} else {
&[] as &[u32]
});
}
}
#[allow(dead_code)]
mod issue_9782 {
fn foo<T: AsRef<[u8]>>(t: T) {
println!("{}", std::mem::size_of::<T>());
let _t: &[u8] = t.as_ref();
}
fn main() {
let a: [u8; 100] = [0u8; 100];
// 100
foo::<[u8; 100]>(a);
foo(a);
// 16
foo::<&[u8]>(&a);
foo(a.as_slice());
// 8
foo::<&[u8; 100]>(&a);
foo(a);
}
}
#[allow(dead_code)]
mod issue_9782_type_relative_variant {
struct S;
impl S {
fn foo<T: AsRef<[u8]>>(t: T) {
println!("{}", std::mem::size_of::<T>());
let _t: &[u8] = t.as_ref();
}
}
fn main() {
let a: [u8; 100] = [0u8; 100];
S::foo::<&[u8; 100]>(&a);
}
}
#[allow(dead_code)]
mod issue_9782_method_variant {
struct S;
impl S {
fn foo<T: AsRef<[u8]>>(&self, t: T) {
println!("{}", std::mem::size_of::<T>());
let _t: &[u8] = t.as_ref();
}
}
fn main() {
let a: [u8; 100] = [0u8; 100];
S.foo::<&[u8; 100]>(&a);
}
}

View file

@ -420,3 +420,93 @@ mod issue_9710 {
fn f<T: AsRef<str>>(_: T) {} fn f<T: AsRef<str>>(_: T) {}
} }
#[allow(dead_code)]
mod issue_9739 {
fn foo<D: std::fmt::Display>(_it: impl IntoIterator<Item = D>) {}
fn main() {
foo(if std::env::var_os("HI").is_some() {
&[0]
} else {
&[] as &[u32]
});
}
}
#[allow(dead_code)]
mod issue_9739_method_variant {
struct S;
impl S {
fn foo<D: std::fmt::Display>(&self, _it: impl IntoIterator<Item = D>) {}
}
fn main() {
S.foo(if std::env::var_os("HI").is_some() {
&[0]
} else {
&[] as &[u32]
});
}
}
#[allow(dead_code)]
mod issue_9782 {
fn foo<T: AsRef<[u8]>>(t: T) {
println!("{}", std::mem::size_of::<T>());
let _t: &[u8] = t.as_ref();
}
fn main() {
let a: [u8; 100] = [0u8; 100];
// 100
foo::<[u8; 100]>(a);
foo(a);
// 16
foo::<&[u8]>(&a);
foo(a.as_slice());
// 8
foo::<&[u8; 100]>(&a);
foo(&a);
}
}
#[allow(dead_code)]
mod issue_9782_type_relative_variant {
struct S;
impl S {
fn foo<T: AsRef<[u8]>>(t: T) {
println!("{}", std::mem::size_of::<T>());
let _t: &[u8] = t.as_ref();
}
}
fn main() {
let a: [u8; 100] = [0u8; 100];
S::foo::<&[u8; 100]>(&a);
}
}
#[allow(dead_code)]
mod issue_9782_method_variant {
struct S;
impl S {
fn foo<T: AsRef<[u8]>>(&self, t: T) {
println!("{}", std::mem::size_of::<T>());
let _t: &[u8] = t.as_ref();
}
}
fn main() {
let a: [u8; 100] = [0u8; 100];
S.foo::<&[u8; 100]>(&a);
}
}

View file

@ -210,5 +210,11 @@ error: the borrowed expression implements the required traits
LL | use_x(&x); LL | use_x(&x);
| ^^ help: change this to: `x` | ^^ help: change this to: `x`
error: aborting due to 35 previous errors error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:474:13
|
LL | foo(&a);
| ^^ help: change this to: `a`
error: aborting due to 36 previous errors