From 8d6b65c544602c719817d7f41f895a19b9cd1481 Mon Sep 17 00:00:00 2001 From: roife Date: Fri, 29 Mar 2024 13:13:31 +0800 Subject: [PATCH 1/3] fix: silence mismatches involving unresolved projections --- crates/hir-ty/src/chalk_ext.rs | 8 ++++++ crates/hir-ty/src/infer.rs | 51 +++++++++++++++++++++++++--------- crates/hir-ty/src/mir/lower.rs | 17 +++++++----- crates/hir/src/lib.rs | 4 +-- 4 files changed, 58 insertions(+), 22 deletions(-) diff --git a/crates/hir-ty/src/chalk_ext.rs b/crates/hir-ty/src/chalk_ext.rs index d1aebeff261c..436fb31bcc50 100644 --- a/crates/hir-ty/src/chalk_ext.rs +++ b/crates/hir-ty/src/chalk_ext.rs @@ -29,6 +29,7 @@ pub trait TyExt { fn contains_unknown(&self) -> bool; fn is_ty_var(&self) -> bool; fn is_union(&self) -> bool; + fn is_projection(&self) -> bool; fn as_adt(&self) -> Option<(hir_def::AdtId, &Substitution)>; fn as_builtin(&self) -> Option; @@ -101,6 +102,13 @@ impl TyExt for Ty { matches!(self.adt_id(Interner), Some(AdtId(hir_def::AdtId::UnionId(_)))) } + fn is_projection(&self) -> bool { + matches!( + self.kind(Interner), + TyKind::Alias(AliasTy::Projection(_)) | TyKind::AssociatedType(_, _) + ) + } + fn as_adt(&self) -> Option<(hir_def::AdtId, &Substitution)> { match self.kind(Interner) { TyKind::Adt(AdtId(adt), parameters) => Some((*adt, parameters)), diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index be3b50e1411d..880c2b4b4195 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -429,6 +429,8 @@ pub struct InferenceResult { /// Type of the result of `.into_iter()` on the for. `ExprId` is the one of the whole for loop. pub type_of_for_iterator: FxHashMap, type_mismatches: FxHashMap, + /// Whether there are any type-mismatching errors in the result. + pub(crate) has_errors: bool, /// Interned common types to return references to. standard_types: InternedStandardTypes, /// Stores the types which were implicitly dereferenced in pattern binding modes. @@ -654,6 +656,7 @@ impl<'a> InferenceContext<'a> { type_of_rpit, type_of_for_iterator, type_mismatches, + has_errors, standard_types: _, pat_adjustments, binding_modes: _, @@ -695,16 +698,33 @@ impl<'a> InferenceContext<'a> { for ty in type_of_for_iterator.values_mut() { *ty = table.resolve_completely(ty.clone()); } + + *has_errors = !type_mismatches.is_empty(); + type_mismatches.retain(|_, mismatch| { mismatch.expected = table.resolve_completely(mismatch.expected.clone()); mismatch.actual = table.resolve_completely(mismatch.actual.clone()); - chalk_ir::zip::Zip::zip_with( - &mut UnknownMismatch(self.db), - Variance::Invariant, - &mismatch.expected, - &mismatch.actual, - ) - .is_ok() + let unresolved_ty_mismatch = || { + chalk_ir::zip::Zip::zip_with( + &mut UnknownMismatch(self.db, |ty| matches!(ty.kind(Interner), TyKind::Error)), + Variance::Invariant, + &mismatch.expected, + &mismatch.actual, + ) + .is_ok() + }; + + let unresolved_projections_mismatch = || { + chalk_ir::zip::Zip::zip_with( + &mut UnknownMismatch(self.db, |ty| ty.contains_unknown() && ty.is_projection()), + chalk_ir::Variance::Invariant, + &mismatch.expected, + &mismatch.actual, + ) + .is_ok() + }; + + unresolved_ty_mismatch() && unresolved_projections_mismatch() }); diagnostics.retain_mut(|diagnostic| { use InferenceDiagnostic::*; @@ -1646,11 +1666,16 @@ impl std::ops::BitOrAssign for Diverges { *self = *self | other; } } -/// A zipper that checks for unequal `{unknown}` occurrences in the two types. Used to filter out -/// mismatch diagnostics that only differ in `{unknown}`. These mismatches are usually not helpful. -/// As the cause is usually an underlying name resolution problem. -struct UnknownMismatch<'db>(&'db dyn HirDatabase); -impl chalk_ir::zip::Zipper for UnknownMismatch<'_> { +/// A zipper that checks for unequal `{unknown}` occurrences in the two types. +/// Types that have different constructors are filtered out and tested by the +/// provided closure `F`. Commonly used to filter out mismatch diagnostics that +/// only differ in `{unknown}`. These mismatches are usually not helpful, as the +/// cause is usually an underlying name resolution problem. +/// +/// E.g. when F is `|ty| matches!(ty.kind(Interer), TyKind::Unknown)`, the zipper +/// will skip over all mismatches that only differ in `{unknown}`. +struct UnknownMismatch<'db, F: Fn(&Ty) -> bool>(&'db dyn HirDatabase, F); +impl bool> chalk_ir::zip::Zipper for UnknownMismatch<'_, F> { fn zip_tys(&mut self, variance: Variance, a: &Ty, b: &Ty) -> chalk_ir::Fallible<()> { let zip_substs = |this: &mut Self, variances, @@ -1721,7 +1746,7 @@ impl chalk_ir::zip::Zipper for UnknownMismatch<'_> { zip_substs(self, None, &fn_ptr_a.substitution.0, &fn_ptr_b.substitution.0)? } (TyKind::Error, TyKind::Error) => (), - (TyKind::Error, _) | (_, TyKind::Error) => return Err(chalk_ir::NoSolution), + _ if (self.1)(a) || (self.1)(b) => return Err(chalk_ir::NoSolution), _ => (), } diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index 7e582c03efcd..f2505c193777 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -90,7 +90,7 @@ pub enum MirLowerError { UnresolvedField, UnsizedTemporary(Ty), MissingFunctionDefinition(DefWithBodyId, ExprId), - TypeMismatch(TypeMismatch), + TypeMismatch(Option), /// This should never happen. Type mismatch should catch everything. TypeError(&'static str), NotSupported(String), @@ -170,14 +170,15 @@ impl MirLowerError { body.pretty_print_expr(db.upcast(), *owner, *it) )?; } - MirLowerError::TypeMismatch(e) => { - writeln!( + MirLowerError::TypeMismatch(e) => match e { + Some(e) => writeln!( f, "Type mismatch: Expected {}, found {}", e.expected.display(db), e.actual.display(db), - )?; - } + )?, + None => writeln!(f, "Type mismatch: types mismatch with {{unknown}}",)?, + }, MirLowerError::GenericArgNotProvided(id, subst) => { let parent = id.parent; let param = &db.generic_params(parent).type_or_consts[id.local_id]; @@ -2152,8 +2153,10 @@ pub fn lower_to_mir( // need to take this input explicitly. root_expr: ExprId, ) -> Result { - if let Some((_, it)) = infer.type_mismatches().next() { - return Err(MirLowerError::TypeMismatch(it.clone())); + if infer.has_errors { + return Err(MirLowerError::TypeMismatch( + infer.type_mismatches().next().map(|(_, it)| it.clone()), + )); } let mut ctx = MirLowerCtx::new(db, owner, body, infer); // 0 is return local diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index b4388ad9656f..1d4d458e89e4 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -73,8 +73,7 @@ use hir_ty::{ traits::FnTrait, AliasTy, CallableDefId, CallableSig, Canonical, CanonicalVarKinds, Cast, ClosureId, GenericArg, GenericArgData, Interner, ParamKind, QuantifiedWhereClause, Scalar, Substitution, - TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyDefId, TyExt, TyKind, ValueTyDefId, - WhereClause, + TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyDefId, TyExt, TyKind, ValueTyDefId, WhereClause, }; use itertools::Itertools; use nameres::diagnostics::DefDiagnosticKind; @@ -1678,6 +1677,7 @@ impl DefWithBody { for d in &infer.diagnostics { acc.extend(AnyDiagnostic::inference_diagnostic(db, self.into(), d, &source_map)); } + for (pat_or_expr, mismatch) in infer.type_mismatches() { let expr_or_pat = match pat_or_expr { ExprOrPatId::ExprId(expr) => source_map.expr_syntax(expr).map(Either::Left), From 3d373fec8c2cbaa025a2db5c519ecc89cd1e95d9 Mon Sep 17 00:00:00 2001 From: roife Date: Tue, 2 Apr 2024 01:32:39 +0800 Subject: [PATCH 2/3] tests: add tests for mismatches with unresolved projections --- crates/hir-ty/src/tests/diagnostics.rs | 17 +++++++++++++++++ crates/hir/src/lib.rs | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/hir-ty/src/tests/diagnostics.rs b/crates/hir-ty/src/tests/diagnostics.rs index 80f92eaf4355..fde90b0a55f6 100644 --- a/crates/hir-ty/src/tests/diagnostics.rs +++ b/crates/hir-ty/src/tests/diagnostics.rs @@ -136,3 +136,20 @@ impl Trait for () { "#, ); } + +#[test] +fn no_mismatches_with_unresolved_projections() { + check_no_mismatches( + r#" +// Thing is {unknown} +fn create() -> Option<(i32, Thing)> { + Some((69420, Thing)) +} + +fn consume() -> Option<()> { + let (number, thing) = create()?; + Some(()) +} +"#, + ); +} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 1d4d458e89e4..b9f7ff67a00a 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -73,7 +73,8 @@ use hir_ty::{ traits::FnTrait, AliasTy, CallableDefId, CallableSig, Canonical, CanonicalVarKinds, Cast, ClosureId, GenericArg, GenericArgData, Interner, ParamKind, QuantifiedWhereClause, Scalar, Substitution, - TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyDefId, TyExt, TyKind, ValueTyDefId, WhereClause, + TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyDefId, TyExt, TyKind, ValueTyDefId, + WhereClause, }; use itertools::Itertools; use nameres::diagnostics::DefDiagnosticKind; From 2636e44378c4e1bf674e9f71f7ec672909359832 Mon Sep 17 00:00:00 2001 From: roife Date: Tue, 2 Apr 2024 02:29:36 +0800 Subject: [PATCH 3/3] fix: simplify the usage of UnknownMismatch --- crates/hir-ty/src/chalk_ext.rs | 8 ---- crates/hir-ty/src/infer.rs | 52 ++++++++++---------------- crates/hir-ty/src/tests/diagnostics.rs | 2 +- 3 files changed, 21 insertions(+), 41 deletions(-) diff --git a/crates/hir-ty/src/chalk_ext.rs b/crates/hir-ty/src/chalk_ext.rs index 436fb31bcc50..d1aebeff261c 100644 --- a/crates/hir-ty/src/chalk_ext.rs +++ b/crates/hir-ty/src/chalk_ext.rs @@ -29,7 +29,6 @@ pub trait TyExt { fn contains_unknown(&self) -> bool; fn is_ty_var(&self) -> bool; fn is_union(&self) -> bool; - fn is_projection(&self) -> bool; fn as_adt(&self) -> Option<(hir_def::AdtId, &Substitution)>; fn as_builtin(&self) -> Option; @@ -102,13 +101,6 @@ impl TyExt for Ty { matches!(self.adt_id(Interner), Some(AdtId(hir_def::AdtId::UnionId(_)))) } - fn is_projection(&self) -> bool { - matches!( - self.kind(Interner), - TyKind::Alias(AliasTy::Projection(_)) | TyKind::AssociatedType(_, _) - ) - } - fn as_adt(&self) -> Option<(hir_def::AdtId, &Substitution)> { match self.kind(Interner) { TyKind::Adt(AdtId(adt), parameters) => Some((*adt, parameters)), diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 880c2b4b4195..bb558e533c4e 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -704,27 +704,13 @@ impl<'a> InferenceContext<'a> { type_mismatches.retain(|_, mismatch| { mismatch.expected = table.resolve_completely(mismatch.expected.clone()); mismatch.actual = table.resolve_completely(mismatch.actual.clone()); - let unresolved_ty_mismatch = || { - chalk_ir::zip::Zip::zip_with( - &mut UnknownMismatch(self.db, |ty| matches!(ty.kind(Interner), TyKind::Error)), - Variance::Invariant, - &mismatch.expected, - &mismatch.actual, - ) - .is_ok() - }; - - let unresolved_projections_mismatch = || { - chalk_ir::zip::Zip::zip_with( - &mut UnknownMismatch(self.db, |ty| ty.contains_unknown() && ty.is_projection()), - chalk_ir::Variance::Invariant, - &mismatch.expected, - &mismatch.actual, - ) - .is_ok() - }; - - unresolved_ty_mismatch() && unresolved_projections_mismatch() + chalk_ir::zip::Zip::zip_with( + &mut UnknownMismatch(self.db), + Variance::Invariant, + &mismatch.expected, + &mismatch.actual, + ) + .is_ok() }); diagnostics.retain_mut(|diagnostic| { use InferenceDiagnostic::*; @@ -1666,16 +1652,13 @@ impl std::ops::BitOrAssign for Diverges { *self = *self | other; } } -/// A zipper that checks for unequal `{unknown}` occurrences in the two types. -/// Types that have different constructors are filtered out and tested by the -/// provided closure `F`. Commonly used to filter out mismatch diagnostics that -/// only differ in `{unknown}`. These mismatches are usually not helpful, as the -/// cause is usually an underlying name resolution problem. -/// -/// E.g. when F is `|ty| matches!(ty.kind(Interer), TyKind::Unknown)`, the zipper -/// will skip over all mismatches that only differ in `{unknown}`. -struct UnknownMismatch<'db, F: Fn(&Ty) -> bool>(&'db dyn HirDatabase, F); -impl bool> chalk_ir::zip::Zipper for UnknownMismatch<'_, F> { + +/// A zipper that checks for unequal occurrences of `{unknown}` and unresolved projections +/// in the two types. Used to filter out mismatch diagnostics that only differ in +/// `{unknown}` and unresolved projections. These mismatches are usually not helpful. +/// As the cause is usually an underlying name resolution problem +struct UnknownMismatch<'db>(&'db dyn HirDatabase); +impl chalk_ir::zip::Zipper for UnknownMismatch<'_> { fn zip_tys(&mut self, variance: Variance, a: &Ty, b: &Ty) -> chalk_ir::Fallible<()> { let zip_substs = |this: &mut Self, variances, @@ -1746,7 +1729,12 @@ impl bool> chalk_ir::zip::Zipper for UnknownMismatch<'_, zip_substs(self, None, &fn_ptr_a.substitution.0, &fn_ptr_b.substitution.0)? } (TyKind::Error, TyKind::Error) => (), - _ if (self.1)(a) || (self.1)(b) => return Err(chalk_ir::NoSolution), + (TyKind::Error, _) + | (_, TyKind::Error) + | (TyKind::Alias(AliasTy::Projection(_)) | TyKind::AssociatedType(_, _), _) + | (_, TyKind::Alias(AliasTy::Projection(_)) | TyKind::AssociatedType(_, _)) => { + return Err(chalk_ir::NoSolution) + } _ => (), } diff --git a/crates/hir-ty/src/tests/diagnostics.rs b/crates/hir-ty/src/tests/diagnostics.rs index fde90b0a55f6..119de7f050e7 100644 --- a/crates/hir-ty/src/tests/diagnostics.rs +++ b/crates/hir-ty/src/tests/diagnostics.rs @@ -141,7 +141,7 @@ impl Trait for () { fn no_mismatches_with_unresolved_projections() { check_no_mismatches( r#" -// Thing is {unknown} +// `Thing` is `{unknown}` fn create() -> Option<(i32, Thing)> { Some((69420, Thing)) }