Skip to content

Commit

Permalink
Auto merge of #101604 - compiler-errors:issue-101465, r=lcnr
Browse files Browse the repository at this point in the history
Fix ICE in opt_suggest_box_span

We were _totally_ mishandling substs and obligations in `opt_suggest_box_span`, so I reworked that function pretty heavily.

Also some drive-by changes, namely removing `ret_type_span`.

Fixes #101465
  • Loading branch information
bors committed Sep 12, 2022
2 parents 56e7678 + 5599a45 commit a93214e
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 50 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// for opaque types, and then use that kind to fix the spans for type errors
// that we see later on.
let ty_var = self.next_ty_var(TypeVariableOrigin {
kind: TypeVariableOriginKind::TypeInference,
kind: TypeVariableOriginKind::OpaqueTypeInference(def_id),
span,
});
obligations.extend(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/type_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ pub enum TypeVariableOriginKind {
MiscVariable,
NormalizeProjectionType,
TypeInference,
OpaqueTypeInference(DefId),
TypeParameterDefinition(Symbol, Option<DefId>),

/// One of the upvars or closure kind parameters in a `ClosureSubsts`
Expand Down
113 changes: 68 additions & 45 deletions compiler/rustc_typeck/src/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_errors::{Applicability, MultiSpan};
use rustc_hir::{self as hir, ExprKind};
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::traits::Obligation;
use rustc_middle::ty::{self, ToPredicate, Ty};
use rustc_middle::ty::{self, Subst, ToPredicate, Ty};
use rustc_span::Span;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{
Expand Down Expand Up @@ -137,9 +137,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(&arm.body),
arm_ty,
Some(&mut |err| {
let Some(ret) = self.ret_type_span else {
return;
};
let Some(ret) = self
.tcx
.hir()
.find_by_def_id(self.body_id.owner)
.and_then(|owner| owner.fn_decl())
.map(|decl| decl.output.span())
else { return; };
let Expectation::IsLast(stmt) = orig_expected else {
return
};
Expand Down Expand Up @@ -468,58 +472,77 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

// When we have a `match` as a tail expression in a `fn` with a returned `impl Trait`
// we check if the different arms would work with boxed trait objects instead and
// provide a structured suggestion in that case.
/// When we have a `match` as a tail expression in a `fn` with a returned `impl Trait`
/// we check if the different arms would work with boxed trait objects instead and
/// provide a structured suggestion in that case.
pub(crate) fn opt_suggest_box_span(
&self,
first_ty: Ty<'tcx>,
second_ty: Ty<'tcx>,
orig_expected: Expectation<'tcx>,
) -> Option<Span> {
// FIXME(compiler-errors): This really shouldn't need to be done during the
// "good" path of typeck, but here we are.
match orig_expected {
Expectation::ExpectHasType(expected)
if self.in_tail_expr
&& self.return_type_has_opaque
&& self.can_coerce(first_ty, expected)
&& self.can_coerce(second_ty, expected) =>
{
let obligations = self.fulfillment_cx.borrow().pending_obligations();
let mut suggest_box = !obligations.is_empty();
'outer: for o in obligations {
for outer_ty in &[first_ty, second_ty] {
match o.predicate.kind().skip_binder() {
ty::PredicateKind::Trait(t) => {
let pred = ty::Binder::dummy(ty::PredicateKind::Trait(
ty::TraitPredicate {
trait_ref: ty::TraitRef {
def_id: t.def_id(),
substs: self.tcx.mk_substs_trait(*outer_ty, &[]),
},
constness: t.constness,
polarity: t.polarity,
},
));
let obl = Obligation::new(
o.cause.clone(),
self.param_env,
pred.to_predicate(self.tcx),
);
suggest_box &= self.predicate_must_hold_modulo_regions(&obl);
if !suggest_box {
// We've encountered some obligation that didn't hold, so the
// return expression can't just be boxed. We don't need to
// evaluate the rest of the obligations.
break 'outer;
}
Expectation::ExpectHasType(expected) => {
let TypeVariableOrigin {
span,
kind: TypeVariableOriginKind::OpaqueTypeInference(rpit_def_id),
..
} = self.type_var_origin(expected)? else { return None; };

let sig = *self
.typeck_results
.borrow()
.liberated_fn_sigs()
.get(hir::HirId::make_owner(self.body_id.owner))?;

let substs = sig.output().walk().find_map(|arg| {
if let ty::GenericArgKind::Type(ty) = arg.unpack()
&& let ty::Opaque(def_id, substs) = *ty.kind()
&& def_id == rpit_def_id
{
Some(substs)
} else {
None
}
})?;
let opaque_ty = self.tcx.mk_opaque(rpit_def_id, substs);

if !self.can_coerce(first_ty, expected) || !self.can_coerce(second_ty, expected) {
return None;
}

for ty in [first_ty, second_ty] {
for pred in self.tcx.bound_explicit_item_bounds(rpit_def_id).transpose_iter() {
let pred = pred.map_bound(|(pred, _)| *pred).subst(self.tcx, substs);
let pred = match pred.kind().skip_binder() {
ty::PredicateKind::Trait(mut trait_pred) => {
assert_eq!(trait_pred.trait_ref.self_ty(), opaque_ty);
trait_pred.trait_ref.substs =
self.tcx.mk_substs_trait(ty, &trait_pred.trait_ref.substs[1..]);
pred.kind().rebind(trait_pred).to_predicate(self.tcx)
}
_ => {}
ty::PredicateKind::Projection(mut proj_pred) => {
assert_eq!(proj_pred.projection_ty.self_ty(), opaque_ty);
proj_pred.projection_ty.substs = self
.tcx
.mk_substs_trait(ty, &proj_pred.projection_ty.substs[1..]);
pred.kind().rebind(proj_pred).to_predicate(self.tcx)
}
_ => continue,
};
if !self.predicate_must_hold_modulo_regions(&Obligation::new(
ObligationCause::misc(span, self.body_id),
self.param_env,
pred,
)) {
return None;
}
}
}
// If all the obligations hold (or there are no obligations) the tail expression
// we can suggest to return a boxed trait object instead of an opaque type.
if suggest_box { self.ret_type_span } else { None }

Some(span)
}
_ => None,
}
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_typeck/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ pub(super) fn check_fn<'a, 'tcx>(
fcx.return_type_has_opaque = ret_ty != declared_ret_ty;

fcx.ret_coercion = Some(RefCell::new(CoerceMany::new(ret_ty)));
fcx.ret_type_span = Some(decl.output.span());

let span = body.value.span;

Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_typeck/src/check/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ pub struct FnCtxt<'a, 'tcx> {
/// any).
pub(super) ret_coercion: Option<RefCell<DynamicCoerceMany<'tcx>>>,

pub(super) ret_type_span: Option<Span>,

/// Used exclusively to reduce cost of advanced evaluation used for
/// more helpful diagnostics.
pub(super) in_tail_expr: bool,
Expand Down Expand Up @@ -142,7 +140,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
param_env,
err_count_on_creation: inh.tcx.sess.err_count(),
ret_coercion: None,
ret_type_span: None,
in_tail_expr: false,
ret_coercion_span: Cell::new(None),
resume_yield_tys: None,
Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/suggestions/issue-101465.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![feature(trait_alias)]

struct B;
struct C;

trait Tr {}

impl Tr for B {}
impl Tr for C {}

trait Tr2<S> = Into<S>;

fn foo2<T: Tr2<()>>() {}

fn foo() -> impl Tr {
let x = foo2::<_>();

match true {
true => B,
false => C,
//~^ `match` arms have incompatible types
}
}

fn main() {}
25 changes: 25 additions & 0 deletions src/test/ui/suggestions/issue-101465.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error[E0308]: `match` arms have incompatible types
--> $DIR/issue-101465.rs:20:18
|
LL | / match true {
LL | | true => B,
| | - this is found to be of type `B`
LL | | false => C,
| | ^ expected struct `B`, found struct `C`
LL | |
LL | | }
| |_____- `match` arms have incompatible types
|
help: you could change the return type to be a boxed trait object
|
LL | fn foo() -> Box<dyn Tr> {
| ~~~~~~~ +
help: if you change the return type to expect trait objects, box the returned expressions
|
LL ~ true => Box::new(B),
LL ~ false => Box::new(C),
|

error: aborting due to previous error

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

0 comments on commit a93214e

Please sign in to comment.