Skip to content

[useless_conversion]: don't lint if type parameter has unsatisfiable bounds for .into_iter() receiver #11301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 90 additions & 20 deletions clippy_lints/src/useless_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ use rustc_errors::Applicability;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::Obligation;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_middle::traits::ObligationCause;
use rustc_middle::ty::{self, EarlyBinder, GenericArg, GenericArgsRef, Ty, TypeVisitableExt};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{sym, Span};
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -61,22 +65,69 @@ impl MethodOrFunction {
}
}

/// Returns the span of the `IntoIterator` trait bound in the function pointed to by `fn_did`
fn into_iter_bound(cx: &LateContext<'_>, fn_did: DefId, into_iter_did: DefId, param_index: u32) -> Option<Span> {
cx.tcx
.predicates_of(fn_did)
.predicates
.iter()
.find_map(|&(ref pred, span)| {
if let ty::ClauseKind::Trait(tr) = pred.kind().skip_binder()
&& tr.def_id() == into_iter_did
&& tr.self_ty().is_param(param_index)
{
Some(span)
} else {
None
/// Returns the span of the `IntoIterator` trait bound in the function pointed to by `fn_did`,
/// iff all of the bounds also hold for the type of the `.into_iter()` receiver.
/// ```ignore
/// pub fn foo<I>(i: I)
/// where I: IntoIterator<Item=i32> + ExactSizeIterator
/// ^^^^^^^^^^^^^^^^^ this extra bound stops us from suggesting to remove `.into_iter()` ...
/// {
/// assert_eq!(i.len(), 3);
/// }
///
/// pub fn bar() {
/// foo([1, 2, 3].into_iter());
/// ^^^^^^^^^^^^ ... here, because `[i32; 3]` is not `ExactSizeIterator`
/// }
/// ```
fn into_iter_bound<'tcx>(
cx: &LateContext<'tcx>,
fn_did: DefId,
into_iter_did: DefId,
into_iter_receiver: Ty<'tcx>,
param_index: u32,
node_args: GenericArgsRef<'tcx>,
) -> Option<Span> {
let param_env = cx.tcx.param_env(fn_did);
let mut into_iter_span = None;

for (pred, span) in cx.tcx.explicit_predicates_of(fn_did).predicates {
if let ty::ClauseKind::Trait(tr) = pred.kind().skip_binder() {
if tr.self_ty().is_param(param_index) {
if tr.def_id() == into_iter_did {
into_iter_span = Some(*span);
} else {
let tr = cx.tcx.erase_regions(tr);
if tr.has_escaping_bound_vars() {
return None;
}

// Substitute generics in the predicate and replace the IntoIterator type parameter with the
// `.into_iter()` receiver to see if the bound also holds for that type.
let args = cx.tcx.mk_args_from_iter(node_args.iter().enumerate().map(|(i, arg)| {
if i == param_index as usize {
GenericArg::from(into_iter_receiver)
} else {
arg
}
}));

let predicate = EarlyBinder::bind(tr).instantiate(cx.tcx, args);
let obligation = Obligation::new(cx.tcx, ObligationCause::dummy(), param_env, predicate);
if !cx
.tcx
.infer_ctxt()
.build()
.predicate_must_hold_modulo_regions(&obligation)
{
return None;
}
}
}
})
}
}

into_iter_span
}

/// Extracts the receiver of a `.into_iter()` method call.
Expand Down Expand Up @@ -160,22 +211,41 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
// `fn_sig` does not ICE. (see #11065)
&& cx.tcx.opt_def_kind(did).is_some_and(DefKind::is_fn_like) =>
{
Some((did, args, MethodOrFunction::Function))
Some((
did,
args,
cx.typeck_results().node_args(recv.hir_id),
MethodOrFunction::Function
))
}
ExprKind::MethodCall(.., args, _) => {
cx.typeck_results().type_dependent_def_id(parent.hir_id)
.map(|did| (did, args, MethodOrFunction::Method))
.map(|did| {
return (
did,
args,
cx.typeck_results().node_args(parent.hir_id),
MethodOrFunction::Method
);
})
}
_ => None,
};

if let Some((parent_fn_did, args, kind)) = parent_fn
if let Some((parent_fn_did, args, node_args, kind)) = parent_fn
&& let Some(into_iter_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
&& let sig = cx.tcx.fn_sig(parent_fn_did).skip_binder().skip_binder()
&& let Some(arg_pos) = args.iter().position(|x| x.hir_id == e.hir_id)
&& let Some(&into_iter_param) = sig.inputs().get(kind.param_pos(arg_pos))
&& let ty::Param(param) = into_iter_param.kind()
&& let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
&& let Some(span) = into_iter_bound(
cx,
parent_fn_did,
into_iter_did,
cx.typeck_results().expr_ty(into_iter_recv),
param.index,
node_args
)
&& self.expn_depth == 0
{
// Get the "innermost" `.into_iter()` call, e.g. given this expression:
Expand Down
91 changes: 91 additions & 0 deletions tests/ui/useless_conversion.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ fn main() {
let _ = s3;
let s4: Foo<'a'> = Foo;
let _ = vec![s4, s4, s4].into_iter();

issue11300::bar();
}

#[allow(dead_code)]
Expand Down Expand Up @@ -196,6 +198,95 @@ fn explicit_into_iter_fn_arg() {
b(macro_generated!());
}

mod issue11300 {
pub fn foo<I>(i: I)
where
I: IntoIterator<Item = i32> + ExactSizeIterator,
{
assert_eq!(i.len(), 3);
}

trait Helper<T: ?Sized> {}
impl Helper<i32> for [i32; 3] {}
impl Helper<i32> for std::array::IntoIter<i32, 3> {}
impl Helper<()> for std::array::IntoIter<i32, 3> {}

fn foo2<X: ?Sized, I>(_: I)
where
I: IntoIterator<Item = i32> + Helper<X>,
{
}

trait Helper2<T> {}
impl Helper2<std::array::IntoIter<i32, 3>> for i32 {}
impl Helper2<[i32; 3]> for i32 {}
fn foo3<I>(_: I)
where
I: IntoIterator<Item = i32>,
i32: Helper2<I>,
{
}

pub fn bar() {
// This should not trigger the lint:
// `[i32, 3]` does not satisfy the `ExactSizeIterator` bound, so the into_iter call cannot be
// removed and is not useless.
foo([1, 2, 3].into_iter());

// This should trigger the lint, receiver type [i32; 3] also implements `Helper`
foo2::<i32, _>([1, 2, 3]);

// This again should *not* lint, since X = () and I = std::array::IntoIter<i32, 3>,
// and `[i32; 3]: Helper<()>` is not true (only `std::array::IntoIter<i32, 3>: Helper<()>` is).
foo2::<(), _>([1, 2, 3].into_iter());

// This should lint. Removing the `.into_iter()` means that `I` gets substituted with `[i32; 3]`,
// and `i32: Helper2<[i32, 3]>` is true, so this call is indeed unncessary.
foo3([1, 2, 3]);
}

fn ice() {
struct S1;
impl S1 {
pub fn foo<I: IntoIterator>(&self, _: I) {}
}

S1.foo([1, 2]);

// ICE that occured in itertools
trait Itertools {
fn interleave_shortest<J>(self, other: J)
where
J: IntoIterator,
Self: Sized;
}
impl<I: Iterator> Itertools for I {
fn interleave_shortest<J>(self, other: J)
where
J: IntoIterator,
Self: Sized,
{
}
}
let v0: Vec<i32> = vec![0, 2, 4];
let v1: Vec<i32> = vec![1, 3, 5, 7];
v0.into_iter().interleave_shortest(v1);

trait TraitWithLifetime<'a> {}
impl<'a> TraitWithLifetime<'a> for std::array::IntoIter<&'a i32, 2> {}

struct Helper;
impl<'a> Helper {
fn with_lt<I>(&self, _: I)
where
I: IntoIterator + TraitWithLifetime<'a>,
{
}
}
Helper.with_lt([&1, &2].into_iter());
}
}

#[derive(Copy, Clone)]
struct Foo<const C: char>;

Expand Down
91 changes: 91 additions & 0 deletions tests/ui/useless_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ fn main() {
let _ = Foo::<'a'>::from(s3);
let s4: Foo<'a'> = Foo;
let _ = vec![s4, s4, s4].into_iter().into_iter();

issue11300::bar();
}

#[allow(dead_code)]
Expand Down Expand Up @@ -196,6 +198,95 @@ fn explicit_into_iter_fn_arg() {
b(macro_generated!());
}

mod issue11300 {
pub fn foo<I>(i: I)
where
I: IntoIterator<Item = i32> + ExactSizeIterator,
{
assert_eq!(i.len(), 3);
}

trait Helper<T: ?Sized> {}
impl Helper<i32> for [i32; 3] {}
impl Helper<i32> for std::array::IntoIter<i32, 3> {}
impl Helper<()> for std::array::IntoIter<i32, 3> {}

fn foo2<X: ?Sized, I>(_: I)
where
I: IntoIterator<Item = i32> + Helper<X>,
{
}

trait Helper2<T> {}
impl Helper2<std::array::IntoIter<i32, 3>> for i32 {}
impl Helper2<[i32; 3]> for i32 {}
fn foo3<I>(_: I)
where
I: IntoIterator<Item = i32>,
i32: Helper2<I>,
{
}

pub fn bar() {
// This should not trigger the lint:
// `[i32, 3]` does not satisfy the `ExactSizeIterator` bound, so the into_iter call cannot be
// removed and is not useless.
foo([1, 2, 3].into_iter());

// This should trigger the lint, receiver type [i32; 3] also implements `Helper`
foo2::<i32, _>([1, 2, 3].into_iter());

// This again should *not* lint, since X = () and I = std::array::IntoIter<i32, 3>,
// and `[i32; 3]: Helper<()>` is not true (only `std::array::IntoIter<i32, 3>: Helper<()>` is).
foo2::<(), _>([1, 2, 3].into_iter());

// This should lint. Removing the `.into_iter()` means that `I` gets substituted with `[i32; 3]`,
// and `i32: Helper2<[i32, 3]>` is true, so this call is indeed unncessary.
foo3([1, 2, 3].into_iter());
}

fn ice() {
struct S1;
impl S1 {
pub fn foo<I: IntoIterator>(&self, _: I) {}
}

S1.foo([1, 2].into_iter());

// ICE that occured in itertools
trait Itertools {
fn interleave_shortest<J>(self, other: J)
where
J: IntoIterator,
Self: Sized;
}
impl<I: Iterator> Itertools for I {
fn interleave_shortest<J>(self, other: J)
where
J: IntoIterator,
Self: Sized,
{
}
}
let v0: Vec<i32> = vec![0, 2, 4];
let v1: Vec<i32> = vec![1, 3, 5, 7];
v0.into_iter().interleave_shortest(v1.into_iter());

trait TraitWithLifetime<'a> {}
impl<'a> TraitWithLifetime<'a> for std::array::IntoIter<&'a i32, 2> {}

struct Helper;
impl<'a> Helper {
fn with_lt<I>(&self, _: I)
where
I: IntoIterator + TraitWithLifetime<'a>,
{
}
}
Helper.with_lt([&1, &2].into_iter());
}
}

#[derive(Copy, Clone)]
struct Foo<const C: char>;

Expand Down
Loading