Skip to content

Commit cb8df45

Browse files
committed
Auto merge of #10027 - smoelius:fix-10021, r=dswij
Fix 10021 This PR proposes a fix for #10021. The problem is similar to the one that `@mikerite` described in #9505. The compiler is generating an empty substitution for a call, even though the type of `Self` seems to be needed for a predicate. In `@mikerite's` case, the call was to [`IntoFuture::into_future`](https://doc.rust-lang.org/std/future/trait.IntoFuture.html#tymethod.into_future). In this case, the call is to [`Try::branch`](https://doc.rust-lang.org/std/ops/trait.Try.html#tymethod.branch). The proposed fix is to verify that the parameter whose type is changing has an index within the substitution. The strikes me as a reasonable approach, since if the check were to fail, the following code would be a no-op: https://github.com/rust-lang/rust-clippy/blob/4c123a06ba3c2ec17d3a4dfa251dccdc5368b712/clippy_lints/src/methods/unnecessary_to_owned.rs#L420-L428 Like `@mikerite's` original solution, this solution turns ICEs into false negatives. changelog: fix `unnecessary_to_owned` false positive involving `Try::branch`
2 parents 4c123a0 + 2701a40 commit cb8df45

File tree

3 files changed

+42
-4
lines changed

3 files changed

+42
-4
lines changed

clippy_lints/src/methods/unnecessary_to_owned.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -386,14 +386,12 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty<
386386
Node::Expr(parent_expr) => {
387387
if let Some((callee_def_id, call_substs, recv, call_args)) = get_callee_substs_and_args(cx, parent_expr)
388388
{
389-
if Some(callee_def_id) == cx.tcx.lang_items().into_future_fn() {
390-
return false;
391-
}
392-
393389
let fn_sig = cx.tcx.fn_sig(callee_def_id).skip_binder();
394390
if let Some(arg_index) = recv.into_iter().chain(call_args).position(|arg| arg.hir_id == expr.hir_id)
395391
&& let Some(param_ty) = fn_sig.inputs().get(arg_index)
396392
&& let ty::Param(ParamTy { index: param_index , ..}) = param_ty.kind()
393+
// https://github.com/rust-lang/rust-clippy/issues/9504 and https://github.com/rust-lang/rust-clippy/issues/10021
394+
&& (*param_index as usize) < call_substs.len()
397395
{
398396
if fn_sig
399397
.inputs()

tests/ui/unnecessary_to_owned.fixed

+20
Original file line numberDiff line numberDiff line change
@@ -454,3 +454,23 @@ mod issue_9771b {
454454
Key(v.to_vec())
455455
}
456456
}
457+
458+
// This is a watered down version of the code in: https://github.com/oxigraph/rio
459+
// The ICE is triggered by the call to `to_owned` on this line:
460+
// https://github.com/oxigraph/rio/blob/66635b9ff8e5423e58932353fa40d6e64e4820f7/testsuite/src/parser_evaluator.rs#L116
461+
mod issue_10021 {
462+
#![allow(unused)]
463+
464+
pub struct Iri<T>(T);
465+
466+
impl<T: AsRef<str>> Iri<T> {
467+
pub fn parse(iri: T) -> Result<Self, ()> {
468+
unimplemented!()
469+
}
470+
}
471+
472+
pub fn parse_w3c_rdf_test_file(url: &str) -> Result<(), ()> {
473+
let base_iri = Iri::parse(url.to_owned())?;
474+
Ok(())
475+
}
476+
}

tests/ui/unnecessary_to_owned.rs

+20
Original file line numberDiff line numberDiff line change
@@ -454,3 +454,23 @@ mod issue_9771b {
454454
Key(v.to_vec())
455455
}
456456
}
457+
458+
// This is a watered down version of the code in: https://github.com/oxigraph/rio
459+
// The ICE is triggered by the call to `to_owned` on this line:
460+
// https://github.com/oxigraph/rio/blob/66635b9ff8e5423e58932353fa40d6e64e4820f7/testsuite/src/parser_evaluator.rs#L116
461+
mod issue_10021 {
462+
#![allow(unused)]
463+
464+
pub struct Iri<T>(T);
465+
466+
impl<T: AsRef<str>> Iri<T> {
467+
pub fn parse(iri: T) -> Result<Self, ()> {
468+
unimplemented!()
469+
}
470+
}
471+
472+
pub fn parse_w3c_rdf_test_file(url: &str) -> Result<(), ()> {
473+
let base_iri = Iri::parse(url.to_owned())?;
474+
Ok(())
475+
}
476+
}

0 commit comments

Comments
 (0)