about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-12-04 01:58:43 +0000
committerbors <bors@rust-lang.org>2022-12-04 01:58:43 +0000
commitcb8df45333786fc89cd566bbaa71447bb4b105bd (patch)
tree129c61f85a65dedda7e80e19e38ba438867a8586
parent4c123a06ba3c2ec17d3a4dfa251dccdc5368b712 (diff)
parent2701a4076f3d18edc06781d7a9b3091ee5dbd05f (diff)
downloadrust-cb8df45333786fc89cd566bbaa71447bb4b105bd.tar.gz
rust-cb8df45333786fc89cd566bbaa71447bb4b105bd.zip
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`
-rw-r--r--clippy_lints/src/methods/unnecessary_to_owned.rs6
-rw-r--r--tests/ui/unnecessary_to_owned.fixed20
-rw-r--r--tests/ui/unnecessary_to_owned.rs20
3 files changed, 42 insertions, 4 deletions
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
index 17b0507682a..9263f051972 100644
--- a/clippy_lints/src/methods/unnecessary_to_owned.rs
+++ b/clippy_lints/src/methods/unnecessary_to_owned.rs
@@ -386,14 +386,12 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty<
             Node::Expr(parent_expr) => {
                 if let Some((callee_def_id, call_substs, recv, call_args)) = get_callee_substs_and_args(cx, parent_expr)
                 {
-                    if Some(callee_def_id) == cx.tcx.lang_items().into_future_fn() {
-                        return false;
-                    }
-
                     let fn_sig = cx.tcx.fn_sig(callee_def_id).skip_binder();
                     if let Some(arg_index) = recv.into_iter().chain(call_args).position(|arg| arg.hir_id == expr.hir_id)
                         && let Some(param_ty) = fn_sig.inputs().get(arg_index)
                         && let ty::Param(ParamTy { index: param_index , ..}) = param_ty.kind()
+                        // https://github.com/rust-lang/rust-clippy/issues/9504 and https://github.com/rust-lang/rust-clippy/issues/10021
+                        && (*param_index as usize) < call_substs.len()
                     {
                         if fn_sig
                             .inputs()
diff --git a/tests/ui/unnecessary_to_owned.fixed b/tests/ui/unnecessary_to_owned.fixed
index ddeda795f81..345f6d604c4 100644
--- a/tests/ui/unnecessary_to_owned.fixed
+++ b/tests/ui/unnecessary_to_owned.fixed
@@ -454,3 +454,23 @@ mod issue_9771b {
         Key(v.to_vec())
     }
 }
+
+// This is a watered down version of the code in: https://github.com/oxigraph/rio
+// The ICE is triggered by the call to `to_owned` on this line:
+// https://github.com/oxigraph/rio/blob/66635b9ff8e5423e58932353fa40d6e64e4820f7/testsuite/src/parser_evaluator.rs#L116
+mod issue_10021 {
+    #![allow(unused)]
+
+    pub struct Iri<T>(T);
+
+    impl<T: AsRef<str>> Iri<T> {
+        pub fn parse(iri: T) -> Result<Self, ()> {
+            unimplemented!()
+        }
+    }
+
+    pub fn parse_w3c_rdf_test_file(url: &str) -> Result<(), ()> {
+        let base_iri = Iri::parse(url.to_owned())?;
+        Ok(())
+    }
+}
diff --git a/tests/ui/unnecessary_to_owned.rs b/tests/ui/unnecessary_to_owned.rs
index 95d2576733c..7eb53df39e5 100644
--- a/tests/ui/unnecessary_to_owned.rs
+++ b/tests/ui/unnecessary_to_owned.rs
@@ -454,3 +454,23 @@ mod issue_9771b {
         Key(v.to_vec())
     }
 }
+
+// This is a watered down version of the code in: https://github.com/oxigraph/rio
+// The ICE is triggered by the call to `to_owned` on this line:
+// https://github.com/oxigraph/rio/blob/66635b9ff8e5423e58932353fa40d6e64e4820f7/testsuite/src/parser_evaluator.rs#L116
+mod issue_10021 {
+    #![allow(unused)]
+
+    pub struct Iri<T>(T);
+
+    impl<T: AsRef<str>> Iri<T> {
+        pub fn parse(iri: T) -> Result<Self, ()> {
+            unimplemented!()
+        }
+    }
+
+    pub fn parse_w3c_rdf_test_file(url: &str) -> Result<(), ()> {
+        let base_iri = Iri::parse(url.to_owned())?;
+        Ok(())
+    }
+}