Auto merge of #14240 - Veykril:coerce-many, r=Veykril

Diagnose value breaks in incorrect breakables
This commit is contained in:
bors 2023-03-03 17:03:39 +00:00
commit 3ba876a4a6
6 changed files with 250 additions and 118 deletions

View file

@ -66,8 +66,10 @@ pub(crate) fn infer_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc<Infer
let mut ctx = InferenceContext::new(db, def, &body, resolver);
match def {
DefWithBodyId::FunctionId(f) => {
ctx.collect_fn(f);
}
DefWithBodyId::ConstId(c) => ctx.collect_const(&db.const_data(c)),
DefWithBodyId::FunctionId(f) => ctx.collect_fn(f),
DefWithBodyId::StaticId(s) => ctx.collect_static(&db.static_data(s)),
DefWithBodyId::VariantId(v) => {
ctx.return_ty = TyBuilder::builtin(match db.enum_data(v.parent).variant_body_type() {
@ -165,7 +167,8 @@ pub enum InferenceDiagnostic {
NoSuchField { expr: ExprId },
PrivateField { expr: ExprId, field: FieldId },
PrivateAssocItem { id: ExprOrPatId, item: AssocItemId },
BreakOutsideOfLoop { expr: ExprId, is_break: bool },
// FIXME: Make this proper
BreakOutsideOfLoop { expr: ExprId, is_break: bool, bad_value_break: bool },
MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize },
}
@ -392,9 +395,12 @@ pub(crate) struct InferenceContext<'a> {
/// currently within one.
///
/// We might consider using a nested inference context for checking
/// closures, but currently this is the only field that will change there,
/// so it doesn't make sense.
/// closures so we can swap all shared things out at once.
return_ty: Ty,
/// If `Some`, this stores coercion information for returned
/// expressions. If `None`, this is in a context where return is
/// inappropriate, such as a const expression.
return_coercion: Option<CoerceMany>,
/// The resume type and the yield type, respectively, of the generator being inferred.
resume_yield_tys: Option<(Ty, Ty)>,
diverges: Diverges,
@ -406,7 +412,7 @@ struct BreakableContext {
/// Whether this context contains at least one break expression.
may_break: bool,
/// The coercion target of the context.
coerce: CoerceMany,
coerce: Option<CoerceMany>,
/// The optional label of the context.
label: Option<name::Name>,
kind: BreakableKind,
@ -462,6 +468,7 @@ impl<'a> InferenceContext<'a> {
trait_env,
return_ty: TyKind::Error.intern(Interner), // set in collect_* calls
resume_yield_tys: None,
return_coercion: None,
db,
owner,
body,
@ -595,10 +602,19 @@ impl<'a> InferenceContext<'a> {
};
self.return_ty = self.normalize_associated_types_in(return_ty);
self.return_coercion = Some(CoerceMany::new(self.return_ty.clone()));
}
fn infer_body(&mut self) {
self.infer_expr_coerce(self.body.body_expr, &Expectation::has_type(self.return_ty.clone()));
match self.return_coercion {
Some(_) => self.infer_return(self.body.body_expr),
None => {
_ = self.infer_expr_coerce(
self.body.body_expr,
&Expectation::has_type(self.return_ty.clone()),
)
}
}
}
fn write_expr_ty(&mut self, expr: ExprId, ty: Ty) {

View file

@ -50,11 +50,44 @@ fn success(
#[derive(Clone, Debug)]
pub(super) struct CoerceMany {
expected_ty: Ty,
final_ty: Option<Ty>,
}
impl CoerceMany {
pub(super) fn new(expected: Ty) -> Self {
CoerceMany { expected_ty: expected }
CoerceMany { expected_ty: expected, final_ty: None }
}
/// Returns the "expected type" with which this coercion was
/// constructed. This represents the "downward propagated" type
/// that was given to us at the start of typing whatever construct
/// we are typing (e.g., the match expression).
///
/// Typically, this is used as the expected type when
/// type-checking each of the alternative expressions whose types
/// we are trying to merge.
pub(super) fn expected_ty(&self) -> Ty {
self.expected_ty.clone()
}
/// Returns the current "merged type", representing our best-guess
/// at the LUB of the expressions we've seen so far (if any). This
/// isn't *final* until you call `self.complete()`, which will return
/// the merged type.
pub(super) fn merged_ty(&self) -> Ty {
self.final_ty.clone().unwrap_or_else(|| self.expected_ty.clone())
}
pub(super) fn complete(self, ctx: &mut InferenceContext<'_>) -> Ty {
if let Some(final_ty) = self.final_ty {
final_ty
} else {
ctx.result.standard_types.never.clone()
}
}
pub(super) fn coerce_forced_unit(&mut self, ctx: &mut InferenceContext<'_>) {
self.coerce(ctx, None, &ctx.result.standard_types.unit.clone())
}
/// Merge two types from different branches, with possible coercion.
@ -76,25 +109,25 @@ impl CoerceMany {
// Special case: two function types. Try to coerce both to
// pointers to have a chance at getting a match. See
// https://github.com/rust-lang/rust/blob/7b805396bf46dce972692a6846ce2ad8481c5f85/src/librustc_typeck/check/coercion.rs#L877-L916
let sig = match (self.expected_ty.kind(Interner), expr_ty.kind(Interner)) {
let sig = match (self.merged_ty().kind(Interner), expr_ty.kind(Interner)) {
(TyKind::FnDef(..) | TyKind::Closure(..), TyKind::FnDef(..) | TyKind::Closure(..)) => {
// FIXME: we're ignoring safety here. To be more correct, if we have one FnDef and one Closure,
// we should be coercing the closure to a fn pointer of the safety of the FnDef
cov_mark::hit!(coerce_fn_reification);
let sig =
self.expected_ty.callable_sig(ctx.db).expect("FnDef without callable sig");
self.merged_ty().callable_sig(ctx.db).expect("FnDef without callable sig");
Some(sig)
}
_ => None,
};
if let Some(sig) = sig {
let target_ty = TyKind::Function(sig.to_fn_ptr()).intern(Interner);
let result1 = ctx.table.coerce_inner(self.expected_ty.clone(), &target_ty);
let result1 = ctx.table.coerce_inner(self.merged_ty(), &target_ty);
let result2 = ctx.table.coerce_inner(expr_ty.clone(), &target_ty);
if let (Ok(result1), Ok(result2)) = (result1, result2) {
ctx.table.register_infer_ok(result1);
ctx.table.register_infer_ok(result2);
return self.expected_ty = target_ty;
return self.final_ty = Some(target_ty);
}
}
@ -102,25 +135,20 @@ 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 ctx.coerce(expr, &expr_ty, &self.expected_ty).is_ok() {
/* self.expected_ty is already correct */
} else if ctx.coerce(expr, &self.expected_ty, &expr_ty).is_ok() {
self.expected_ty = expr_ty;
if let Ok(res) = ctx.coerce(expr, &expr_ty, &self.merged_ty()) {
self.final_ty = Some(res);
} else if let Ok(res) = ctx.coerce(expr, &self.merged_ty(), &expr_ty) {
self.final_ty = Some(res);
} else {
if let Some(id) = expr {
ctx.result.type_mismatches.insert(
id.into(),
TypeMismatch { expected: self.expected_ty.clone(), actual: expr_ty },
TypeMismatch { expected: self.merged_ty().clone(), actual: expr_ty.clone() },
);
}
cov_mark::hit!(coerce_merge_fail_fallback);
/* self.expected_ty is already correct */
}
}
pub(super) fn complete(self) -> Ty {
self.expected_ty
}
}
pub fn could_coerce(

View file

@ -60,6 +60,10 @@ impl<'a> InferenceContext<'a> {
ty
}
pub(crate) fn infer_expr_no_expect(&mut self, tgt_expr: ExprId) -> Ty {
self.infer_expr_inner(tgt_expr, &Expectation::None)
}
/// Infer type of expression with possibly implicit coerce to the expected type.
/// Return the type after possible coercion.
pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty {
@ -99,17 +103,20 @@ impl<'a> InferenceContext<'a> {
both_arms_diverge &= mem::replace(&mut self.diverges, Diverges::Maybe);
let mut coerce = CoerceMany::new(expected.coercion_target_type(&mut self.table));
coerce.coerce(self, Some(then_branch), &then_ty);
let else_ty = match else_branch {
Some(else_branch) => self.infer_expr_inner(else_branch, expected),
None => TyBuilder::unit(),
};
match else_branch {
Some(else_branch) => {
let else_ty = self.infer_expr_inner(else_branch, expected);
coerce.coerce(self, Some(else_branch), &else_ty);
}
None => {
coerce.coerce_forced_unit(self);
}
}
both_arms_diverge &= self.diverges;
// FIXME: create a synthetic `else {}` so we have something to refer to here instead of None?
coerce.coerce(self, else_branch, &else_ty);
self.diverges = condition_diverges | both_arms_diverge;
coerce.complete()
coerce.complete(self)
}
&Expr::Let { pat, expr } => {
let input_ty = self.infer_expr(expr, &Expectation::none());
@ -126,7 +133,7 @@ impl<'a> InferenceContext<'a> {
let break_ty = self.table.new_type_var();
let (breaks, ty) = self.with_breakable_ctx(
BreakableKind::Block,
break_ty.clone(),
Some(break_ty.clone()),
*label,
|this| {
this.infer_block(
@ -146,7 +153,7 @@ impl<'a> InferenceContext<'a> {
}
Expr::Unsafe { body } => self.infer_expr(*body, expected),
Expr::Const { body } => {
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
self.with_breakable_ctx(BreakableKind::Border, None, None, |this| {
this.infer_expr(*body, expected)
})
.1
@ -162,7 +169,7 @@ impl<'a> InferenceContext<'a> {
let ok_ty =
self.resolve_associated_type(try_ty.clone(), self.resolve_ops_try_output());
self.with_breakable_ctx(BreakableKind::Block, ok_ty.clone(), None, |this| {
self.with_breakable_ctx(BreakableKind::Block, Some(ok_ty.clone()), None, |this| {
this.infer_expr(*body, &Expectation::has_type(ok_ty));
});
@ -172,14 +179,17 @@ impl<'a> InferenceContext<'a> {
let ret_ty = self.table.new_type_var();
let prev_diverges = mem::replace(&mut self.diverges, Diverges::Maybe);
let prev_ret_ty = mem::replace(&mut self.return_ty, ret_ty.clone());
let prev_ret_coercion =
mem::replace(&mut self.return_coercion, Some(CoerceMany::new(ret_ty.clone())));
let (_, inner_ty) =
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
self.with_breakable_ctx(BreakableKind::Border, None, None, |this| {
this.infer_expr_coerce(*body, &Expectation::has_type(ret_ty))
});
self.diverges = prev_diverges;
self.return_ty = prev_ret_ty;
self.return_coercion = prev_ret_coercion;
// Use the first type parameter as the output type of future.
// existential type AsyncBlockImplTrait<InnerType>: Future<Output = InnerType>
@ -193,7 +203,7 @@ impl<'a> InferenceContext<'a> {
// let ty = expected.coercion_target_type(&mut self.table);
let ty = self.table.new_type_var();
let (breaks, ()) =
self.with_breakable_ctx(BreakableKind::Loop, ty, label, |this| {
self.with_breakable_ctx(BreakableKind::Loop, Some(ty), label, |this| {
this.infer_expr(body, &Expectation::HasType(TyBuilder::unit()));
});
@ -206,7 +216,7 @@ impl<'a> InferenceContext<'a> {
}
}
&Expr::While { condition, body, label } => {
self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| {
self.with_breakable_ctx(BreakableKind::Loop, None, label, |this| {
this.infer_expr(
condition,
&Expectation::HasType(this.result.standard_types.bool_.clone()),
@ -226,7 +236,7 @@ impl<'a> InferenceContext<'a> {
self.resolve_associated_type(into_iter_ty, self.resolve_iterator_item());
self.infer_top_pat(pat, &pat_ty);
self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| {
self.with_breakable_ctx(BreakableKind::Loop, None, label, |this| {
this.infer_expr(body, &Expectation::HasType(TyBuilder::unit()));
});
@ -303,17 +313,21 @@ impl<'a> InferenceContext<'a> {
self.infer_top_pat(*arg_pat, &arg_ty);
}
// FIXME: lift these out into a struct
let prev_diverges = mem::replace(&mut self.diverges, Diverges::Maybe);
let prev_ret_ty = mem::replace(&mut self.return_ty, ret_ty.clone());
let prev_ret_coercion =
mem::replace(&mut self.return_coercion, Some(CoerceMany::new(ret_ty.clone())));
let prev_resume_yield_tys =
mem::replace(&mut self.resume_yield_tys, resume_yield_tys);
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
this.infer_expr_coerce(*body, &Expectation::has_type(ret_ty));
self.with_breakable_ctx(BreakableKind::Border, None, None, |this| {
this.infer_return(*body);
});
self.diverges = prev_diverges;
self.return_ty = prev_ret_ty;
self.return_coercion = prev_ret_coercion;
self.resume_yield_tys = prev_resume_yield_tys;
ty
@ -413,7 +427,7 @@ impl<'a> InferenceContext<'a> {
self.diverges = matchee_diverges | all_arms_diverge;
coerce.complete()
coerce.complete(self)
}
Expr::Path(p) => {
// FIXME this could be more efficient...
@ -425,51 +439,56 @@ impl<'a> InferenceContext<'a> {
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
expr: tgt_expr,
is_break: false,
bad_value_break: false,
});
};
self.result.standard_types.never.clone()
}
Expr::Break { expr, label } => {
let val_ty = if let Some(expr) = *expr {
self.infer_expr(expr, &Expectation::none())
let opt_coerce_to = match find_breakable(&mut self.breakables, label.as_ref()) {
Some(ctxt) => match &ctxt.coerce {
Some(coerce) => coerce.expected_ty(),
None => {
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
expr: tgt_expr,
is_break: true,
bad_value_break: true,
});
self.err_ty()
}
},
None => self.err_ty(),
};
self.infer_expr_inner(expr, &Expectation::HasType(opt_coerce_to))
} else {
TyBuilder::unit()
};
match find_breakable(&mut self.breakables, label.as_ref()) {
Some(ctxt) => {
// avoiding the borrowck
let mut coerce = mem::replace(
&mut ctxt.coerce,
CoerceMany::new(expected.coercion_target_type(&mut self.table)),
);
// FIXME: create a synthetic `()` during lowering so we have something to refer to here?
Some(ctxt) => match ctxt.coerce.take() {
Some(mut coerce) => {
coerce.coerce(self, *expr, &val_ty);
// Avoiding borrowck
let ctxt = find_breakable(&mut self.breakables, label.as_ref())
.expect("breakable stack changed during coercion");
ctxt.coerce = coerce;
ctxt.may_break = true;
ctxt.coerce = Some(coerce);
}
None => ctxt.may_break = true,
},
None => {
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
expr: tgt_expr,
is_break: true,
bad_value_break: false,
});
}
}
self.result.standard_types.never.clone()
}
Expr::Return { expr } => {
if let Some(expr) = expr {
self.infer_expr_coerce(*expr, &Expectation::has_type(self.return_ty.clone()));
} else {
let unit = TyBuilder::unit();
let _ = self.coerce(Some(tgt_expr), &unit, &self.return_ty.clone());
}
self.result.standard_types.never.clone()
}
&Expr::Return { expr } => self.infer_expr_return(expr),
Expr::Yield { expr } => {
if let Some((resume_ty, yield_ty)) = self.resume_yield_tys.clone() {
if let Some(expr) = expr {
@ -486,7 +505,7 @@ impl<'a> InferenceContext<'a> {
}
Expr::Yeet { expr } => {
if let &Some(expr) = expr {
self.infer_expr_inner(expr, &Expectation::None);
self.infer_expr_no_expect(expr);
}
self.result.standard_types.never.clone()
}
@ -614,7 +633,7 @@ impl<'a> InferenceContext<'a> {
Expr::Cast { expr, type_ref } => {
let cast_ty = self.make_ty(type_ref);
// FIXME: propagate the "castable to" expectation
let _inner_ty = self.infer_expr_inner(*expr, &Expectation::None);
let _inner_ty = self.infer_expr_no_expect(*expr);
// FIXME check the cast...
cast_ty
}
@ -810,53 +829,7 @@ impl<'a> InferenceContext<'a> {
TyKind::Tuple(tys.len(), Substitution::from_iter(Interner, tys)).intern(Interner)
}
Expr::Array(array) => {
let elem_ty =
match expected.to_option(&mut self.table).as_ref().map(|t| t.kind(Interner)) {
Some(TyKind::Array(st, _) | TyKind::Slice(st)) => st.clone(),
_ => self.table.new_type_var(),
};
let mut coerce = CoerceMany::new(elem_ty.clone());
let expected = Expectation::has_type(elem_ty.clone());
let len = match array {
Array::ElementList { elements, .. } => {
for &expr in elements.iter() {
let cur_elem_ty = self.infer_expr_inner(expr, &expected);
coerce.coerce(self, Some(expr), &cur_elem_ty);
}
consteval::usize_const(
self.db,
Some(elements.len() as u128),
self.resolver.krate(),
)
}
&Array::Repeat { initializer, repeat } => {
self.infer_expr_coerce(initializer, &Expectation::has_type(elem_ty));
self.infer_expr(
repeat,
&Expectation::HasType(
TyKind::Scalar(Scalar::Uint(UintTy::Usize)).intern(Interner),
),
);
if let Some(g_def) = self.owner.as_generic_def_id() {
let generics = generics(self.db.upcast(), g_def);
consteval::eval_to_const(
repeat,
ParamLoweringMode::Placeholder,
self,
|| generics,
DebruijnIndex::INNERMOST,
)
} else {
consteval::usize_const(self.db, None, self.resolver.krate())
}
}
};
TyKind::Array(coerce.complete(), len).intern(Interner)
}
Expr::Array(array) => self.infer_expr_array(array, expected),
Expr::Literal(lit) => match lit {
Literal::Bool(..) => self.result.standard_types.bool_.clone(),
Literal::String(..) => {
@ -915,6 +888,97 @@ impl<'a> InferenceContext<'a> {
ty
}
fn infer_expr_array(
&mut self,
array: &Array,
expected: &Expectation,
) -> chalk_ir::Ty<Interner> {
let elem_ty = match expected.to_option(&mut self.table).as_ref().map(|t| t.kind(Interner)) {
Some(TyKind::Array(st, _) | TyKind::Slice(st)) => st.clone(),
_ => self.table.new_type_var(),
};
let krate = self.resolver.krate();
let expected = Expectation::has_type(elem_ty.clone());
let (elem_ty, len) = match array {
Array::ElementList { elements, .. } if elements.is_empty() => {
(elem_ty, consteval::usize_const(self.db, Some(0), krate))
}
Array::ElementList { elements, .. } => {
let mut coerce = CoerceMany::new(elem_ty.clone());
for &expr in elements.iter() {
let cur_elem_ty = self.infer_expr_inner(expr, &expected);
coerce.coerce(self, Some(expr), &cur_elem_ty);
}
(
coerce.complete(self),
consteval::usize_const(self.db, Some(elements.len() as u128), krate),
)
}
&Array::Repeat { initializer, repeat } => {
self.infer_expr_coerce(initializer, &Expectation::has_type(elem_ty.clone()));
self.infer_expr(
repeat,
&Expectation::HasType(
TyKind::Scalar(Scalar::Uint(UintTy::Usize)).intern(Interner),
),
);
(
elem_ty,
if let Some(g_def) = self.owner.as_generic_def_id() {
let generics = generics(self.db.upcast(), g_def);
consteval::eval_to_const(
repeat,
ParamLoweringMode::Placeholder,
self,
|| generics,
DebruijnIndex::INNERMOST,
)
} else {
consteval::usize_const(self.db, None, krate)
},
)
}
};
TyKind::Array(elem_ty, len).intern(Interner)
}
pub(super) fn infer_return(&mut self, expr: ExprId) {
let ret_ty = self
.return_coercion
.as_mut()
.expect("infer_return called outside function body")
.expected_ty();
let return_expr_ty = self.infer_expr_inner(expr, &Expectation::HasType(ret_ty));
let mut coerce_many = self.return_coercion.take().unwrap();
coerce_many.coerce(self, Some(expr), &return_expr_ty);
self.return_coercion = Some(coerce_many);
}
fn infer_expr_return(&mut self, expr: Option<ExprId>) -> Ty {
match self.return_coercion {
Some(_) => {
if let Some(expr) = expr {
self.infer_return(expr);
} else {
let mut coerce = self.return_coercion.take().unwrap();
coerce.coerce_forced_unit(self);
self.return_coercion = Some(coerce);
}
}
None => {
// FIXME: diagnose return outside of function
if let Some(expr) = expr {
self.infer_expr_no_expect(expr);
}
}
}
self.result.standard_types.never.clone()
}
fn infer_expr_box(&mut self, inner_expr: ExprId, expected: &Expectation) -> Ty {
if let Some(box_id) = self.resolve_boxed_box() {
let table = &mut self.table;
@ -1656,16 +1720,16 @@ impl<'a> InferenceContext<'a> {
fn with_breakable_ctx<T>(
&mut self,
kind: BreakableKind,
ty: Ty,
ty: Option<Ty>,
label: Option<LabelId>,
cb: impl FnOnce(&mut Self) -> T,
) -> (Option<Ty>, T) {
self.breakables.push({
let label = label.map(|label| self.body[label].name.clone());
BreakableContext { kind, may_break: false, coerce: CoerceMany::new(ty), label }
BreakableContext { kind, may_break: false, coerce: ty.map(CoerceMany::new), label }
});
let res = cb(self);
let ctx = self.breakables.pop().expect("breakable stack broken");
(ctx.may_break.then(|| ctx.coerce.complete()), res)
(if ctx.may_break { ctx.coerce.map(|ctx| ctx.complete(self)) } else { None }, res)
}
}

View file

@ -140,6 +140,7 @@ pub struct PrivateField {
pub struct BreakOutsideOfLoop {
pub expr: InFile<AstPtr<ast::Expr>>,
pub is_break: bool,
pub bad_value_break: bool,
}
#[derive(Debug)]

View file

@ -1381,11 +1381,15 @@ impl DefWithBody {
let field = source_map.field_syntax(*expr);
acc.push(NoSuchField { field }.into())
}
&hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { expr, is_break } => {
&hir_ty::InferenceDiagnostic::BreakOutsideOfLoop {
expr,
is_break,
bad_value_break,
} => {
let expr = source_map
.expr_syntax(expr)
.expect("break outside of loop in synthetic syntax");
acc.push(BreakOutsideOfLoop { expr, is_break }.into())
acc.push(BreakOutsideOfLoop { expr, is_break, bad_value_break }.into())
}
hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => {
match source_map.expr_syntax(*call_expr) {

View file

@ -7,10 +7,15 @@ pub(crate) fn break_outside_of_loop(
ctx: &DiagnosticsContext<'_>,
d: &hir::BreakOutsideOfLoop,
) -> Diagnostic {
let message = if d.bad_value_break {
"can't break with a value in this position".to_owned()
} else {
let construct = if d.is_break { "break" } else { "continue" };
format!("{construct} outside of loop")
};
Diagnostic::new(
"break-outside-of-loop",
format!("{construct} outside of loop"),
message,
ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range,
)
}
@ -132,6 +137,20 @@ fn foo() {
//^^^^^^^^^^^ error: continue outside of loop
}
}
"#,
);
}
#[test]
fn value_break_in_for_loop() {
check_diagnostics(
r#"
fn test() {
for _ in [()] {
break 3;
// ^^^^^^^ error: can't break with a value in this position
}
}
"#,
);
}