Skip to content

Dont compute opt_suggest_box_span span for TAIT #112513

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

Merged
merged 3 commits into from
Jun 11, 2023
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: 8 additions & 0 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
..
} = self.type_var_origin(expected)? else { return None; };

let Some(rpit_local_def_id) = rpit_def_id.as_local() else { return None; };
if !matches!(
self.tcx.hir().expect_item(rpit_local_def_id).expect_opaque_ty().origin,
hir::OpaqueTyOrigin::FnReturn(..)
) {
return None;
}

let sig = self.body_fn_sig()?;

let substs = sig.output().walk().find_map(|arg| {
Expand Down
113 changes: 4 additions & 109 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@
//! ```

use crate::FnCtxt;
use rustc_errors::{
struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
};
use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, Visitor};
Expand All @@ -58,7 +56,7 @@ use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::{self, Ty, TypeAndMut};
use rustc_session::parse::feature_err;
use rustc_span::symbol::sym;
use rustc_span::{self, BytePos, DesugaringKind, Span};
use rustc_span::{self, DesugaringKind};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
Expand Down Expand Up @@ -1702,9 +1700,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
let mut err = fcx.err_ctxt().report_mismatched_types(cause, expected, found, ty_err);

let mut pointing_at_return_type = false;
let mut fn_output = None;

let parent_id = fcx.tcx.hir().parent_id(id);
let parent = fcx.tcx.hir().get(parent_id);
if let Some(expr) = expression
Expand All @@ -1717,7 +1712,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
// label pointing out the cause for the type coercion will be wrong
// as prior return coercions would not be relevant (#57664).
let fn_decl = if let (Some(expr), Some(blk_id)) = (expression, blk_id) {
pointing_at_return_type =
let pointing_at_return_type =
fcx.suggest_mismatched_types_on_tail(&mut err, expr, expected, found, blk_id);
if let (Some(cond_expr), true, false) = (
fcx.tcx.hir().get_if_cause(expr.hir_id),
Expand Down Expand Up @@ -1749,7 +1744,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {

if let Some((fn_id, fn_decl, can_suggest)) = fn_decl {
if blk_id.is_none() {
pointing_at_return_type |= fcx.suggest_missing_return_type(
fcx.suggest_missing_return_type(
&mut err,
&fn_decl,
expected,
Expand All @@ -1758,9 +1753,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
fn_id,
);
}
if !pointing_at_return_type {
fn_output = Some(&fn_decl.output); // `impl Trait` return type
}
}

let parent_id = fcx.tcx.hir().get_parent_item(id);
Expand Down Expand Up @@ -1795,106 +1787,9 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
);
}

if let (Some(sp), Some(fn_output)) = (ret_coercion_span, fn_output) {
self.add_impl_trait_explanation(&mut err, cause, fcx, expected, sp, fn_output);
}

err
}

fn add_impl_trait_explanation<'a>(
&self,
err: &mut Diagnostic,
cause: &ObligationCause<'tcx>,
fcx: &FnCtxt<'a, 'tcx>,
expected: Ty<'tcx>,
sp: Span,
fn_output: &hir::FnRetTy<'_>,
) {
let return_sp = fn_output.span();
err.span_label(return_sp, "expected because this return type...");
err.span_label(
sp,
format!("...is found to be `{}` here", fcx.resolve_vars_with_obligations(expected)),
);
let impl_trait_msg = "for information on `impl Trait`, see \
<https://doc.rust-lang.org/book/ch10-02-traits.html\
#returning-types-that-implement-traits>";
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>";
err.note("to return `impl Trait`, all returned values must be of the same type");
err.note(impl_trait_msg);
let snippet = fcx
.tcx
.sess
.source_map()
.span_to_snippet(return_sp)
.unwrap_or_else(|_| "dyn Trait".to_string());
let mut snippet_iter = snippet.split_whitespace();
let has_impl = snippet_iter.next().is_some_and(|s| s == "impl");
// Only suggest `Box<dyn Trait>` if `Trait` in `impl Trait` is object safe.
let mut is_object_safe = false;
if let hir::FnRetTy::Return(ty) = fn_output
// Get the return type.
&& let hir::TyKind::OpaqueDef(..) = ty.kind
{
let ty = fcx.astconv().ast_ty_to_ty( ty);
// Get the `impl Trait`'s `DefId`.
if let ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }) = ty.kind()
// Get the `impl Trait`'s `Item` so that we can get its trait bounds and
// get the `Trait`'s `DefId`.
&& let hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds, .. }) =
fcx.tcx.hir().expect_item(def_id.expect_local()).kind
{
// Are of this `impl Trait`'s traits object safe?
is_object_safe = bounds.iter().all(|bound| {
bound
.trait_ref()
.and_then(|t| t.trait_def_id())
.is_some_and(|def_id| {
fcx.tcx.check_is_object_safe(def_id)
})
})
}
};
if has_impl {
if is_object_safe {
err.multipart_suggestion(
"you could change the return type to be a boxed trait object",
vec![
(return_sp.with_hi(return_sp.lo() + BytePos(4)), "Box<dyn".to_string()),
(return_sp.shrink_to_hi(), ">".to_string()),
],
Applicability::MachineApplicable,
);
let sugg = [sp, cause.span]
.into_iter()
.flat_map(|sp| {
[
(sp.shrink_to_lo(), "Box::new(".to_string()),
(sp.shrink_to_hi(), ")".to_string()),
]
.into_iter()
})
.collect::<Vec<_>>();
err.multipart_suggestion(
"if you change the return type to expect trait objects, box the returned \
expressions",
sugg,
Applicability::MaybeIncorrect,
);
} else {
err.help(format!(
"if the trait `{}` were object safe, you could return a boxed trait object",
&snippet[5..]
));
}
err.note(trait_obj_msg);
}
err.help("you could instead create a new `enum` with a variant for each returned type");
}

/// Checks whether the return type is unsized via an obligation, which makes
/// sure we consider `dyn Trait: Sized` where clauses, which are trivially
/// false but technically valid for typeck.
Expand Down
20 changes: 19 additions & 1 deletion compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,25 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
) {
err.subdiagnostic(subdiag);
}
if let Some(ret_sp) = opt_suggest_box_span {
// don't suggest wrapping either blocks in `if .. {} else {}`
let is_empty_arm = |id| {
let hir::Node::Block(blk) = self.tcx.hir().get(id)
else {
return false;
};
if blk.expr.is_some() || !blk.stmts.is_empty() {
return false;
}
let Some((_, hir::Node::Expr(expr))) = self.tcx.hir().parent_iter(id).nth(1)
else {
return false;
};
matches!(expr.kind, hir::ExprKind::If(..))
};
if let Some(ret_sp) = opt_suggest_box_span
&& !is_empty_arm(then_id)
&& !is_empty_arm(else_id)
{
self.suggest_boxing_for_return_impl_trait(
err,
ret_sp,
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn test() -> impl std::fmt::Debug {
if true {
"boo2"
} else {
//~^ ERROR `if` and `else` have incompatible types
}
}

fn main() {}
16 changes: 16 additions & 0 deletions tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0308]: `if` and `else` have incompatible types
--> $DIR/dont-suggest-box-on-empty-else-arm.rs:4:12
|
LL | if true {
| ------- `if` and `else` have incompatible types
LL | "boo2"
| ------ expected because of this
LL | } else {
| ____________^
LL | |
LL | | }
| |_____^ expected `&str`, found `()`

error: aborting due to previous error

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