Skip to content

Commit

Permalink
Auto merge of rust-lang#18278 - ShoyuVanilla:never-place, r=Veykril
Browse files Browse the repository at this point in the history
Do not consider match/let/ref of place that evaluates to ! to diverge, disallow coercions from them too

Resolves rust-lang#18237
  • Loading branch information
bors committed Oct 15, 2024
2 parents be1c31d + 3c8e146 commit 6bb6fd8
Show file tree
Hide file tree
Showing 8 changed files with 637 additions and 102 deletions.
3 changes: 2 additions & 1 deletion src/tools/rust-analyzer/crates/hir-ty/src/infer.rs
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 src/tools/rust-analyzer/crates/hir-ty/src/infer/cast.rs
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 src/tools/rust-analyzer/crates/hir-ty/src/infer/closure.rs
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 src/tools/rust-analyzer/crates/hir-ty/src/infer/coerce.rs
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

0 comments on commit 6bb6fd8

Please sign in to comment.