Skip to content

Do not consider match/let/ref of place that evaluates to ! to diverge, disallow coercions from them too #18278

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.

Sign up for GitHub

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 1 commit into from
Oct 15, 2024
Merged

Do not consider match/let/ref of place that evaluates to ! to diverge, disallow coercions from them too #18278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion crates/hir-ty/src/infer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use crate::{
db::HirDatabase,
fold_tys,
generics::Generics,
infer::{coerce::CoerceMany, unify::InferenceTable},
infer::{coerce::CoerceMany, expr::ExprIsRead, unify::InferenceTable},
lower::ImplTraitLoweringMode,
mir::MirSpan,
to_assoc_type_id,
Expand Down Expand Up @@ -1154,6 +1154,7 @@ impl<'a> InferenceContext<'a> {
_ = self.infer_expr_coerce(
self.body.body_expr,
&Expectation::has_type(self.return_ty.clone()),
ExprIsRead::Yes,
)
}
}
Expand Down
15 changes: 9 additions & 6 deletions crates/hir-ty/src/infer/cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use hir_def::{hir::ExprId, AdtId};
use stdx::never;

use crate::{
infer::unify::InferenceTable, Adjustment, Binders, DynTy, InferenceDiagnostic, Interner,
PlaceholderIndex, QuantifiedWhereClauses, Ty, TyExt, TyKind, TypeFlags, WhereClause,
infer::{coerce::CoerceNever, unify::InferenceTable},
Adjustment, Binders, DynTy, InferenceDiagnostic, Interner, PlaceholderIndex,
QuantifiedWhereClauses, Ty, TyExt, TyKind, TypeFlags, WhereClause,
};

#[derive(Debug)]
Expand Down Expand Up @@ -128,7 +129,7 @@ impl CastCheck {
return Ok(());
}

if let Ok((adj, _)) = table.coerce(&self.expr_ty, &self.cast_ty) {
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &self.cast_ty, CoerceNever::Yes) {
apply_adjustments(self.source_expr, adj);
set_coercion_cast(self.source_expr);
return Ok(());
Expand All @@ -154,7 +155,8 @@ impl CastCheck {
let sig = self.expr_ty.callable_sig(table.db).expect("FnDef had no sig");
let sig = table.normalize_associated_types_in(sig);
let fn_ptr = TyKind::Function(sig.to_fn_ptr()).intern(Interner);
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &fn_ptr) {
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &fn_ptr, CoerceNever::Yes)
{
apply_adjustments(self.source_expr, adj);
} else {
return Err(CastError::IllegalCast);
Expand Down Expand Up @@ -241,7 +243,8 @@ impl CastCheck {
if let TyKind::Array(ety, _) = t_expr.kind(Interner) {
// Coerce to a raw pointer so that we generate RawPtr in MIR.
let array_ptr_type = TyKind::Raw(m_expr, t_expr.clone()).intern(Interner);
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &array_ptr_type) {
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &array_ptr_type, CoerceNever::Yes)
{
apply_adjustments(self.source_expr, adj);
} else {
never!(
Expand All @@ -253,7 +256,7 @@ impl CastCheck {

// This is a less strict condition than rustc's `demand_eqtype`,
// but false negative is better than false positive
if table.coerce(ety, t_cast).is_ok() {
if table.coerce(ety, t_cast, CoerceNever::Yes).is_ok() {
return Ok(());
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/hir-ty/src/infer/closure.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use crate::{
db::{HirDatabase, InternedClosure},
error_lifetime, from_chalk_trait_id, from_placeholder_idx,
generics::Generics,
infer::coerce::CoerceNever,
make_binders,
mir::{BorrowKind, MirSpan, MutBorrowKind, ProjectionElem},
to_chalk_trait_id,
Expand Down Expand Up @@ -65,7 +66,7 @@ impl InferenceContext<'_> {
}

// Deduction from where-clauses in scope, as well as fn-pointer coercion are handled here.
let _ = self.coerce(Some(closure_expr), closure_ty, &expected_ty);
let _ = self.coerce(Some(closure_expr), closure_ty, &expected_ty, CoerceNever::Yes);

// Coroutines are not Fn* so return early.
if matches!(closure_ty.kind(Interner), TyKind::Coroutine(..)) {
Expand Down
49 changes: 33 additions & 16 deletions crates/hir-ty/src/infer/coerce.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ impl CoerceMany {
};
if let Some(sig) = sig {
let target_ty = TyKind::Function(sig.to_fn_ptr()).intern(Interner);
let result1 = ctx.table.coerce_inner(self.merged_ty(), &target_ty);
let result2 = ctx.table.coerce_inner(expr_ty.clone(), &target_ty);
let result1 = ctx.table.coerce_inner(self.merged_ty(), &target_ty, CoerceNever::Yes);
let result2 = ctx.table.coerce_inner(expr_ty.clone(), &target_ty, CoerceNever::Yes);
if let (Ok(result1), Ok(result2)) = (result1, result2) {
ctx.table.register_infer_ok(InferOk { value: (), goals: result1.goals });
for &e in &self.expressions {
Expand All @@ -159,9 +159,9 @@ impl CoerceMany {
// type is a type variable and the new one is `!`, trying it the other
// way around first would mean we make the type variable `!`, instead of
// just marking it as possibly diverging.
if let Ok(res) = ctx.coerce(expr, &expr_ty, &self.merged_ty()) {
if let Ok(res) = ctx.coerce(expr, &expr_ty, &self.merged_ty(), CoerceNever::Yes) {
self.final_ty = Some(res);
} else if let Ok(res) = ctx.coerce(expr, &self.merged_ty(), &expr_ty) {
} else if let Ok(res) = ctx.coerce(expr, &self.merged_ty(), &expr_ty, CoerceNever::Yes) {
self.final_ty = Some(res);
} else {
match cause {
Expand Down Expand Up @@ -197,7 +197,7 @@ pub(crate) fn coerce(
let vars = table.fresh_subst(tys.binders.as_slice(Interner));
let ty1_with_vars = vars.apply(tys.value.0.clone(), Interner);
let ty2_with_vars = vars.apply(tys.value.1.clone(), Interner);
let (adjustments, ty) = table.coerce(&ty1_with_vars, &ty2_with_vars)?;
let (adjustments, ty) = table.coerce(&ty1_with_vars, &ty2_with_vars, CoerceNever::Yes)?;
// default any type vars that weren't unified back to their original bound vars
// (kind of hacky)
let find_var = |iv| {
Expand All @@ -219,6 +219,12 @@ pub(crate) fn coerce(
Ok((adjustments, table.resolve_with_fallback(ty, &fallback)))
}

#[derive(Clone, Copy, PartialEq, Eq)]
pub(crate) enum CoerceNever {
Yes,
No,
}

impl InferenceContext<'_> {
/// Unify two types, but may coerce the first one to the second one
/// using "implicit coercion rules" if needed.
Expand All @@ -227,10 +233,16 @@ impl InferenceContext<'_> {
expr: Option<ExprId>,
from_ty: &Ty,
to_ty: &Ty,
// [Comment from rustc](https://github.com/rust-lang/rust/blob/4cc494bbfe9911d24f3ee521f98d5c6bb7e3ffe8/compiler/rustc_hir_typeck/src/coercion.rs#L85-L89)
// Whether we allow `NeverToAny` coercions. This is unsound if we're
// coercing a place expression without it counting as a read in the MIR.
// This is a side-effect of HIR not really having a great distinction
// between places and values.
coerce_never: CoerceNever,
) -> Result<Ty, TypeError> {
let from_ty = self.resolve_ty_shallow(from_ty);
let to_ty = self.resolve_ty_shallow(to_ty);
let (adjustments, ty) = self.table.coerce(&from_ty, &to_ty)?;
let (adjustments, ty) = self.table.coerce(&from_ty, &to_ty, coerce_never)?;
if let Some(expr) = expr {
self.write_expr_adj(expr, adjustments);
}
Expand All @@ -245,10 +257,11 @@ impl InferenceTable<'_> {
&mut self,
from_ty: &Ty,
to_ty: &Ty,
coerce_never: CoerceNever,
) -> Result<(Vec<Adjustment>, Ty), TypeError> {
let from_ty = self.resolve_ty_shallow(from_ty);
let to_ty = self.resolve_ty_shallow(to_ty);
match self.coerce_inner(from_ty, &to_ty) {
match self.coerce_inner(from_ty, &to_ty, coerce_never) {
Ok(InferOk { value: (adjustments, ty), goals }) => {
self.register_infer_ok(InferOk { value: (), goals });
Ok((adjustments, ty))
Expand All @@ -260,19 +273,23 @@ impl InferenceTable<'_> {
}
}

fn coerce_inner(&mut self, from_ty: Ty, to_ty: &Ty) -> CoerceResult {
fn coerce_inner(&mut self, from_ty: Ty, to_ty: &Ty, coerce_never: CoerceNever) -> CoerceResult {
if from_ty.is_never() {
// Subtle: If we are coercing from `!` to `?T`, where `?T` is an unbound
// type variable, we want `?T` to fallback to `!` if not
// otherwise constrained. An example where this arises:
//
// let _: Option<?T> = Some({ return; });
//
// here, we would coerce from `!` to `?T`.
if let TyKind::InferenceVar(tv, TyVariableKind::General) = to_ty.kind(Interner) {
self.set_diverging(*tv, true);
}
return success(simple(Adjust::NeverToAny)(to_ty.clone()), to_ty.clone(), vec![]);
if coerce_never == CoerceNever::Yes {
// Subtle: If we are coercing from `!` to `?T`, where `?T` is an unbound
// type variable, we want `?T` to fallback to `!` if not
// otherwise constrained. An example where this arises:
//
// let _: Option<?T> = Some({ return; });
//
// here, we would coerce from `!` to `?T`.
return success(simple(Adjust::NeverToAny)(to_ty.clone()), to_ty.clone(), vec![]);
} else {
return self.unify_and(&from_ty, to_ty, identity);
}
}

// If we are coercing into a TAIT, coerce into its proxy inference var, instead.
Expand Down
Loading