From b899251f2dfe3a9849f844418e0d11e2073c2423 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 2 Apr 2022 14:22:35 -0700 Subject: [PATCH] Fix late-bound ICE in unsized return suggestion --- .../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, 242 insertions(+), 84 deletions(-) create mode 100644 src/test/ui/unsized/box-instead-of-dyn-fn.rs create mode 100644 src/test/ui/unsized/box-instead-of-dyn-fn.stderr create mode 100644 src/test/ui/unsized/issue-91801.rs create mode 100644 src/test/ui/unsized/issue-91801.stderr create mode 100644 src/test/ui/unsized/issue-91803.rs create 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 b49a5f6578f75..ac17b0d68657e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1045,8 +1045,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } let hir = self.tcx.hir(); - let parent_node = hir.get_parent_node(obligation.cause.body_id); - let node = hir.find(parent_node); + let fn_hir_id = hir.get_parent_node(obligation.cause.body_id); + let node = hir.find(fn_hir_id); let Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, _, body_id), .. @@ -1084,16 +1084,17 @@ 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 mut ret_types = visitor + let ret_types = visitor .returns .iter() - .filter_map(|expr| typeck_results.node_type_opt(expr.hir_id)) - .map(|ty| self.resolve_vars_if_possible(ty)); + .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))); 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(_)) @@ -1114,39 +1115,60 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { (Some(ty), same, only_never_return && matches!(ty.kind(), ty::Never)) }, ); - 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) - }) + 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) }) + }; + + 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, } - } else { - true - }; + } + _ => return false, + }; let sm = self.tcx.sess.source_map(); - 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 { + if !ret_ty.span.overlaps(span) { 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(); @@ -1156,6 +1178,7 @@ 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 { @@ -1183,26 +1206,25 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } else { if is_object_safe { // Suggest `-> Box` and `Box::new(returned_value)`. - // 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, - ); + 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, + ); + } } } else { // This is currently not possible to trigger because E0038 takes precedence, but @@ -2677,13 +2699,15 @@ fn suggest_trait_object_return_type_alternatives( Applicability::MaybeIncorrect, ); if is_object_safe { - err.span_suggestion( - ret_ty, + err.multipart_suggestion( &format!( "use a boxed trait object if all return paths implement trait `{}`", trait_obj, ), - format!("Box", trait_obj), + vec![ + (ret_ty.shrink_to_lo(), "Box<".to_string()), + (ret_ty.shrink_to_hi(), ">".to_string()), + ], 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 0d4f82bfc153f..f90399b6b9458 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,12 +95,16 @@ 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 { -LL | if true { -LL ~ return Box::new(Struct); -LL | } -LL ~ Box::new(42) +LL | fn bal() -> Box { + | ++++ + +help: ... and box this value + | +LL | return Box::new(Struct); + | +++++++++ + +help: ... and box this value | +LL | Box::new(42) + | +++++++++ + error[E0308]: `if` and `else` have incompatible types --> $DIR/dyn-trait-return-should-be-impl-trait.rs:29:9 @@ -126,12 +130,16 @@ 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 { -LL | if true { -LL ~ Box::new(Struct) -LL | } else { -LL ~ Box::new(42) +LL | fn bax() -> Box { + | ++++ + +help: ... and box this value + | +LL | Box::new(Struct) + | +++++++++ + +help: ... and box this value | +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 0c595f441ba8e..10510c1754eda 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,13 +103,16 @@ 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 { -LL | match 13 { -LL | 0 => { -LL ~ return Box::new(0i32); -LL | } -LL | _ => { - ... +LL | fn hat() -> Box { + | ++++ + +help: ... and box this value + | +LL | return Box::new(0i32); + | +++++++++ + +help: ... and box this value + | +LL | Box::new(1u32) + | +++++++++ + error[E0308]: `match` arms have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:80:14 @@ -135,12 +138,20 @@ 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 { -LL | match 13 { -LL ~ 0 => Box::new(0i32), -LL ~ 1 => Box::new(1u32), -LL ~ _ => Box::new(2u32), +LL | fn pug() -> Box { + | ++++ + +help: ... and box this value + | +LL | 0 => Box::new(0i32), + | +++++++++ + +help: ... and box this value | +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 @@ -166,12 +177,16 @@ 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 { -LL | if false { -LL ~ Box::new(0i32) -LL | } else { -LL ~ Box::new(1u32) +LL | fn man() -> Box { + | ++++ + +help: ... and box this value + | +LL | Box::new(0i32) + | +++++++++ + +help: ... and box this value | +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 1eb6822b8a11a..28478457b296d 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 new file mode 100644 index 0000000000000..2fa741bc1c50b --- /dev/null +++ b/src/test/ui/unsized/box-instead-of-dyn-fn.rs @@ -0,0 +1,15 @@ +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 new file mode 100644 index 0000000000000..80f61cb3eae11 --- /dev/null +++ b/src/test/ui/unsized/box-instead-of-dyn-fn.stderr @@ -0,0 +1,39 @@ +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 new file mode 100644 index 0000000000000..096b1a93574fc --- /dev/null +++ b/src/test/ui/unsized/issue-91801.rs @@ -0,0 +1,19 @@ +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 new file mode 100644 index 0000000000000..e854514958629 --- /dev/null +++ b/src/test/ui/unsized/issue-91801.stderr @@ -0,0 +1,15 @@ +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 new file mode 100644 index 0000000000000..c74897cc4bc50 --- /dev/null +++ b/src/test/ui/unsized/issue-91803.rs @@ -0,0 +1,8 @@ +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 new file mode 100644 index 0000000000000..2dad9e8929421 --- /dev/null +++ b/src/test/ui/unsized/issue-91803.stderr @@ -0,0 +1,15 @@ +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`.