Skip to content

Commit 418c136

Browse files
committed
Auto merge of #18278 - ShoyuVanilla:never-place, r=Veykril
Do not consider match/let/ref of place that evaluates to ! to diverge, disallow coercions from them too Resolves #18237
2 parents 674c01e + 91293ea commit 418c136

File tree

8 files changed

+637
-102
lines changed

8 files changed

+637
-102
lines changed

crates/hir-ty/src/infer.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use crate::{
5757
db::HirDatabase,
5858
fold_tys,
5959
generics::Generics,
60-
infer::{coerce::CoerceMany, unify::InferenceTable},
60+
infer::{coerce::CoerceMany, expr::ExprIsRead, unify::InferenceTable},
6161
lower::ImplTraitLoweringMode,
6262
mir::MirSpan,
6363
to_assoc_type_id,
@@ -1154,6 +1154,7 @@ impl<'a> InferenceContext<'a> {
11541154
_ = self.infer_expr_coerce(
11551155
self.body.body_expr,
11561156
&Expectation::has_type(self.return_ty.clone()),
1157+
ExprIsRead::Yes,
11571158
)
11581159
}
11591160
}

crates/hir-ty/src/infer/cast.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ use hir_def::{hir::ExprId, AdtId};
55
use stdx::never;
66

77
use crate::{
8-
infer::unify::InferenceTable, Adjustment, Binders, DynTy, InferenceDiagnostic, Interner,
9-
PlaceholderIndex, QuantifiedWhereClauses, Ty, TyExt, TyKind, TypeFlags, WhereClause,
8+
infer::{coerce::CoerceNever, unify::InferenceTable},
9+
Adjustment, Binders, DynTy, InferenceDiagnostic, Interner, PlaceholderIndex,
10+
QuantifiedWhereClauses, Ty, TyExt, TyKind, TypeFlags, WhereClause,
1011
};
1112

1213
#[derive(Debug)]
@@ -128,7 +129,7 @@ impl CastCheck {
128129
return Ok(());
129130
}
130131

131-
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &self.cast_ty) {
132+
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &self.cast_ty, CoerceNever::Yes) {
132133
apply_adjustments(self.source_expr, adj);
133134
set_coercion_cast(self.source_expr);
134135
return Ok(());
@@ -154,7 +155,8 @@ impl CastCheck {
154155
let sig = self.expr_ty.callable_sig(table.db).expect("FnDef had no sig");
155156
let sig = table.normalize_associated_types_in(sig);
156157
let fn_ptr = TyKind::Function(sig.to_fn_ptr()).intern(Interner);
157-
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &fn_ptr) {
158+
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &fn_ptr, CoerceNever::Yes)
159+
{
158160
apply_adjustments(self.source_expr, adj);
159161
} else {
160162
return Err(CastError::IllegalCast);
@@ -241,7 +243,8 @@ impl CastCheck {
241243
if let TyKind::Array(ety, _) = t_expr.kind(Interner) {
242244
// Coerce to a raw pointer so that we generate RawPtr in MIR.
243245
let array_ptr_type = TyKind::Raw(m_expr, t_expr.clone()).intern(Interner);
244-
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &array_ptr_type) {
246+
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &array_ptr_type, CoerceNever::Yes)
247+
{
245248
apply_adjustments(self.source_expr, adj);
246249
} else {
247250
never!(
@@ -253,7 +256,7 @@ impl CastCheck {
253256

254257
// This is a less strict condition than rustc's `demand_eqtype`,
255258
// but false negative is better than false positive
256-
if table.coerce(ety, t_cast).is_ok() {
259+
if table.coerce(ety, t_cast, CoerceNever::Yes).is_ok() {
257260
return Ok(());
258261
}
259262
}

crates/hir-ty/src/infer/closure.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use crate::{
2929
db::{HirDatabase, InternedClosure},
3030
error_lifetime, from_chalk_trait_id, from_placeholder_idx,
3131
generics::Generics,
32+
infer::coerce::CoerceNever,
3233
make_binders,
3334
mir::{BorrowKind, MirSpan, MutBorrowKind, ProjectionElem},
3435
to_chalk_trait_id,
@@ -65,7 +66,7 @@ impl InferenceContext<'_> {
6566
}
6667

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

7071
// Coroutines are not Fn* so return early.
7172
if matches!(closure_ty.kind(Interner), TyKind::Coroutine(..)) {

crates/hir-ty/src/infer/coerce.rs

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ impl CoerceMany {
139139
};
140140
if let Some(sig) = sig {
141141
let target_ty = TyKind::Function(sig.to_fn_ptr()).intern(Interner);
142-
let result1 = ctx.table.coerce_inner(self.merged_ty(), &target_ty);
143-
let result2 = ctx.table.coerce_inner(expr_ty.clone(), &target_ty);
142+
let result1 = ctx.table.coerce_inner(self.merged_ty(), &target_ty, CoerceNever::Yes);
143+
let result2 = ctx.table.coerce_inner(expr_ty.clone(), &target_ty, CoerceNever::Yes);
144144
if let (Ok(result1), Ok(result2)) = (result1, result2) {
145145
ctx.table.register_infer_ok(InferOk { value: (), goals: result1.goals });
146146
for &e in &self.expressions {
@@ -159,9 +159,9 @@ impl CoerceMany {
159159
// type is a type variable and the new one is `!`, trying it the other
160160
// way around first would mean we make the type variable `!`, instead of
161161
// just marking it as possibly diverging.
162-
if let Ok(res) = ctx.coerce(expr, &expr_ty, &self.merged_ty()) {
162+
if let Ok(res) = ctx.coerce(expr, &expr_ty, &self.merged_ty(), CoerceNever::Yes) {
163163
self.final_ty = Some(res);
164-
} else if let Ok(res) = ctx.coerce(expr, &self.merged_ty(), &expr_ty) {
164+
} else if let Ok(res) = ctx.coerce(expr, &self.merged_ty(), &expr_ty, CoerceNever::Yes) {
165165
self.final_ty = Some(res);
166166
} else {
167167
match cause {
@@ -197,7 +197,7 @@ pub(crate) fn coerce(
197197
let vars = table.fresh_subst(tys.binders.as_slice(Interner));
198198
let ty1_with_vars = vars.apply(tys.value.0.clone(), Interner);
199199
let ty2_with_vars = vars.apply(tys.value.1.clone(), Interner);
200-
let (adjustments, ty) = table.coerce(&ty1_with_vars, &ty2_with_vars)?;
200+
let (adjustments, ty) = table.coerce(&ty1_with_vars, &ty2_with_vars, CoerceNever::Yes)?;
201201
// default any type vars that weren't unified back to their original bound vars
202202
// (kind of hacky)
203203
let find_var = |iv| {
@@ -219,6 +219,12 @@ pub(crate) fn coerce(
219219
Ok((adjustments, table.resolve_with_fallback(ty, &fallback)))
220220
}
221221

222+
#[derive(Clone, Copy, PartialEq, Eq)]
223+
pub(crate) enum CoerceNever {
224+
Yes,
225+
No,
226+
}
227+
222228
impl InferenceContext<'_> {
223229
/// Unify two types, but may coerce the first one to the second one
224230
/// using "implicit coercion rules" if needed.
@@ -227,10 +233,16 @@ impl InferenceContext<'_> {
227233
expr: Option<ExprId>,
228234
from_ty: &Ty,
229235
to_ty: &Ty,
236+
// [Comment from rustc](https://github.com/rust-lang/rust/blob/4cc494bbfe9911d24f3ee521f98d5c6bb7e3ffe8/compiler/rustc_hir_typeck/src/coercion.rs#L85-L89)
237+
// Whether we allow `NeverToAny` coercions. This is unsound if we're
238+
// coercing a place expression without it counting as a read in the MIR.
239+
// This is a side-effect of HIR not really having a great distinction
240+
// between places and values.
241+
coerce_never: CoerceNever,
230242
) -> Result<Ty, TypeError> {
231243
let from_ty = self.resolve_ty_shallow(from_ty);
232244
let to_ty = self.resolve_ty_shallow(to_ty);
233-
let (adjustments, ty) = self.table.coerce(&from_ty, &to_ty)?;
245+
let (adjustments, ty) = self.table.coerce(&from_ty, &to_ty, coerce_never)?;
234246
if let Some(expr) = expr {
235247
self.write_expr_adj(expr, adjustments);
236248
}
@@ -245,10 +257,11 @@ impl InferenceTable<'_> {
245257
&mut self,
246258
from_ty: &Ty,
247259
to_ty: &Ty,
260+
coerce_never: CoerceNever,
248261
) -> Result<(Vec<Adjustment>, Ty), TypeError> {
249262
let from_ty = self.resolve_ty_shallow(from_ty);
250263
let to_ty = self.resolve_ty_shallow(to_ty);
251-
match self.coerce_inner(from_ty, &to_ty) {
264+
match self.coerce_inner(from_ty, &to_ty, coerce_never) {
252265
Ok(InferOk { value: (adjustments, ty), goals }) => {
253266
self.register_infer_ok(InferOk { value: (), goals });
254267
Ok((adjustments, ty))
@@ -260,19 +273,23 @@ impl InferenceTable<'_> {
260273
}
261274
}
262275

263-
fn coerce_inner(&mut self, from_ty: Ty, to_ty: &Ty) -> CoerceResult {
276+
fn coerce_inner(&mut self, from_ty: Ty, to_ty: &Ty, coerce_never: CoerceNever) -> CoerceResult {
264277
if from_ty.is_never() {
265-
// Subtle: If we are coercing from `!` to `?T`, where `?T` is an unbound
266-
// type variable, we want `?T` to fallback to `!` if not
267-
// otherwise constrained. An example where this arises:
268-
//
269-
// let _: Option<?T> = Some({ return; });
270-
//
271-
// here, we would coerce from `!` to `?T`.
272278
if let TyKind::InferenceVar(tv, TyVariableKind::General) = to_ty.kind(Interner) {
273279
self.set_diverging(*tv, true);
274280
}
275-
return success(simple(Adjust::NeverToAny)(to_ty.clone()), to_ty.clone(), vec![]);
281+
if coerce_never == CoerceNever::Yes {
282+
// Subtle: If we are coercing from `!` to `?T`, where `?T` is an unbound
283+
// type variable, we want `?T` to fallback to `!` if not
284+
// otherwise constrained. An example where this arises:
285+
//
286+
// let _: Option<?T> = Some({ return; });
287+
//
288+
// here, we would coerce from `!` to `?T`.
289+
return success(simple(Adjust::NeverToAny)(to_ty.clone()), to_ty.clone(), vec![]);
290+
} else {
291+
return self.unify_and(&from_ty, to_ty, identity);
292+
}
276293
}
277294

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

0 commit comments

Comments
 (0)