From 390ef9ba0297ae5ba5aacdf0be0d0c47be8d166a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 22 Jan 2024 20:51:19 +0000 Subject: [PATCH 1/3] Fix incorrect suggestion for boxing tail expression in blocks --- compiler/rustc_hir_typeck/src/demand.rs | 8 ++++++- ...uggest-box-on-divergent-if-else-arms.fixed | 14 +++++++++++ .../suggest-box-on-divergent-if-else-arms.rs | 14 +++++++++++ ...ggest-box-on-divergent-if-else-arms.stderr | 24 +++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed create mode 100644 tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs create mode 100644 tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 9850892bd36af..b6dfc34d3ac19 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -44,7 +44,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { || self.suggest_non_zero_new_unwrap(err, expr, expected, expr_ty) || self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) || self.suggest_no_capture_closure(err, expected, expr_ty) - || self.suggest_boxing_when_appropriate(err, expr.span, expr.hir_id, expected, expr_ty) + || self.suggest_boxing_when_appropriate( + err, + expr.peel_blocks().span, + expr.hir_id, + expected, + expr_ty, + ) || self.suggest_block_to_brackets_peeling_refs(err, expr, expr_ty, expected) || self.suggest_copied_cloned_or_as_ref(err, expr, expr_ty, expected) || self.suggest_clone_for_ref(err, expr, expr_ty, expected) diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed new file mode 100644 index 0000000000000..d8031083f6cf8 --- /dev/null +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed @@ -0,0 +1,14 @@ +// run-rustfix +trait Trait {} +struct Struct; +impl Trait for Struct {} +fn foo() -> Box { + Box::new(Struct) +} +fn main() { + let _ = if true { + foo() + } else { + Box::new(Struct) //~ ERROR E0308 + }; +} diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs new file mode 100644 index 0000000000000..53e718a56635b --- /dev/null +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs @@ -0,0 +1,14 @@ +// run-rustfix +trait Trait {} +struct Struct; +impl Trait for Struct {} +fn foo() -> Box { + Box::new(Struct) +} +fn main() { + let _ = if true { + foo() + } else { + Struct //~ ERROR E0308 + }; +} diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr new file mode 100644 index 0000000000000..183f48ca1fb78 --- /dev/null +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr @@ -0,0 +1,24 @@ +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:12:9 + | +LL | let _ = if true { + | _____________- +LL | | foo() + | | ----- expected because of this +LL | | } else { +LL | | Struct + | | ^^^^^^ expected `Box`, found `Struct` +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected struct `Box` + found struct `Struct` + = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html +help: store this in the heap by calling `Box::new` + | +LL | Box::new(Struct) + | +++++++++ + + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. From ac56a2b564a3e15b8377e72294a3d565a1c8c659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 22 Jan 2024 20:53:41 +0000 Subject: [PATCH 2/3] Suggest boxing if then expr if that solves divergent arms When encountering ```rust let _ = if true { Struct } else { foo() // -> Box }; ``` if `Struct` implements `Trait`, suggest boxing the then arm tail expression. Part of #102629. --- .../infer/error_reporting/note_and_explain.rs | 32 +++++++++++++++++++ ...uggest-box-on-divergent-if-else-arms.fixed | 5 +++ .../suggest-box-on-divergent-if-else-arms.rs | 5 +++ ...ggest-box-on-divergent-if-else-arms.stderr | 22 ++++++++++++- 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs index afb3c5c1e5656..a0dfaf33a6e85 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs @@ -330,6 +330,38 @@ impl Trait for X { ); } } + (ty::Adt(_, _), ty::Adt(def, args)) + if let ObligationCauseCode::IfExpression(cause) = cause.code() + && let hir::Node::Block(blk) = self.tcx.hir_node(cause.then_id) + && let Some(then) = blk.expr + && def.is_box() + && let boxed_ty = args.type_at(0) + && let ty::Dynamic(t, _, _) = boxed_ty.kind() + && let Some(def_id) = t.principal_def_id() + && let mut impl_def_ids = vec![] + && let _ = + tcx.for_each_relevant_impl(def_id, values.expected, |did| { + impl_def_ids.push(did) + }) + && let [_] = &impl_def_ids[..] => + { + // We have divergent if/else arms where the expected value is a type that + // implements the trait of the found boxed trait object. + diag.multipart_suggestion( + format!( + "`{}` implements `{}` so you can box it to coerce to the trait \ + object `{}`", + values.expected, + tcx.item_name(def_id), + values.found, + ), + vec![ + (then.span.shrink_to_lo(), "Box::new(".to_string()), + (then.span.shrink_to_hi(), ")".to_string()), + ], + MachineApplicable, + ); + } _ => {} } debug!( diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed index d8031083f6cf8..dfdf6b24bf7e0 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed @@ -6,6 +6,11 @@ fn foo() -> Box { Box::new(Struct) } fn main() { + let _ = if true { + Box::new(Struct) + } else { + foo() //~ ERROR E0308 + }; let _ = if true { foo() } else { diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs index 53e718a56635b..e138ad9b33647 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs @@ -6,6 +6,11 @@ fn foo() -> Box { Box::new(Struct) } fn main() { + let _ = if true { + Struct + } else { + foo() //~ ERROR E0308 + }; let _ = if true { foo() } else { diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr index 183f48ca1fb78..98c9d880f97aa 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr @@ -3,6 +3,26 @@ error[E0308]: `if` and `else` have incompatible types | LL | let _ = if true { | _____________- +LL | | Struct + | | ------ expected because of this +LL | | } else { +LL | | foo() + | | ^^^^^ expected `Struct`, found `Box` +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected struct `Struct` + found struct `Box` +help: `Struct` implements `Trait` so you can box it to coerce to the trait object `Box` + | +LL | Box::new(Struct) + | +++++++++ + + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:17:9 + | +LL | let _ = if true { + | _____________- LL | | foo() | | ----- expected because of this LL | | } else { @@ -19,6 +39,6 @@ help: store this in the heap by calling `Box::new` LL | Box::new(Struct) | +++++++++ + -error: aborting due to 1 previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0308`. From 34f4f3da4f3e2ac8fa086c4d3a3c89d49d23a263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jan 2024 04:42:26 +0000 Subject: [PATCH 3/3] Suggest boxing both arms of if expr if that solves divergent arms involving `impl Trait` When encountering the following ```rust // run-rustfix trait Trait {} struct Struct; impl Trait for Struct {} fn foo() -> Box { Box::new(Struct) } fn bar() -> impl Trait { Struct } fn main() { let _ = if true { Struct } else { foo() //~ ERROR E0308 }; let _ = if true { foo() } else { Struct //~ ERROR E0308 }; let _ = if true { Struct } else { bar() // impl Trait }; let _ = if true { bar() // impl Trait } else { Struct }; } ``` suggest boxing both arms ```rust let _ = if true { Box::new(Struct) as Box } else { Box::new(bar()) }; let _ = if true { Box::new(bar()) as Box } else { Box::new(Struct) }; ``` --- .../infer/error_reporting/note_and_explain.rs | 86 +++++++++++++++---- ...uggest-box-on-divergent-if-else-arms.fixed | 13 +++ .../suggest-box-on-divergent-if-else-arms.rs | 13 +++ ...ggest-box-on-divergent-if-else-arms.stderr | 56 +++++++++++- 4 files changed, 149 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs index a0dfaf33a6e85..01cd3c57925b3 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs @@ -294,8 +294,9 @@ impl Trait for X { ); } } - (ty::Alias(ty::Opaque, alias), _) | (_, ty::Alias(ty::Opaque, alias)) - if alias.def_id.is_local() + (_, ty::Alias(ty::Opaque, opaque_ty)) + | (ty::Alias(ty::Opaque, opaque_ty), _) => { + if opaque_ty.def_id.is_local() && matches!( tcx.def_kind(body_owner_def_id), DefKind::Fn @@ -303,21 +304,74 @@ impl Trait for X { | DefKind::Const | DefKind::AssocFn | DefKind::AssocConst - ) => - { - if tcx.is_type_alias_impl_trait(alias.def_id) { - if !tcx + ) + && tcx.is_type_alias_impl_trait(opaque_ty.def_id) + && !tcx .opaque_types_defined_by(body_owner_def_id.expect_local()) - .contains(&alias.def_id.expect_local()) - { - let sp = tcx - .def_ident_span(body_owner_def_id) - .unwrap_or_else(|| tcx.def_span(body_owner_def_id)); - diag.span_note( - sp, - "\ - this item must have the opaque type in its signature \ - in order to be able to register hidden types", + .contains(&opaque_ty.def_id.expect_local()) + { + let sp = tcx + .def_ident_span(body_owner_def_id) + .unwrap_or_else(|| tcx.def_span(body_owner_def_id)); + diag.span_note( + sp, + "this item must have the opaque type in its signature in order to \ + be able to register hidden types", + ); + } + // If two if arms can be coerced to a trait object, provide a structured + // suggestion. + let ObligationCauseCode::IfExpression(cause) = cause.code() else { + return; + }; + let hir::Node::Block(blk) = self.tcx.hir_node(cause.then_id) else { + return; + }; + let Some(then) = blk.expr else { + return; + }; + let hir::Node::Block(blk) = self.tcx.hir_node(cause.else_id) else { + return; + }; + let Some(else_) = blk.expr else { + return; + }; + let expected = match values.found.kind() { + ty::Alias(..) => values.expected, + _ => values.found, + }; + let preds = tcx.explicit_item_bounds(opaque_ty.def_id); + for (pred, _span) in preds.skip_binder() { + let ty::ClauseKind::Trait(trait_predicate) = pred.kind().skip_binder() + else { + continue; + }; + if trait_predicate.polarity != ty::ImplPolarity::Positive { + continue; + } + let def_id = trait_predicate.def_id(); + let mut impl_def_ids = vec![]; + tcx.for_each_relevant_impl(def_id, expected, |did| { + impl_def_ids.push(did) + }); + if let [_] = &impl_def_ids[..] { + let trait_name = tcx.item_name(def_id); + diag.multipart_suggestion( + format!( + "`{expected}` implements `{trait_name}` so you can box \ + both arms and coerce to the trait object \ + `Box`", + ), + vec![ + (then.span.shrink_to_lo(), "Box::new(".to_string()), + ( + then.span.shrink_to_hi(), + format!(") as Box", tcx.def_path_str(def_id)), + ), + (else_.span.shrink_to_lo(), "Box::new(".to_string()), + (else_.span.shrink_to_hi(), ")".to_string()), + ], + MachineApplicable, ); } } diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed index dfdf6b24bf7e0..9ce46bc1a65b2 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed @@ -5,6 +5,9 @@ impl Trait for Struct {} fn foo() -> Box { Box::new(Struct) } +fn bar() -> impl Trait { + Struct +} fn main() { let _ = if true { Box::new(Struct) @@ -16,4 +19,14 @@ fn main() { } else { Box::new(Struct) //~ ERROR E0308 }; + let _ = if true { + Box::new(Struct) as Box + } else { + Box::new(bar()) //~ ERROR E0308 + }; + let _ = if true { + Box::new(bar()) as Box + } else { + Box::new(Struct) //~ ERROR E0308 + }; } diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs index e138ad9b33647..7f65a3bb59d31 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs @@ -5,6 +5,9 @@ impl Trait for Struct {} fn foo() -> Box { Box::new(Struct) } +fn bar() -> impl Trait { + Struct +} fn main() { let _ = if true { Struct @@ -16,4 +19,14 @@ fn main() { } else { Struct //~ ERROR E0308 }; + let _ = if true { + Struct + } else { + bar() //~ ERROR E0308 + }; + let _ = if true { + bar() + } else { + Struct //~ ERROR E0308 + }; } diff --git a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr index 98c9d880f97aa..c58bf60e7d656 100644 --- a/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr +++ b/tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr @@ -1,5 +1,5 @@ error[E0308]: `if` and `else` have incompatible types - --> $DIR/suggest-box-on-divergent-if-else-arms.rs:12:9 + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:15:9 | LL | let _ = if true { | _____________- @@ -19,7 +19,7 @@ LL | Box::new(Struct) | +++++++++ + error[E0308]: `if` and `else` have incompatible types - --> $DIR/suggest-box-on-divergent-if-else-arms.rs:17:9 + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:20:9 | LL | let _ = if true { | _____________- @@ -39,6 +39,56 @@ help: store this in the heap by calling `Box::new` LL | Box::new(Struct) | +++++++++ + -error: aborting due to 2 previous errors +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:25:9 + | +LL | fn bar() -> impl Trait { + | ---------- the found opaque type +... +LL | let _ = if true { + | _____________- +LL | | Struct + | | ------ expected because of this +LL | | } else { +LL | | bar() + | | ^^^^^ expected `Struct`, found opaque type +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected struct `Struct` + found opaque type `impl Trait` +help: `Struct` implements `Trait` so you can box both arms and coerce to the trait object `Box` + | +LL ~ Box::new(Struct) as Box +LL | } else { +LL ~ Box::new(bar()) + | + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/suggest-box-on-divergent-if-else-arms.rs:30:9 + | +LL | fn bar() -> impl Trait { + | ---------- the expected opaque type +... +LL | let _ = if true { + | _____________- +LL | | bar() + | | ----- expected because of this +LL | | } else { +LL | | Struct + | | ^^^^^^ expected opaque type, found `Struct` +LL | | }; + | |_____- `if` and `else` have incompatible types + | + = note: expected opaque type `impl Trait` + found struct `Struct` +help: `Struct` implements `Trait` so you can box both arms and coerce to the trait object `Box` + | +LL ~ Box::new(bar()) as Box +LL | } else { +LL ~ Box::new(Struct) + | + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`.