From cf6809645e2327e20edd30eb535d4f06fa116b5c Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sat, 30 Nov 2019 12:35:37 +0100 Subject: Handle cycles in impl types better - impl Trait for S is allowed - impl Trait for S is an invalid cycle, but we can add cycle recovery for it in Salsa now --- crates/ra_hir_ty/src/db.rs | 10 +++++--- crates/ra_hir_ty/src/infer/coerce.rs | 7 ++---- crates/ra_hir_ty/src/infer/path.rs | 2 +- crates/ra_hir_ty/src/lib.rs | 15 ----------- crates/ra_hir_ty/src/lower.rs | 35 +++++++++++++++----------- crates/ra_hir_ty/src/method_resolution.rs | 9 ++++--- crates/ra_hir_ty/src/tests.rs | 42 +++++++++++++++++++++++++++++++ crates/ra_hir_ty/src/traits/chalk.rs | 15 +++-------- 8 files changed, 82 insertions(+), 53 deletions(-) (limited to 'crates/ra_hir_ty') diff --git a/crates/ra_hir_ty/src/db.rs b/crates/ra_hir_ty/src/db.rs index 9ce154593..6ecc0b096 100644 --- a/crates/ra_hir_ty/src/db.rs +++ b/crates/ra_hir_ty/src/db.rs @@ -11,7 +11,7 @@ use ra_db::{salsa, CrateId}; use crate::{ method_resolution::CrateImplBlocks, traits::{AssocTyValue, Impl}, - CallableDef, FnSig, GenericPredicate, ImplTy, InferenceResult, Substs, Ty, TyDefId, TypeCtor, + CallableDef, FnSig, GenericPredicate, InferenceResult, Substs, TraitRef, Ty, TyDefId, TypeCtor, ValueTyDefId, }; @@ -27,8 +27,12 @@ pub trait HirDatabase: DefDatabase { #[salsa::invoke(crate::lower::value_ty_query)] fn value_ty(&self, def: ValueTyDefId) -> Ty; - #[salsa::invoke(crate::lower::impl_ty_query)] - fn impl_ty(&self, def: ImplId) -> ImplTy; + #[salsa::invoke(crate::lower::impl_self_ty_query)] + #[salsa::cycle(crate::lower::impl_self_ty_recover)] + fn impl_self_ty(&self, def: ImplId) -> Ty; + + #[salsa::invoke(crate::lower::impl_trait_query)] + fn impl_trait(&self, def: ImplId) -> Option; #[salsa::invoke(crate::lower::field_types_query)] fn field_types(&self, var: VariantId) -> Arc>; diff --git a/crates/ra_hir_ty/src/infer/coerce.rs b/crates/ra_hir_ty/src/infer/coerce.rs index 719a0f395..064993d34 100644 --- a/crates/ra_hir_ty/src/infer/coerce.rs +++ b/crates/ra_hir_ty/src/infer/coerce.rs @@ -8,7 +8,7 @@ use hir_def::{lang_item::LangItemTarget, resolver::Resolver, type_ref::Mutabilit use rustc_hash::FxHashMap; use test_utils::tested_by; -use crate::{autoderef, db::HirDatabase, ImplTy, Substs, Ty, TypeCtor, TypeWalk}; +use crate::{autoderef, db::HirDatabase, Substs, Ty, TypeCtor, TypeWalk}; use super::{InEnvironment, InferTy, InferenceContext, TypeVarValue}; @@ -54,10 +54,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { impls .iter() .filter_map(|&impl_id| { - let trait_ref = match db.impl_ty(impl_id) { - ImplTy::TraitRef(it) => it, - ImplTy::Inherent(_) => return None, - }; + let trait_ref = db.impl_trait(impl_id)?; // `CoerseUnsized` has one generic parameter for the target type. let cur_from_ty = trait_ref.substs.0.get(0)?; diff --git a/crates/ra_hir_ty/src/infer/path.rs b/crates/ra_hir_ty/src/infer/path.rs index 14be66836..bbf146418 100644 --- a/crates/ra_hir_ty/src/infer/path.rs +++ b/crates/ra_hir_ty/src/infer/path.rs @@ -244,7 +244,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { ContainerId::ImplId(it) => it, _ => return None, }; - let self_ty = self.db.impl_ty(impl_id).self_type().clone(); + let self_ty = self.db.impl_self_ty(impl_id).clone(); let self_ty_substs = self_ty.substs()?; let actual_substs = actual_def_ty.substs()?; diff --git a/crates/ra_hir_ty/src/lib.rs b/crates/ra_hir_ty/src/lib.rs index b45c8f82f..3c1f738df 100644 --- a/crates/ra_hir_ty/src/lib.rs +++ b/crates/ra_hir_ty/src/lib.rs @@ -486,21 +486,6 @@ impl TypeWalk for TraitRef { } } -#[derive(Clone, PartialEq, Eq, Debug)] -pub enum ImplTy { - Inherent(Ty), - TraitRef(TraitRef), -} - -impl ImplTy { - pub(crate) fn self_type(&self) -> &Ty { - match self { - ImplTy::Inherent(it) => it, - ImplTy::TraitRef(tr) => &tr.substs[0], - } - } -} - /// Like `generics::WherePredicate`, but with resolved types: A condition on the /// parameters of a generic item. #[derive(Debug, Clone, PartialEq, Eq, Hash)] diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs index 091c60f4f..a646406f1 100644 --- a/crates/ra_hir_ty/src/lower.rs +++ b/crates/ra_hir_ty/src/lower.rs @@ -27,8 +27,8 @@ use crate::{ all_super_traits, associated_type_by_name_including_super_traits, make_mut_slice, variant_data, }, - FnSig, GenericPredicate, ImplTy, ProjectionPredicate, ProjectionTy, Substs, TraitEnvironment, - TraitRef, Ty, TypeCtor, TypeWalk, + FnSig, GenericPredicate, ProjectionPredicate, ProjectionTy, Substs, TraitEnvironment, TraitRef, + Ty, TypeCtor, TypeWalk, }; impl Ty { @@ -179,7 +179,7 @@ impl Ty { let name = resolved_segment.name.clone(); Ty::Param { idx, name } } - TypeNs::SelfType(impl_id) => db.impl_ty(impl_id).self_type().clone(), + TypeNs::SelfType(impl_id) => db.impl_self_ty(impl_id).clone(), TypeNs::AdtSelfType(adt) => db.ty(adt.into()), TypeNs::AdtId(it) => Ty::from_hir_path_inner(db, resolver, resolved_segment, it.into()), @@ -743,17 +743,24 @@ pub(crate) fn value_ty_query(db: &impl HirDatabase, def: ValueTyDefId) -> Ty { } } -pub(crate) fn impl_ty_query(db: &impl HirDatabase, impl_id: ImplId) -> ImplTy { +pub(crate) fn impl_self_ty_query(db: &impl HirDatabase, impl_id: ImplId) -> Ty { let impl_data = db.impl_data(impl_id); let resolver = impl_id.resolver(db); - let self_ty = Ty::from_hir(db, &resolver, &impl_data.target_type); - match impl_data.target_trait.as_ref() { - Some(trait_ref) => { - match TraitRef::from_hir(db, &resolver, trait_ref, Some(self_ty.clone())) { - Some(it) => ImplTy::TraitRef(it), - None => ImplTy::Inherent(self_ty), - } - } - None => ImplTy::Inherent(self_ty), - } + Ty::from_hir(db, &resolver, &impl_data.target_type) +} + +pub(crate) fn impl_self_ty_recover( + _db: &impl HirDatabase, + _cycle: &[String], + _impl_id: &ImplId, +) -> Ty { + Ty::Unknown +} + +pub(crate) fn impl_trait_query(db: &impl HirDatabase, impl_id: ImplId) -> Option { + let impl_data = db.impl_data(impl_id); + let resolver = impl_id.resolver(db); + let self_ty = db.impl_self_ty(impl_id); + let target_trait = impl_data.target_trait.as_ref()?; + TraitRef::from_hir(db, &resolver, target_trait, Some(self_ty.clone())) } diff --git a/crates/ra_hir_ty/src/method_resolution.rs b/crates/ra_hir_ty/src/method_resolution.rs index ee1936b0e..2bded3dbd 100644 --- a/crates/ra_hir_ty/src/method_resolution.rs +++ b/crates/ra_hir_ty/src/method_resolution.rs @@ -19,7 +19,7 @@ use crate::{ db::HirDatabase, primitive::{FloatBitness, Uncertain}, utils::all_super_traits, - Canonical, ImplTy, InEnvironment, TraitEnvironment, TraitRef, Ty, TypeCtor, + Canonical, InEnvironment, TraitEnvironment, TraitRef, Ty, TypeCtor, }; /// This is used as a key for indexing impls. @@ -58,11 +58,12 @@ impl CrateImplBlocks { let crate_def_map = db.crate_def_map(krate); for (_module_id, module_data) in crate_def_map.modules.iter() { for &impl_id in module_data.impls.iter() { - match db.impl_ty(impl_id) { - ImplTy::TraitRef(tr) => { + match db.impl_trait(impl_id) { + Some(tr) => { res.impls_by_trait.entry(tr.trait_).or_default().push(impl_id); } - ImplTy::Inherent(self_ty) => { + None => { + let self_ty = db.impl_self_ty(impl_id); if let Some(self_ty_fp) = TyFingerprint::for_impl(&self_ty) { res.impls.entry(self_ty_fp).or_default().push(impl_id); } diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 4ba87e667..b72f0f279 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -4675,6 +4675,48 @@ fn test() where T::Item: Trait2, T: Trait, U: Trait<()> { assert_eq!(t, "u32"); } +#[test] +fn trait_impl_self_ty() { + let t = type_at( + r#" +//- /main.rs +trait Trait { + fn foo(&self); +} + +struct S; + +impl Trait for S {} + +fn test() { + S.foo()<|>; +} +"#, + ); + assert_eq!(t, "()"); +} + +#[test] +fn trait_impl_self_ty_cycle() { + let t = type_at( + r#" +//- /main.rs +trait Trait { + fn foo(&self); +} + +struct S; + +impl Trait for S {} + +fn test() { + S.foo()<|>; +} +"#, + ); + assert_eq!(t, "{unknown}"); +} + #[test] // FIXME this is currently a Salsa panic; it would be nicer if it just returned // in Unknown, and we should be able to do that once Salsa allows us to handle diff --git a/crates/ra_hir_ty/src/traits/chalk.rs b/crates/ra_hir_ty/src/traits/chalk.rs index 35de37e6b..104346ada 100644 --- a/crates/ra_hir_ty/src/traits/chalk.rs +++ b/crates/ra_hir_ty/src/traits/chalk.rs @@ -20,8 +20,8 @@ use ra_db::salsa::{InternId, InternKey}; use super::{AssocTyValue, Canonical, ChalkContext, Impl, Obligation}; use crate::{ - db::HirDatabase, display::HirDisplay, ApplicationTy, GenericPredicate, ImplTy, ProjectionTy, - Substs, TraitRef, Ty, TypeCtor, TypeWalk, + db::HirDatabase, display::HirDisplay, ApplicationTy, GenericPredicate, ProjectionTy, Substs, + TraitRef, Ty, TypeCtor, TypeWalk, }; /// This represents a trait whose name we could not resolve. @@ -630,10 +630,7 @@ fn impl_block_datum( chalk_id: chalk_ir::ImplId, impl_id: ImplId, ) -> Option>> { - let trait_ref = match db.impl_ty(impl_id) { - ImplTy::TraitRef(it) => it, - ImplTy::Inherent(_) => return None, - }; + let trait_ref = db.impl_trait(impl_id)?; let impl_data = db.impl_data(impl_id); let generic_params = db.generic_params(impl_id.into()); @@ -787,11 +784,7 @@ fn type_alias_associated_ty_value( _ => panic!("assoc ty value should be in impl"), }; - let trait_ref = match db.impl_ty(impl_id) { - ImplTy::TraitRef(it) => it, - // we don't return any assoc ty values if the impl'd trait can't be resolved - ImplTy::Inherent(_) => panic!("assoc ty value should not exist"), - }; + let trait_ref = db.impl_trait(impl_id).expect("assoc ty value should not exist"); // we don't return any assoc ty values if the impl'd trait can't be resolved let assoc_ty = db .trait_data(trait_ref.trait_) -- cgit v1.2.3 From 3ca40f7c08718a44c6d08d2cbe060244340e7157 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sat, 30 Nov 2019 12:39:21 +0100 Subject: Add cycle recovery for generic predicates --- crates/ra_hir_ty/src/db.rs | 1 + crates/ra_hir_ty/src/lower.rs | 9 +++++++++ crates/ra_hir_ty/src/tests.rs | 8 -------- 3 files changed, 10 insertions(+), 8 deletions(-) (limited to 'crates/ra_hir_ty') diff --git a/crates/ra_hir_ty/src/db.rs b/crates/ra_hir_ty/src/db.rs index 6ecc0b096..8e461e359 100644 --- a/crates/ra_hir_ty/src/db.rs +++ b/crates/ra_hir_ty/src/db.rs @@ -41,6 +41,7 @@ pub trait HirDatabase: DefDatabase { fn callable_item_signature(&self, def: CallableDef) -> FnSig; #[salsa::invoke(crate::lower::generic_predicates_for_param_query)] + #[salsa::cycle(crate::lower::generic_predicates_for_param_recover)] fn generic_predicates_for_param( &self, def: GenericDefId, diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs index a646406f1..c6ee75c7a 100644 --- a/crates/ra_hir_ty/src/lower.rs +++ b/crates/ra_hir_ty/src/lower.rs @@ -532,6 +532,15 @@ pub(crate) fn generic_predicates_for_param_query( .collect() } +pub(crate) fn generic_predicates_for_param_recover( + _db: &impl HirDatabase, + _cycle: &[String], + _def: &GenericDefId, + _param_idx: &u32, +) -> Arc<[GenericPredicate]> { + Arc::new([]) +} + impl TraitEnvironment { pub fn lower(db: &impl HirDatabase, resolver: &Resolver) -> Arc { let predicates = resolver diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index b72f0f279..552eb8f75 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -4718,10 +4718,6 @@ fn test() { } #[test] -// FIXME this is currently a Salsa panic; it would be nicer if it just returned -// in Unknown, and we should be able to do that once Salsa allows us to handle -// the cycle. But at least it doesn't overflow for now. -#[should_panic] fn unselected_projection_in_trait_env_cycle_1() { let t = type_at( r#" @@ -4742,10 +4738,6 @@ fn test() where T: Trait2 { } #[test] -// FIXME this is currently a Salsa panic; it would be nicer if it just returned -// in Unknown, and we should be able to do that once Salsa allows us to handle -// the cycle. But at least it doesn't overflow for now. -#[should_panic] fn unselected_projection_in_trait_env_cycle_2() { let t = type_at( r#" -- cgit v1.2.3 From 1c622e9fed97de7711da7b5bffec0fa4b19d7500 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sat, 30 Nov 2019 12:48:51 +0100 Subject: Add cycle recovery for type aliases --- crates/ra_hir_ty/src/db.rs | 1 + crates/ra_hir_ty/src/lower.rs | 5 +++++ crates/ra_hir_ty/src/tests.rs | 6 ++++-- 3 files changed, 10 insertions(+), 2 deletions(-) (limited to 'crates/ra_hir_ty') diff --git a/crates/ra_hir_ty/src/db.rs b/crates/ra_hir_ty/src/db.rs index 8e461e359..222a36a9f 100644 --- a/crates/ra_hir_ty/src/db.rs +++ b/crates/ra_hir_ty/src/db.rs @@ -22,6 +22,7 @@ pub trait HirDatabase: DefDatabase { fn infer(&self, def: DefWithBodyId) -> Arc; #[salsa::invoke(crate::lower::ty_query)] + #[salsa::cycle(crate::lower::ty_recover)] fn ty(&self, def: TyDefId) -> Ty; #[salsa::invoke(crate::lower::value_ty_query)] diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs index c6ee75c7a..32569ac66 100644 --- a/crates/ra_hir_ty/src/lower.rs +++ b/crates/ra_hir_ty/src/lower.rs @@ -742,6 +742,11 @@ pub(crate) fn ty_query(db: &impl HirDatabase, def: TyDefId) -> Ty { TyDefId::TypeAliasId(it) => type_for_type_alias(db, it), } } + +pub(crate) fn ty_recover(_db: &impl HirDatabase, _cycle: &[String], _def: &TyDefId) -> Ty { + Ty::Unknown +} + pub(crate) fn value_ty_query(db: &impl HirDatabase, def: ValueTyDefId) -> Ty { match def { ValueTyDefId::FunctionId(it) => type_for_fn(db, it), diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 552eb8f75..c856d6afd 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -2154,7 +2154,6 @@ fn test(x: Foo, y: Bar<&str>, z: Baz) { } #[test] -#[should_panic] // we currently can't handle this fn recursive_type_alias() { assert_snapshot!( infer(r#" @@ -2163,7 +2162,10 @@ type Foo = Foo; type Bar = A; fn test(x: Foo) {} "#), - @"" + @r###" + [59; 60) 'x': {unknown} + [67; 69) '{}': () + "### ) } -- cgit v1.2.3