Skip to content

Commit

Permalink
Rollup merge of #131751 - compiler-errors:structurally-resolve, r=lcnr
Browse files Browse the repository at this point in the history
Rename `can_coerce` to `may_coerce`, and then structurally resolve correctly in the probe

We need to structurally resolve the lhs and rhs of the coercion. Also, renaming the method so it's less ambiguous about what it's doing... the word "may" gives more clear signal that it has false positives imo.

r? lcnr
  • Loading branch information
matthiaskrgr authored Oct 16, 2024
2 parents 82952da + 9070aba commit 3b8fd5f
Show file tree
Hide file tree
Showing 11 changed files with 121 additions and 57 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(ret_coercion) => {
let ret_ty = ret_coercion.borrow().expected_ty();
let ret_ty = self.infcx.shallow_resolve(ret_ty);
self.can_coerce(arm_ty, ret_ty)
&& prior_arm.is_none_or(|(_, ty, _)| self.can_coerce(ty, ret_ty))
self.may_coerce(arm_ty, ret_ty)
&& prior_arm.is_none_or(|(_, ty, _)| self.may_coerce(ty, ret_ty))
// The match arms need to unify for the case of `impl Trait`.
&& !matches!(ret_ty.kind(), ty::Alias(ty::Opaque, ..))
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
let mut sugg_mutref = false;
if let ty::Ref(reg, cast_ty, mutbl) = *self.cast_ty.kind() {
if let ty::RawPtr(expr_ty, _) = *self.expr_ty.kind()
&& fcx.can_coerce(
&& fcx.may_coerce(
Ty::new_ref(fcx.tcx, fcx.tcx.lifetimes.re_erased, expr_ty, mutbl),
self.cast_ty,
)
Expand All @@ -418,22 +418,22 @@ impl<'a, 'tcx> CastCheck<'tcx> {
} else if let ty::Ref(expr_reg, expr_ty, expr_mutbl) = *self.expr_ty.kind()
&& expr_mutbl == Mutability::Not
&& mutbl == Mutability::Mut
&& fcx.can_coerce(Ty::new_mut_ref(fcx.tcx, expr_reg, expr_ty), self.cast_ty)
&& fcx.may_coerce(Ty::new_mut_ref(fcx.tcx, expr_reg, expr_ty), self.cast_ty)
{
sugg_mutref = true;
}

if !sugg_mutref
&& sugg == None
&& fcx.can_coerce(
&& fcx.may_coerce(
Ty::new_ref(fcx.tcx, reg, self.expr_ty, mutbl),
self.cast_ty,
)
{
sugg = Some((format!("&{}", mutbl.prefix_str()), false));
}
} else if let ty::RawPtr(_, mutbl) = *self.cast_ty.kind()
&& fcx.can_coerce(
&& fcx.may_coerce(
Ty::new_ref(fcx.tcx, fcx.tcx.lifetimes.re_erased, self.expr_ty, mutbl),
self.cast_ty,
)
Expand Down
42 changes: 30 additions & 12 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1084,24 +1084,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
}

/// Same as `coerce()`, but without side-effects.
/// Probe whether `expr_ty` can be coerced to `target_ty`. This has no side-effects,
/// and may return false positives if types are not yet fully constrained by inference.
///
/// Returns false if the coercion creates any obligations that result in
/// errors.
pub(crate) fn can_coerce(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> bool {
// FIXME(-Znext-solver): We need to structurally resolve both types here.
let source = self.resolve_vars_with_obligations(expr_ty);
debug!("coercion::can_with_predicates({:?} -> {:?})", source, target);

/// Returns false if the coercion is not possible, or if the coercion creates any
/// sub-obligations that result in errors.
///
/// This should only be used for diagnostics.
pub(crate) fn may_coerce(&self, expr_ty: Ty<'tcx>, target_ty: Ty<'tcx>) -> bool {
let cause = self.cause(DUMMY_SP, ObligationCauseCode::ExprAssignable);
// We don't ever need two-phase here since we throw out the result of the coercion.
// We also just always set `coerce_never` to true, since this is a heuristic.
let coerce = Coerce::new(self, cause, AllowTwoPhase::No, true);
let coerce = Coerce::new(self, cause.clone(), AllowTwoPhase::No, true);
self.probe(|_| {
let Ok(ok) = coerce.coerce(source, target) else {
// Make sure to structurally resolve the types, since we use
// the `TyKind`s heavily in coercion.
let ocx = ObligationCtxt::new(self);
let structurally_resolve = |ty| {
let ty = self.shallow_resolve(ty);
if self.next_trait_solver()
&& let ty::Alias(..) = ty.kind()
{
ocx.structurally_normalize(&cause, self.param_env, ty)
} else {
Ok(ty)
}
};
let Ok(expr_ty) = structurally_resolve(expr_ty) else {
return false;
};
let Ok(target_ty) = structurally_resolve(target_ty) else {
return false;
};

let Ok(ok) = coerce.coerce(expr_ty, target_ty) else {
return false;
};
let ocx = ObligationCtxt::new(self);
ocx.register_obligations(ok.obligations);
ocx.select_where_possible().is_empty()
})
Expand Down Expand Up @@ -1370,7 +1388,7 @@ pub fn can_coerce<'tcx>(
) -> bool {
let root_ctxt = crate::typeck_root_ctxt::TypeckRootCtxt::new(tcx, body_id);
let fn_ctxt = FnCtxt::new(&root_ctxt, param_env, body_id);
fn_ctxt.can_coerce(ty, output_ty)
fn_ctxt.may_coerce(ty, output_ty)
}

/// CoerceMany encapsulates the pattern you should use when you have
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1330,9 +1330,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let refs_can_coerce = |lhs: Ty<'tcx>, rhs: Ty<'tcx>| {
let lhs = Ty::new_imm_ref(self.tcx, self.tcx.lifetimes.re_erased, lhs.peel_refs());
let rhs = Ty::new_imm_ref(self.tcx, self.tcx.lifetimes.re_erased, rhs.peel_refs());
self.can_coerce(rhs, lhs)
self.may_coerce(rhs, lhs)
};
let (applicability, eq) = if self.can_coerce(rhs_ty, lhs_ty) {
let (applicability, eq) = if self.may_coerce(rhs_ty, lhs_ty) {
(Applicability::MachineApplicable, true)
} else if refs_can_coerce(rhs_ty, lhs_ty) {
// The lhs and rhs are likely missing some references in either side. Subsequent
Expand All @@ -1349,7 +1349,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let actual_lhs_ty = self.check_expr(rhs_expr);
(
Applicability::MaybeIncorrect,
self.can_coerce(rhs_ty, actual_lhs_ty)
self.may_coerce(rhs_ty, actual_lhs_ty)
|| refs_can_coerce(rhs_ty, actual_lhs_ty),
)
} else if let ExprKind::Binary(
Expand All @@ -1363,7 +1363,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let actual_rhs_ty = self.check_expr(lhs_expr);
(
Applicability::MaybeIncorrect,
self.can_coerce(actual_rhs_ty, lhs_ty)
self.may_coerce(actual_rhs_ty, lhs_ty)
|| refs_can_coerce(actual_rhs_ty, lhs_ty),
)
} else {
Expand Down Expand Up @@ -1414,7 +1414,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.param_env,
)
.may_apply();
if lhs_deref_ty_is_sized && self.can_coerce(rhs_ty, lhs_deref_ty) {
if lhs_deref_ty_is_sized && self.may_coerce(rhs_ty, lhs_deref_ty) {
err.span_suggestion_verbose(
lhs.span.shrink_to_lo(),
"consider dereferencing here to assign to the mutably borrowed value",
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& fn_sig.inputs()[1..]
.iter()
.zip(input_types.iter())
.all(|(expected, found)| self.can_coerce(*expected, *found))
.all(|(expected, found)| self.may_coerce(*expected, *found))
&& fn_sig.inputs()[1..].len() == input_types.len()
{
err.span_suggestion_verbose(
Expand Down Expand Up @@ -722,7 +722,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let expectation = Expectation::rvalue_hint(self, expected_input_ty);
let coerced_ty = expectation.only_has_type(self).unwrap_or(formal_input_ty);
let can_coerce = self.can_coerce(arg_ty, coerced_ty);
let can_coerce = self.may_coerce(arg_ty, coerced_ty);
if !can_coerce {
return Compatibility::Incompatible(Some(ty::error::TypeError::Sorts(
ty::error::ExpectedFound::new(true, coerced_ty, arg_ty),
Expand Down Expand Up @@ -802,7 +802,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
provided_arg_tys.iter().map(|(ty, _)| *ty).skip(mismatch_idx + tys.len()),
),
) {
if !self.can_coerce(provided_ty, *expected_ty) {
if !self.may_coerce(provided_ty, *expected_ty) {
satisfied = false;
break;
}
Expand Down Expand Up @@ -1023,7 +1023,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
std::iter::zip(formal_and_expected_inputs.iter(), removed_arg_tys.iter()).all(
|((expected_ty, _), (provided_ty, _))| {
!provided_ty.references_error()
&& self.can_coerce(*provided_ty, *expected_ty)
&& self.may_coerce(*provided_ty, *expected_ty)
},
)
};
Expand Down Expand Up @@ -2124,7 +2124,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let expr_ty = self.typeck_results.borrow().expr_ty(expr);
let return_ty = fn_sig.output();
if !matches!(expr.kind, hir::ExprKind::Ret(..))
&& self.can_coerce(expr_ty, return_ty)
&& self.may_coerce(expr_ty, return_ty)
{
found_semi = true;
}
Expand Down
34 changes: 17 additions & 17 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let hir::ExprKind::MethodCall(hir::PathSegment { ident: method, .. }, recv_expr, &[], _) =
expr.kind
&& let Some(recv_ty) = self.typeck_results.borrow().expr_ty_opt(recv_expr)
&& self.can_coerce(recv_ty, expected)
&& self.may_coerce(recv_ty, expected)
&& let name = method.name.as_str()
&& (name.starts_with("to_") || name.starts_with("as_") || name == "into")
{
Expand Down Expand Up @@ -349,7 +349,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return true;
}

if self.suggest_fn_call(err, expr, found, |output| self.can_coerce(output, expected))
if self.suggest_fn_call(err, expr, found, |output| self.may_coerce(output, expected))
&& let ty::FnDef(def_id, ..) = *found.kind()
&& let Some(sp) = self.tcx.hir().span_if_local(def_id)
{
Expand Down Expand Up @@ -568,7 +568,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if self.tcx.hir().is_inside_const_context(hir_id) || !expected.is_box() || found.is_box() {
return false;
}
if self.can_coerce(Ty::new_box(self.tcx, found), expected) {
if self.may_coerce(Ty::new_box(self.tcx, found), expected) {
let suggest_boxing = match found.kind() {
ty::Tuple(tuple) if tuple.is_empty() => {
errors::SuggestBoxing::Unit { start: span.shrink_to_lo(), end: span }
Expand Down Expand Up @@ -663,7 +663,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
match expected.kind() {
ty::Adt(def, _) if Some(def.did()) == pin_did => {
if self.can_coerce(pin_box_found, expected) {
if self.may_coerce(pin_box_found, expected) {
debug!("can coerce {:?} to {:?}, suggesting Box::pin", pin_box_found, expected);
match found.kind() {
ty::Adt(def, _) if def.is_box() => {
Expand All @@ -689,7 +689,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
true
} else if self.can_coerce(pin_found, expected) {
} else if self.may_coerce(pin_found, expected) {
match found.kind() {
ty::Adt(def, _) if def.is_box() => {
err.help("use `Box::pin`");
Expand All @@ -701,7 +701,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
}
}
ty::Adt(def, _) if def.is_box() && self.can_coerce(box_found, expected) => {
ty::Adt(def, _) if def.is_box() && self.may_coerce(box_found, expected) => {
// Check if the parent expression is a call to Pin::new. If it
// is and we were expecting a Box, ergo Pin<Box<expected>>, we
// can suggest Box::pin.
Expand Down Expand Up @@ -884,7 +884,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let ty = Binder::bind_with_vars(ty, bound_vars);
let ty = self.normalize(hir_ty.span, ty);
let ty = self.tcx.instantiate_bound_regions_with_erased(ty);
if self.can_coerce(expected, ty) {
if self.may_coerce(expected, ty) {
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Other {
span: hir_ty.span,
expected,
Expand Down Expand Up @@ -1141,12 +1141,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty::Asyncness::No => ty,
};
let ty = self.normalize(expr.span, ty);
self.can_coerce(found, ty)
self.may_coerce(found, ty)
}
hir::FnRetTy::DefaultReturn(_) if in_closure => {
self.ret_coercion.as_ref().map_or(false, |ret| {
let ret_ty = ret.borrow().expected_ty();
self.can_coerce(found, ret_ty)
self.may_coerce(found, ret_ty)
})
}
_ => false,
Expand Down Expand Up @@ -1510,7 +1510,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
provided_ty
};

if !self.can_coerce(expected_ty, dummy_ty) {
if !self.may_coerce(expected_ty, dummy_ty) {
return;
}
let msg = format!("use `{adt_name}::map_or` to deref inner value of `{adt_name}`");
Expand All @@ -1534,7 +1534,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected_ty: Ty<'tcx>,
) {
if let ty::Slice(elem_ty) | ty::Array(elem_ty, _) = expected_ty.kind() {
if self.can_coerce(blk_ty, *elem_ty)
if self.may_coerce(blk_ty, *elem_ty)
&& blk.stmts.is_empty()
&& blk.rules == hir::BlockCheckMode::DefaultBlock
{
Expand Down Expand Up @@ -1744,7 +1744,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if item_ty.has_param() {
return false;
}
if self.can_coerce(item_ty, expected_ty) {
if self.may_coerce(item_ty, expected_ty) {
err.span_suggestion_verbose(
segment.ident.span,
format!("try referring to the associated const `{capitalized_name}` instead",),
Expand Down Expand Up @@ -1804,7 +1804,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// diagnostic in cases where we have `(&&T).clone()` and we expect `T`).
&& !results.expr_adjustments(callee_expr).iter().any(|adj| matches!(adj.kind, ty::adjustment::Adjust::Deref(..)))
// Check that we're in fact trying to clone into the expected type
&& self.can_coerce(*pointee_ty, expected_ty)
&& self.may_coerce(*pointee_ty, expected_ty)
&& let trait_ref = ty::TraitRef::new(self.tcx, clone_trait_did, [expected_ty])
// And the expected type doesn't implement `Clone`
&& !self.predicate_must_hold_considering_regions(&traits::Obligation::new(
Expand Down Expand Up @@ -2022,7 +2022,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else {
return false;
};
if is_ctor || !self.can_coerce(args.type_at(0), expected) {
if is_ctor || !self.may_coerce(args.type_at(0), expected) {
return false;
}

Expand Down Expand Up @@ -2293,7 +2293,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.then(|| " (its field is private, but it's local to this crate and its privacy can be changed)".to_string());

let sole_field_ty = sole_field.ty(self.tcx, args);
if self.can_coerce(expr_ty, sole_field_ty) {
if self.may_coerce(expr_ty, sole_field_ty) {
let variant_path =
with_no_trimmed_paths!(self.tcx.def_path_str(variant.def_id));
// FIXME #56861: DRYer prelude filtering
Expand Down Expand Up @@ -2401,7 +2401,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

let int_type = args.type_at(0);
if !self.can_coerce(expr_ty, int_type) {
if !self.may_coerce(expr_ty, int_type) {
return false;
}

Expand Down Expand Up @@ -2585,7 +2585,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Ty::new_imm_ref(self.tcx, self.tcx.lifetimes.re_static, checked_ty)
}
};
if self.can_coerce(ref_ty, expected) {
if self.may_coerce(ref_ty, expected) {
let mut sugg_sp = sp;
if let hir::ExprKind::MethodCall(segment, receiver, args, _) = expr.kind {
let clone_trait =
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1934,7 +1934,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& fn_sig.inputs()[1..]
.iter()
.zip(args.into_iter())
.all(|(expected, found)| self.can_coerce(*expected, *found))
.all(|(expected, found)| self.may_coerce(*expected, *found))
&& fn_sig.inputs()[1..].len() == args.len()
{
err.span_suggestion_verbose(
Expand Down Expand Up @@ -4148,7 +4148,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return false;
};

if !self.can_coerce(output, expected) {
if !self.may_coerce(output, expected) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else if inexistent_fields.len() == 1 {
match pat_field.pat.kind {
PatKind::Lit(expr)
if !self.can_coerce(
if !self.may_coerce(
self.typeck_results.borrow().expr_ty(expr),
self.field_ty(field.span, field_def, args),
) => {}
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/traits/next-solver/diagnostics/coerce-in-may-coerce.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//@ compile-flags: -Znext-solver

trait Mirror {
type Assoc;
}
impl<T> Mirror for T {
type Assoc = T;
}

fn arg() -> &'static [i32; 1] { todo!() }

fn arg_error(x: <fn() as Mirror>::Assoc, y: ()) { todo!() }

fn main() {
// Should suggest to reverse the args...
// but if we don't normalize the expected, then we don't.
arg_error((), || ());
//~^ ERROR arguments to this function are incorrect
}
Loading

0 comments on commit 3b8fd5f

Please sign in to comment.