bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #95603 - compiler-errors:dyn-return, r=oli-obk

Browse source 
Fix late-bound ICE in `dyn` return type suggestion

This fixes the root-cause of the attached issues -- the root problem is that we're using the return type from a signature with late-bound instead of early-bound regions. The change on line 1087 (`let Some(liberated_sig) = typeck_results.liberated_fn_sigs().get(fn_hir_id) else { return false; };`) makes sure we're grabbing the _right_ return type for this suggestion to check the `dyn` predicates with.

Fixes #91801
Fixes #91803

This fix also includes some drive-by changes, specifically:

1. Don't suggest boxing when we have `-> dyn Trait` and are already returning `Box<T>` where `T: Trait` (before we always boxed the value).
2. Suggestion applies even when the return type is a type alias (e.g. `type Foo = dyn Trait`). This does cause the suggestion to expand to the aliased type, but I think it's still beneficial.
3. Split up the multipart suggestion because there's a 6-line max in the printed output...

I am open to splitting out the above changes, if we just want to fix the ICE first.

cc: ```@terrarier2111``` and #92289
This commit is contained in:
Dylan DPC 2022-04-05 01:53:32 +02:00 • committed by GitHub
commit 92e53f5cc9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 242 additions and 84 deletions
Download patch file Download diff file Expand all files Collapse all files

134
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file

@ -1121,8 +1121,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),
..
@ -1160,16 +1160,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<Ty<'_>>, bool, bool),
ty| {
(_, ty)| {
let ty = self.resolve_vars_if_possible(ty);
same &=
!matches!(ty.kind(), ty::Error(_))
@ -1190,39 +1191,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();
@ -1232,6 +1254,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let trait_obj_msg = "for information on trait objects, see \
<https://doc.rust-lang.org/book/ch17-02-trait-objects.html\
#using-trait-objects-that-allow-for-values-of-different-types>";
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 {
@ -1259,26 +1282,25 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
} else {
if is_object_safe {
// Suggest `-> Box<dyn Trait>` 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<dyn {}>", 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
@ -2753,13 +2775,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<dyn {}>", trait_obj),
vec![
(ret_ty.shrink_to_lo(), "Box<".to_string()),
(ret_ty.shrink_to_hi(), ">".to_string()),
],
Applicability::MaybeIncorrect,
);
}