From 9a3a89ea6168c9ee4fdf88d4d895e8df047d782e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 5 Apr 2022 14:32:56 -0700 Subject: [PATCH] Revert "Fix late-bound ICE in unsized return suggestion" This reverts commit b899251f2dfe3a9849f844418e0d11e2073c2423. --- .../src/traits/error_reporting/suggestions.rs | 134 +++++++----------- ...n-trait-return-should-be-impl-trait.stderr | 30 ++-- ...type-err-cause-on-impl-trait-return.stderr | 49 +++---- src/test/ui/issues/issue-18107.stderr | 2 +- src/test/ui/unsized/box-instead-of-dyn-fn.rs | 15 -- .../ui/unsized/box-instead-of-dyn-fn.stderr | 39 ----- src/test/ui/unsized/issue-91801.rs | 19 --- src/test/ui/unsized/issue-91801.stderr | 15 -- src/test/ui/unsized/issue-91803.rs | 8 -- src/test/ui/unsized/issue-91803.stderr | 15 -- 10 files changed, 84 insertions(+), 242 deletions(-) delete mode 100644 src/test/ui/unsized/box-instead-of-dyn-fn.rs delete mode 100644 src/test/ui/unsized/box-instead-of-dyn-fn.stderr delete mode 100644 src/test/ui/unsized/issue-91801.rs delete mode 100644 src/test/ui/unsized/issue-91801.stderr delete mode 100644 src/test/ui/unsized/issue-91803.rs delete mode 100644 src/test/ui/unsized/issue-91803.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 7c3f306717a69..e430abeaa8015 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1129,8 +1129,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } let hir = self.tcx.hir(); - let fn_hir_id = hir.get_parent_node(obligation.cause.body_id); - let node = hir.find(fn_hir_id); + let parent_node = hir.get_parent_node(obligation.cause.body_id); + let node = hir.find(parent_node); let Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, _, body_id), .. @@ -1168,17 +1168,16 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { visitor.visit_body(&body); let typeck_results = self.in_progress_typeck_results.map(|t| t.borrow()).unwrap(); - let Some(liberated_sig) = typeck_results.liberated_fn_sigs().get(fn_hir_id) else { return false; }; - let ret_types = visitor + let mut ret_types = visitor .returns .iter() - .filter_map(|expr| Some((expr.span, typeck_results.node_type_opt(expr.hir_id)?))) - .map(|(expr_span, ty)| (expr_span, self.resolve_vars_if_possible(ty))); + .filter_map(|expr| typeck_results.node_type_opt(expr.hir_id)) + .map(|ty| self.resolve_vars_if_possible(ty)); let (last_ty, all_returns_have_same_type, only_never_return) = ret_types.clone().fold( (None, true, true), |(last_ty, mut same, only_never_return): (std::option::Option>, bool, bool), - (_, ty)| { + ty| { let ty = self.resolve_vars_if_possible(ty); same &= !matches!(ty.kind(), ty::Error(_)) @@ -1199,60 +1198,39 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { (Some(ty), same, only_never_return && matches!(ty.kind(), ty::Never)) }, ); - let mut spans_and_needs_box = vec![]; - - match liberated_sig.output().kind() { - ty::Dynamic(predicates, _) => { - let cause = ObligationCause::misc(ret_ty.span, fn_hir_id); - let param_env = ty::ParamEnv::empty(); - - if !only_never_return { - for (expr_span, return_ty) in ret_types { - let self_ty_satisfies_dyn_predicates = |self_ty| { - predicates.iter().all(|predicate| { - let pred = predicate.with_self_ty(self.tcx, self_ty); - let obl = Obligation::new(cause.clone(), param_env, pred); - self.predicate_may_hold(&obl) + let all_returns_conform_to_trait = + if let Some(ty_ret_ty) = typeck_results.node_type_opt(ret_ty.hir_id) { + match ty_ret_ty.kind() { + ty::Dynamic(predicates, _) => { + let cause = ObligationCause::misc(ret_ty.span, ret_ty.hir_id); + let param_env = ty::ParamEnv::empty(); + only_never_return + || ret_types.all(|returned_ty| { + predicates.iter().all(|predicate| { + let pred = predicate.with_self_ty(self.tcx, returned_ty); + let obl = Obligation::new(cause.clone(), param_env, pred); + self.predicate_may_hold(&obl) + }) }) - }; - - if let ty::Adt(def, substs) = return_ty.kind() - && def.is_box() - && self_ty_satisfies_dyn_predicates(substs.type_at(0)) - { - spans_and_needs_box.push((expr_span, false)); - } else if self_ty_satisfies_dyn_predicates(return_ty) { - spans_and_needs_box.push((expr_span, true)); - } else { - return false; - } } + _ => false, } - } - _ => return false, - }; + } else { + true + }; let sm = self.tcx.sess.source_map(); - if !ret_ty.span.overlaps(span) { + let (true, hir::TyKind::TraitObject(..), Ok(snippet), true) = ( + // Verify that we're dealing with a return `dyn Trait` + ret_ty.span.overlaps(span), + &ret_ty.kind, + sm.span_to_snippet(ret_ty.span), + // If any of the return types does not conform to the trait, then we can't + // suggest `impl Trait` nor trait objects: it is a type mismatch error. + all_returns_conform_to_trait, + ) else { return false; - } - let snippet = if let hir::TyKind::TraitObject(..) = ret_ty.kind { - if let Ok(snippet) = sm.span_to_snippet(ret_ty.span) { - snippet - } else { - return false; - } - } else { - // Substitute the type, so we can print a fixup given `type Alias = dyn Trait` - let name = liberated_sig.output().to_string(); - let name = - name.strip_prefix('(').and_then(|name| name.strip_suffix(')')).unwrap_or(&name); - if !name.starts_with("dyn ") { - return false; - } - name.to_owned() }; - err.code(error_code!(E0746)); err.set_primary_message("return type cannot have an unboxed trait object"); err.children.clear(); @@ -1262,7 +1240,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let trait_obj_msg = "for information on trait objects, see \ "; - let has_dyn = snippet.split_whitespace().next().map_or(false, |s| s == "dyn"); let trait_obj = if has_dyn { &snippet[4..] } else { &snippet }; if only_never_return { @@ -1290,25 +1267,26 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } else { if is_object_safe { // Suggest `-> Box` and `Box::new(returned_value)`. - err.multipart_suggestion( - "return a boxed trait object instead", - vec![ - (ret_ty.span.shrink_to_lo(), "Box<".to_string()), - (span.shrink_to_hi(), ">".to_string()), - ], - Applicability::MaybeIncorrect, - ); - for (span, needs_box) in spans_and_needs_box { - if needs_box { - err.multipart_suggestion( - "... and box this value", - vec![ - (span.shrink_to_lo(), "Box::new(".to_string()), - (span.shrink_to_hi(), ")".to_string()), - ], - Applicability::MaybeIncorrect, - ); - } + // Get all the return values and collect their span and suggestion. + let mut suggestions: Vec<_> = visitor + .returns + .iter() + .flat_map(|expr| { + [ + (expr.span.shrink_to_lo(), "Box::new(".to_string()), + (expr.span.shrink_to_hi(), ")".to_string()), + ] + .into_iter() + }) + .collect(); + if !suggestions.is_empty() { + // Add the suggestion for the return type. + suggestions.push((ret_ty.span, format!("Box", trait_obj))); + err.multipart_suggestion( + "return a boxed trait object instead", + suggestions, + Applicability::MaybeIncorrect, + ); } } else { // This is currently not possible to trigger because E0038 takes precedence, but @@ -2845,15 +2823,13 @@ fn suggest_trait_object_return_type_alternatives( Applicability::MaybeIncorrect, ); if is_object_safe { - err.multipart_suggestion( + err.span_suggestion( + ret_ty, &format!( "use a boxed trait object if all return paths implement trait `{}`", trait_obj, ), - vec![ - (ret_ty.shrink_to_lo(), "Box<".to_string()), - (ret_ty.shrink_to_hi(), ">".to_string()), - ], + format!("Box", trait_obj), Applicability::MaybeIncorrect, ); } diff --git a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr index f90399b6b9458..0d4f82bfc153f 100644 --- a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr +++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr @@ -81,7 +81,7 @@ LL | fn bak() -> impl Trait { unimplemented!() } help: use a boxed trait object if all return paths implement trait `Trait` | LL | fn bak() -> Box { unimplemented!() } - | ++++ + + | ~~~~~~~~~~~~~~ error[E0746]: return type cannot have an unboxed trait object --> $DIR/dyn-trait-return-should-be-impl-trait.rs:19:13 @@ -95,16 +95,12 @@ LL | fn bal() -> dyn Trait { = note: you can create a new `enum` with a variant for each returned type help: return a boxed trait object instead | -LL | fn bal() -> Box { - | ++++ + -help: ... and box this value - | -LL | return Box::new(Struct); - | +++++++++ + -help: ... and box this value +LL ~ fn bal() -> Box { +LL | if true { +LL ~ return Box::new(Struct); +LL | } +LL ~ Box::new(42) | -LL | Box::new(42) - | +++++++++ + error[E0308]: `if` and `else` have incompatible types --> $DIR/dyn-trait-return-should-be-impl-trait.rs:29:9 @@ -130,16 +126,12 @@ LL | fn bax() -> dyn Trait { = note: you can create a new `enum` with a variant for each returned type help: return a boxed trait object instead | -LL | fn bax() -> Box { - | ++++ + -help: ... and box this value - | -LL | Box::new(Struct) - | +++++++++ + -help: ... and box this value +LL ~ fn bax() -> Box { +LL | if true { +LL ~ Box::new(Struct) +LL | } else { +LL ~ Box::new(42) | -LL | Box::new(42) - | +++++++++ + error[E0308]: mismatched types --> $DIR/dyn-trait-return-should-be-impl-trait.rs:34:16 diff --git a/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr b/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr index 10510c1754eda..0c595f441ba8e 100644 --- a/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr +++ b/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr @@ -103,16 +103,13 @@ LL | fn hat() -> dyn std::fmt::Display { = note: you can create a new `enum` with a variant for each returned type help: return a boxed trait object instead | -LL | fn hat() -> Box { - | ++++ + -help: ... and box this value - | -LL | return Box::new(0i32); - | +++++++++ + -help: ... and box this value - | -LL | Box::new(1u32) - | +++++++++ + +LL ~ fn hat() -> Box { +LL | match 13 { +LL | 0 => { +LL ~ return Box::new(0i32); +LL | } +LL | _ => { + ... error[E0308]: `match` arms have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:80:14 @@ -138,20 +135,12 @@ LL | fn pug() -> dyn std::fmt::Display { = note: you can create a new `enum` with a variant for each returned type help: return a boxed trait object instead | -LL | fn pug() -> Box { - | ++++ + -help: ... and box this value - | -LL | 0 => Box::new(0i32), - | +++++++++ + -help: ... and box this value +LL ~ fn pug() -> Box { +LL | match 13 { +LL ~ 0 => Box::new(0i32), +LL ~ 1 => Box::new(1u32), +LL ~ _ => Box::new(2u32), | -LL | 1 => Box::new(1u32), - | +++++++++ + -help: ... and box this value - | -LL | _ => Box::new(2u32), - | +++++++++ + error[E0308]: `if` and `else` have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:89:9 @@ -177,16 +166,12 @@ LL | fn man() -> dyn std::fmt::Display { = note: you can create a new `enum` with a variant for each returned type help: return a boxed trait object instead | -LL | fn man() -> Box { - | ++++ + -help: ... and box this value - | -LL | Box::new(0i32) - | +++++++++ + -help: ... and box this value +LL ~ fn man() -> Box { +LL | if false { +LL ~ Box::new(0i32) +LL | } else { +LL ~ Box::new(1u32) | -LL | Box::new(1u32) - | +++++++++ + error: aborting due to 14 previous errors diff --git a/src/test/ui/issues/issue-18107.stderr b/src/test/ui/issues/issue-18107.stderr index 28478457b296d..1eb6822b8a11a 100644 --- a/src/test/ui/issues/issue-18107.stderr +++ b/src/test/ui/issues/issue-18107.stderr @@ -15,7 +15,7 @@ LL | impl AbstractRenderer help: use a boxed trait object if all return paths implement trait `AbstractRenderer` | LL | Box - | ++++ + + | error: aborting due to previous error diff --git a/src/test/ui/unsized/box-instead-of-dyn-fn.rs b/src/test/ui/unsized/box-instead-of-dyn-fn.rs deleted file mode 100644 index 2fa741bc1c50b..0000000000000 --- a/src/test/ui/unsized/box-instead-of-dyn-fn.rs +++ /dev/null @@ -1,15 +0,0 @@ -use std::fmt::Debug; - -// Test to suggest boxing the return type, and the closure branch of the `if` - -fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> dyn Fn() + 'a { - //~^ ERROR return type cannot have an unboxed trait object - if a % 2 == 0 { - move || println!("{a}") - } else { - Box::new(move || println!("{}", b)) - //~^ ERROR `if` and `else` have incompatible types - } -} - -fn main() {} diff --git a/src/test/ui/unsized/box-instead-of-dyn-fn.stderr b/src/test/ui/unsized/box-instead-of-dyn-fn.stderr deleted file mode 100644 index 80f61cb3eae11..0000000000000 --- a/src/test/ui/unsized/box-instead-of-dyn-fn.stderr +++ /dev/null @@ -1,39 +0,0 @@ -error[E0308]: `if` and `else` have incompatible types - --> $DIR/box-instead-of-dyn-fn.rs:10:9 - | -LL | / if a % 2 == 0 { -LL | | move || println!("{a}") - | | ----------------------- expected because of this -LL | | } else { -LL | | Box::new(move || println!("{}", b)) - | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected closure, found struct `Box` -LL | | -LL | | } - | |_____- `if` and `else` have incompatible types - | - = note: expected type `[closure@$DIR/box-instead-of-dyn-fn.rs:8:9: 8:32]` - found struct `Box<[closure@$DIR/box-instead-of-dyn-fn.rs:10:18: 10:43]>` - -error[E0746]: return type cannot have an unboxed trait object - --> $DIR/box-instead-of-dyn-fn.rs:5:56 - | -LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> dyn Fn() + 'a { - | ^^^^^^^^^^^^^ doesn't have a size known at compile-time - | - = note: for information on trait objects, see - = note: if all the returned values were of the same type you could use `impl Fn() + 'a` as the return type - = note: for information on `impl Trait`, see - = note: you can create a new `enum` with a variant for each returned type -help: return a boxed trait object instead - | -LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> Box { - | ++++ + -help: ... and box this value - | -LL | Box::new(move || println!("{a}")) - | +++++++++ + - -error: aborting due to 2 previous errors - -Some errors have detailed explanations: E0308, E0746. -For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/unsized/issue-91801.rs b/src/test/ui/unsized/issue-91801.rs deleted file mode 100644 index 096b1a93574fc..0000000000000 --- a/src/test/ui/unsized/issue-91801.rs +++ /dev/null @@ -1,19 +0,0 @@ -pub struct Something; - -type Validator<'a> = dyn 'a + Send + Sync + Fn(&'a Something) -> Result<(), ()>; - -pub static ALL_VALIDATORS: &[(&'static str, &'static Validator)] = - &[("validate that credits and debits balance", &validate_something)]; - -fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Validator<'a> { - //~^ ERROR return type cannot have an unboxed trait object - return Box::new(move |something: &'_ Something| -> Result<(), ()> { - first(something).or_else(|_| second(something)) - }); -} - -fn validate_something(_: &Something) -> Result<(), ()> { - Ok(()) -} - -fn main() {} diff --git a/src/test/ui/unsized/issue-91801.stderr b/src/test/ui/unsized/issue-91801.stderr deleted file mode 100644 index e854514958629..0000000000000 --- a/src/test/ui/unsized/issue-91801.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0746]: return type cannot have an unboxed trait object - --> $DIR/issue-91801.rs:8:77 - | -LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Validator<'a> { - | ^^^^^^^^^^^^^ doesn't have a size known at compile-time - | - = note: for information on `impl Trait`, see -help: use `impl Fn(&'a Something) -> Result<(), ()> + Send + Sync + 'a` as the return type, as all return paths are of type `Box<[closure@$DIR/issue-91801.rs:10:21: 12:6]>`, which implements `Fn(&'a Something) -> Result<(), ()> + Send + Sync + 'a` - | -LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> impl Fn(&'a Something) -> Result<(), ()> + Send + Sync + 'a { - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0746`. diff --git a/src/test/ui/unsized/issue-91803.rs b/src/test/ui/unsized/issue-91803.rs deleted file mode 100644 index c74897cc4bc50..0000000000000 --- a/src/test/ui/unsized/issue-91803.rs +++ /dev/null @@ -1,8 +0,0 @@ -trait Foo<'a> {} - -fn or<'a>(first: &'static dyn Foo<'a>) -> dyn Foo<'a> { - //~^ ERROR return type cannot have an unboxed trait object - return Box::new(panic!()); -} - -fn main() {} diff --git a/src/test/ui/unsized/issue-91803.stderr b/src/test/ui/unsized/issue-91803.stderr deleted file mode 100644 index 2dad9e8929421..0000000000000 --- a/src/test/ui/unsized/issue-91803.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0746]: return type cannot have an unboxed trait object - --> $DIR/issue-91803.rs:3:43 - | -LL | fn or<'a>(first: &'static dyn Foo<'a>) -> dyn Foo<'a> { - | ^^^^^^^^^^^ doesn't have a size known at compile-time - | - = note: for information on `impl Trait`, see -help: use `impl Foo<'a>` as the return type, as all return paths are of type `Box<_>`, which implements `Foo<'a>` - | -LL | fn or<'a>(first: &'static dyn Foo<'a>) -> impl Foo<'a> { - | ~~~~~~~~~~~~ - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0746`.