From b4795507e3e654bb3efe1b9fe2651ad4966ccc4d Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 11 Jun 2023 19:12:02 +0900 Subject: [PATCH 1/2] autoderef: completely resolve and deduplicate types --- crates/hir-ty/src/autoderef.rs | 24 ++++++++++- crates/hir/src/lib.rs | 8 ++-- crates/ide-completion/src/completions/dot.rs | 42 ++++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/crates/hir-ty/src/autoderef.rs b/crates/hir-ty/src/autoderef.rs index f5b3f176b12..3860bccec8b 100644 --- a/crates/hir-ty/src/autoderef.rs +++ b/crates/hir-ty/src/autoderef.rs @@ -22,17 +22,37 @@ pub(crate) enum AutoderefKind { Overloaded, } +/// Returns types that `ty` transitively dereferences to. This function is only meant to be used +/// outside `hir-ty`. +/// +/// It is guaranteed that: +/// - the yielded types don't contain inference variables (but may contain `TyKind::Error`). +/// - a type won't be yielded more than once; in other words, the returned iterator will stop if it +/// detects a cycle in the deref chain. pub fn autoderef( db: &dyn HirDatabase, env: Arc, ty: Canonical, -) -> impl Iterator> + '_ { +) -> impl Iterator { let mut table = InferenceTable::new(db, env); let ty = table.instantiate_canonical(ty); let mut autoderef = Autoderef::new(&mut table, ty); let mut v = Vec::new(); while let Some((ty, _steps)) = autoderef.next() { - v.push(autoderef.table.canonicalize(ty).value); + // `ty` may contain unresolved inference variables. Since there's no chance they would be + // resolved, just replace with fallback type. + let resolved = autoderef.table.resolve_completely(ty); + + // If the deref chain contains a cycle (e.g. `A` derefs to `B` and `B` derefs to `A`), we + // would revisit some already visited types. Stop here to avoid duplication. + // + // XXX: The recursion limit for `Autoderef` is currently 10, so `Vec::contains()` shouldn't + // be too expensive. Replace this duplicate check with `FxHashSet` if it proves to be more + // performant. + if v.contains(&resolved) { + break; + } + v.push(resolved); } v.into_iter() } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 5527eb59387..c890c83bd43 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -3795,14 +3795,16 @@ pub fn as_array(&self, db: &dyn HirDatabase) -> Option<(Type, usize)> { } } - pub fn autoderef<'a>(&'a self, db: &'a dyn HirDatabase) -> impl Iterator + 'a { + /// Returns types that this type dereferences to (including this type itself). The returned + /// iterator won't yield the same type more than once even if the deref chain contains a cycle. + pub fn autoderef(&self, db: &dyn HirDatabase) -> impl Iterator + '_ { self.autoderef_(db).map(move |ty| self.derived(ty)) } - fn autoderef_<'a>(&'a self, db: &'a dyn HirDatabase) -> impl Iterator + 'a { + fn autoderef_(&self, db: &dyn HirDatabase) -> impl Iterator { // There should be no inference vars in types passed here let canonical = hir_ty::replace_errors_with_variables(&self.ty); - autoderef(db, self.env.clone(), canonical).map(|canonical| canonical.value) + autoderef(db, self.env.clone(), canonical) } // This would be nicer if it just returned an iterator, but that runs into diff --git a/crates/ide-completion/src/completions/dot.rs b/crates/ide-completion/src/completions/dot.rs index 57a784c45b6..78f6c034ba2 100644 --- a/crates/ide-completion/src/completions/dot.rs +++ b/crates/ide-completion/src/completions/dot.rs @@ -979,4 +979,46 @@ fn test(thing: impl Encrypt) { "#]], ) } + + #[test] + fn only_consider_same_type_once() { + check( + r#" +//- minicore: deref +struct A(u8); +struct B(u16); +impl core::ops::Deref for A { + type Target = B; + fn deref(&self) -> &Self::Target { loop {} } +} +impl core::ops::Deref for B { + type Target = A; + fn deref(&self) -> &Self::Target { loop {} } +} +fn test(a: A) { + a.$0 +} +"#, + expect![[r#" + fd 0 u16 + fd 0 u8 + me deref() (use core::ops::Deref) fn(&self) -> &::Target + "#]], + ); + } + + #[test] + fn no_inference_var_in_completion() { + check( + r#" +struct S(T); +fn test(s: S) { + s.$0 +} +"#, + expect![[r#" + fd 0 {unknown} + "#]], + ); + } } From 42eab5e10048869ab5119efd57adc6790f63bad3 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 11 Jun 2023 19:16:13 +0900 Subject: [PATCH 2/2] Deduplicate field names for completion --- crates/ide-completion/src/completions/dot.rs | 52 +++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/crates/ide-completion/src/completions/dot.rs b/crates/ide-completion/src/completions/dot.rs index 78f6c034ba2..a00c6ecb91f 100644 --- a/crates/ide-completion/src/completions/dot.rs +++ b/crates/ide-completion/src/completions/dot.rs @@ -105,9 +105,12 @@ fn complete_fields( mut named_field: impl FnMut(&mut Completions, hir::Field, hir::Type), mut tuple_index: impl FnMut(&mut Completions, usize, hir::Type), ) { + let mut seen_names = FxHashSet::default(); for receiver in receiver.autoderef(ctx.db) { for (field, ty) in receiver.fields(ctx.db) { - named_field(acc, field, ty); + if seen_names.insert(field.name(ctx.db)) { + named_field(acc, field, ty); + } } for (i, ty) in receiver.tuple_fields(ctx.db).into_iter().enumerate() { // Tuple fields are always public (tuple struct fields are handled above). @@ -671,6 +674,52 @@ fn foo(&self) { ); } + #[test] + fn test_field_no_same_name() { + check( + r#" +//- minicore: deref +struct A { field: u8 } +struct B { field: u16, another: u32 } +impl core::ops::Deref for A { + type Target = B; + fn deref(&self) -> &Self::Target { loop {} } +} +fn test(a: A) { + a.$0 +} +"#, + expect![[r#" + fd another u32 + fd field u8 + me deref() (use core::ops::Deref) fn(&self) -> &::Target + "#]], + ); + } + + #[test] + fn test_tuple_field_no_same_index() { + check( + r#" +//- minicore: deref +struct A(u8); +struct B(u16, u32); +impl core::ops::Deref for A { + type Target = B; + fn deref(&self) -> &Self::Target { loop {} } +} +fn test(a: A) { + a.$0 +} +"#, + expect![[r#" + fd 0 u8 + fd 1 u32 + me deref() (use core::ops::Deref) fn(&self) -> &::Target + "#]], + ); + } + #[test] fn test_completion_works_in_consts() { check( @@ -1000,7 +1049,6 @@ fn test(a: A) { } "#, expect![[r#" - fd 0 u16 fd 0 u8 me deref() (use core::ops::Deref) fn(&self) -> &::Target "#]],