Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide structured suggestion to use trait objects in some cases of if arm type divergence #120261

Merged
merged 3 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
118 changes: 102 additions & 16 deletions compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,30 +294,84 @@ impl<T> Trait<T> 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
| DefKind::Static(_)
| 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<dyn {trait_name}>`",
),
vec![
(then.span.shrink_to_lo(), "Box::new(".to_string()),
(
then.span.shrink_to_hi(),
format!(") as Box<dyn {}>", tcx.def_path_str(def_id)),
),
(else_.span.shrink_to_lo(), "Box::new(".to_string()),
(else_.span.shrink_to_hi(), ")".to_string()),
],
MachineApplicable,
);
}
}
Expand All @@ -330,6 +384,38 @@ impl<T> Trait<T> 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!(
Expand Down
32 changes: 32 additions & 0 deletions tests/ui/typeck/suggest-box-on-divergent-if-else-arms.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// run-rustfix
trait Trait {}
struct Struct;
impl Trait for Struct {}
fn foo() -> Box<dyn Trait> {
Box::new(Struct)
}
fn bar() -> impl Trait {
Struct
}
fn main() {
let _ = if true {
Box::new(Struct)
} else {
foo() //~ ERROR E0308
};
let _ = if true {
foo()
} else {
Box::new(Struct) //~ ERROR E0308
};
let _ = if true {
Box::new(Struct) as Box<dyn Trait>
} else {
Box::new(bar()) //~ ERROR E0308
};
let _ = if true {
Box::new(bar()) as Box<dyn Trait>
} else {
Box::new(Struct) //~ ERROR E0308
};
}
32 changes: 32 additions & 0 deletions tests/ui/typeck/suggest-box-on-divergent-if-else-arms.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// run-rustfix
trait Trait {}
struct Struct;
impl Trait for Struct {}
fn foo() -> Box<dyn Trait> {
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() //~ ERROR E0308
};
let _ = if true {
bar()
} else {
Struct //~ ERROR E0308
};
}
94 changes: 94 additions & 0 deletions tests/ui/typeck/suggest-box-on-divergent-if-else-arms.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
error[E0308]: `if` and `else` have incompatible types
--> $DIR/suggest-box-on-divergent-if-else-arms.rs:15:9
|
LL | let _ = if true {
| _____________-
LL | | Struct
| | ------ expected because of this
LL | | } else {
LL | | foo()
| | ^^^^^ expected `Struct`, found `Box<dyn Trait>`
LL | | };
| |_____- `if` and `else` have incompatible types
|
= note: expected struct `Struct`
found struct `Box<dyn Trait>`
help: `Struct` implements `Trait` so you can box it to coerce to the trait object `Box<dyn Trait>`
|
LL | Box::new(Struct)
| +++++++++ +

error[E0308]: `if` and `else` have incompatible types
--> $DIR/suggest-box-on-divergent-if-else-arms.rs:20:9
|
LL | let _ = if true {
| _____________-
LL | | foo()
| | ----- expected because of this
LL | | } else {
LL | | Struct
| | ^^^^^^ expected `Box<dyn Trait>`, found `Struct`
LL | | };
| |_____- `if` and `else` have incompatible types
|
= note: expected struct `Box<dyn Trait>`
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[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<dyn Trait>`
|
LL ~ Box::new(Struct) as Box<dyn Trait>
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<dyn Trait>`
|
LL ~ Box::new(bar()) as Box<dyn Trait>
LL | } else {
LL ~ Box::new(Struct)
|

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0308`.
Loading