From 8f4f5a6ccea129f6858ff862caa4e370633778bb Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 16 Jan 2024 12:09:40 +0100 Subject: [PATCH] fix: Make value_ty query fallible --- crates/hir-def/src/db.rs | 16 +------------ crates/hir-ty/src/db.rs | 4 +++- crates/hir-ty/src/infer/expr.rs | 6 ++--- crates/hir-ty/src/infer/path.rs | 4 ++-- crates/hir-ty/src/lower.rs | 33 +++++++++++++++------------ crates/hir-ty/src/tests/regression.rs | 28 +++++++++++++++++++++++ crates/hir/src/lib.rs | 4 +++- crates/hir/src/source_analyzer.rs | 2 +- 8 files changed, 59 insertions(+), 38 deletions(-) diff --git a/crates/hir-def/src/db.rs b/crates/hir-def/src/db.rs index c1127ccaaf..2fd2f18ee4 100644 --- a/crates/hir-def/src/db.rs +++ b/crates/hir-def/src/db.rs @@ -95,21 +95,7 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast Arc; - /// Computes the block-level `DefMap`, returning `None` when `block` doesn't contain any inner - /// items directly. - /// - /// For example: - /// - /// ``` - /// fn f() { // (0) - /// { // (1) - /// fn inner() {} - /// } - /// } - /// ``` - /// - /// The `block_def_map` for block 0 would return `None`, while `block_def_map` of block 1 would - /// return a `DefMap` containing `inner`. + /// Computes the block-level `DefMap`. #[salsa::invoke(DefMap::block_def_map_query)] fn block_def_map(&self, block: BlockId) -> Arc; diff --git a/crates/hir-ty/src/db.rs b/crates/hir-ty/src/db.rs index 9fc76550aa..aa7ca48b18 100644 --- a/crates/hir-ty/src/db.rs +++ b/crates/hir-ty/src/db.rs @@ -86,8 +86,10 @@ pub trait HirDatabase: DefDatabase + Upcast { #[salsa::cycle(crate::lower::ty_recover)] fn ty(&self, def: TyDefId) -> Binders; + /// Returns the type of the value of the given constant, or `None` if the the `ValueTyDefId` is + /// a `StructId` or `EnumVariantId` with a record constructor. #[salsa::invoke(crate::lower::value_ty_query)] - fn value_ty(&self, def: ValueTyDefId) -> Binders; + fn value_ty(&self, def: ValueTyDefId) -> Option>; #[salsa::invoke(crate::lower::impl_self_ty_query)] #[salsa::cycle(crate::lower::impl_self_ty_recover)] diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 9d8096d32f..d4b6999614 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -1245,7 +1245,7 @@ impl InferenceContext<'_> { .build(); self.write_method_resolution(tgt_expr, func, subst.clone()); - let method_ty = self.db.value_ty(func.into()).substitute(Interner, &subst); + let method_ty = self.db.value_ty(func.into()).unwrap().substitute(Interner, &subst); self.register_obligations_for_call(&method_ty); self.infer_expr_coerce(rhs, &Expectation::has_type(rhs_ty.clone())); @@ -1541,7 +1541,7 @@ impl InferenceContext<'_> { self.check_method_call( tgt_expr, &[], - self.db.value_ty(func.into()), + self.db.value_ty(func.into()).unwrap(), substs, ty, expected, @@ -1586,7 +1586,7 @@ impl InferenceContext<'_> { item: func.into(), }) } - (ty, self.db.value_ty(func.into()), substs) + (ty, self.db.value_ty(func.into()).unwrap(), substs) } None => { let field_with_same_name_exists = match self.lookup_field(&receiver_ty, method_name) diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index a09bb15147..19ffac22d1 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -34,7 +34,7 @@ impl InferenceContext<'_> { self.add_required_obligations_for_value_path(generic_def, &substs); - let ty = self.db.value_ty(value_def).substitute(Interner, &substs); + let ty = self.db.value_ty(value_def)?.substitute(Interner, &substs); let ty = self.normalize_associated_types_in(ty); Some(ty) } @@ -98,7 +98,7 @@ impl InferenceContext<'_> { let Some(generic_def) = value_def.to_generic_def_id() else { // `value_def` is the kind of item that can never be generic (i.e. statics, at least // currently). We can just skip the binders to get its type. - let (ty, binders) = self.db.value_ty(value_def).into_value_and_skipped_binders(); + let (ty, binders) = self.db.value_ty(value_def)?.into_value_and_skipped_binders(); stdx::always!( parent_substs.is_none() && binders.is_empty(Interner), "non-empty binders for non-generic def", diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index f66b584e3b..a07063ba60 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -1727,19 +1727,19 @@ fn fn_sig_for_struct_constructor(db: &dyn HirDatabase, def: StructId) -> PolyFnS } /// Build the type of a tuple struct constructor. -fn type_for_struct_constructor(db: &dyn HirDatabase, def: StructId) -> Binders { +fn type_for_struct_constructor(db: &dyn HirDatabase, def: StructId) -> Option> { let struct_data = db.struct_data(def); match struct_data.variant_data.kind() { - StructKind::Record => unreachable!("callers check for valueness of variant"), - StructKind::Unit => return type_for_adt(db, def.into()), + StructKind::Record => None, + StructKind::Unit => Some(type_for_adt(db, def.into())), StructKind::Tuple => { let generics = generics(db.upcast(), AdtId::from(def).into()); let substs = generics.bound_vars_subst(db, DebruijnIndex::INNERMOST); - make_binders( + Some(make_binders( db, &generics, TyKind::FnDef(CallableDefId::StructId(def).to_chalk(db), substs).intern(Interner), - ) + )) } } } @@ -1757,20 +1757,23 @@ fn fn_sig_for_enum_variant_constructor(db: &dyn HirDatabase, def: EnumVariantId) } /// Build the type of a tuple enum variant constructor. -fn type_for_enum_variant_constructor(db: &dyn HirDatabase, def: EnumVariantId) -> Binders { +fn type_for_enum_variant_constructor( + db: &dyn HirDatabase, + def: EnumVariantId, +) -> Option> { let e = def.lookup(db.upcast()).parent; match db.enum_variant_data(def).variant_data.kind() { - StructKind::Record => unreachable!("callers check for valueness of variant"), - StructKind::Unit => return type_for_adt(db, e.into()), + StructKind::Record => None, + StructKind::Unit => Some(type_for_adt(db, e.into())), StructKind::Tuple => { let generics = generics(db.upcast(), e.into()); let substs = generics.bound_vars_subst(db, DebruijnIndex::INNERMOST); - make_binders( + Some(make_binders( db, &generics, TyKind::FnDef(CallableDefId::EnumVariantId(def).to_chalk(db), substs) .intern(Interner), - ) + )) } } } @@ -1889,14 +1892,14 @@ pub(crate) fn ty_recover(db: &dyn HirDatabase, _cycle: &Cycle, def: &TyDefId) -> make_binders(db, &generics, TyKind::Error.intern(Interner)) } -pub(crate) fn value_ty_query(db: &dyn HirDatabase, def: ValueTyDefId) -> Binders { +pub(crate) fn value_ty_query(db: &dyn HirDatabase, def: ValueTyDefId) -> Option> { match def { - ValueTyDefId::FunctionId(it) => type_for_fn(db, it), + ValueTyDefId::FunctionId(it) => Some(type_for_fn(db, it)), ValueTyDefId::StructId(it) => type_for_struct_constructor(db, it), - ValueTyDefId::UnionId(it) => type_for_adt(db, it.into()), + ValueTyDefId::UnionId(it) => Some(type_for_adt(db, it.into())), ValueTyDefId::EnumVariantId(it) => type_for_enum_variant_constructor(db, it), - ValueTyDefId::ConstId(it) => type_for_const(db, it), - ValueTyDefId::StaticId(it) => type_for_static(db, it), + ValueTyDefId::ConstId(it) => Some(type_for_const(db, it)), + ValueTyDefId::StaticId(it) => Some(type_for_static(db, it)), } } diff --git a/crates/hir-ty/src/tests/regression.rs b/crates/hir-ty/src/tests/regression.rs index 35079e7094..b0532ddcb3 100644 --- a/crates/hir-ty/src/tests/regression.rs +++ b/crates/hir-ty/src/tests/regression.rs @@ -2012,3 +2012,31 @@ fn rustc_test_issue_52437() { "#, ); } + +#[test] +fn incorrect_variant_form_through_alias_caught() { + check_types( + r#" +enum Enum { Braced {}, Unit, Tuple() } +type Alias = Enum; + +fn main() { + Alias::Braced; + //^^^^^^^^^^^^^ {unknown} + let Alias::Braced = loop {}; + //^^^^^^^^^^^^^ ! + let Alias::Braced(..) = loop {}; + //^^^^^^^^^^^^^^^^^ Enum + + Alias::Unit(); + //^^^^^^^^^^^^^ {unknown} + Alias::Unit{}; + //^^^^^^^^^^^^^ Enum + let Alias::Unit() = loop {}; + //^^^^^^^^^^^^^ Enum + let Alias::Unit{} = loop {}; + //^^^^^^^^^^^^^ Enum +} +"#, + ) +} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 45aa3a0530..ab42cbeecc 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -3786,7 +3786,9 @@ impl Type { } fn from_value_def(db: &dyn HirDatabase, def: impl Into + HasResolver) -> Type { - let ty = db.value_ty(def.into()); + let Some(ty) = db.value_ty(def.into()) else { + return Type::new(db, def, TyKind::Error.intern(Interner)); + }; let substs = TyBuilder::unknown_subst( db, match def.into() { diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 73f8db762a..4dc30950da 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -268,7 +268,7 @@ impl SourceAnalyzer { ) -> Option { let expr_id = self.expr_id(db, &call.clone().into())?; let (func, substs) = self.infer.as_ref()?.method_resolution(expr_id)?; - let ty = db.value_ty(func.into()).substitute(Interner, &substs); + let ty = db.value_ty(func.into())?.substitute(Interner, &substs); let ty = Type::new_with_resolver(db, &self.resolver, ty); let mut res = ty.as_callable(db)?; res.is_bound_method = true;