From a4a4a1854ebb53e1cdd7a5e3b308112bbbf3c676 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 29 May 2020 19:14:04 +0200 Subject: Fix type parameter defaults They should not be applied in expression or pattern contexts, unless there are other explicitly given type args. --- crates/ra_hir_ty/src/infer.rs | 4 +- crates/ra_hir_ty/src/infer/path.rs | 3 +- crates/ra_hir_ty/src/lower.rs | 70 +++++++++----- crates/ra_hir_ty/src/tests/display_source_code.rs | 4 +- crates/ra_hir_ty/src/tests/method_resolution.rs | 54 ----------- crates/ra_hir_ty/src/tests/simple.rs | 108 ++++++++++++++++++++++ crates/ra_hir_ty/src/tests/traits.rs | 54 +++++------ 7 files changed, 187 insertions(+), 110 deletions(-) (limited to 'crates/ra_hir_ty') diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs index d9bf3c2f0..2e16e5120 100644 --- a/crates/ra_hir_ty/src/infer.rs +++ b/crates/ra_hir_ty/src/infer.rs @@ -439,13 +439,13 @@ impl<'a> InferenceContext<'a> { }; return match resolution { TypeNs::AdtId(AdtId::StructId(strukt)) => { - let substs = Ty::substs_from_path(&ctx, path, strukt.into()); + let substs = Ty::substs_from_path(&ctx, path, strukt.into(), true); let ty = self.db.ty(strukt.into()); let ty = self.insert_type_vars(ty.subst(&substs)); forbid_unresolved_segments((ty, Some(strukt.into())), unresolved) } TypeNs::EnumVariantId(var) => { - let substs = Ty::substs_from_path(&ctx, path, var.into()); + let substs = Ty::substs_from_path(&ctx, path, var.into(), true); let ty = self.db.ty(var.parent.into()); let ty = self.insert_type_vars(ty.subst(&substs)); forbid_unresolved_segments((ty, Some(var.into())), unresolved) diff --git a/crates/ra_hir_ty/src/infer/path.rs b/crates/ra_hir_ty/src/infer/path.rs index 1c2e56fb0..1ad0d8397 100644 --- a/crates/ra_hir_ty/src/infer/path.rs +++ b/crates/ra_hir_ty/src/infer/path.rs @@ -95,7 +95,7 @@ impl<'a> InferenceContext<'a> { // self_subst is just for the parent let parent_substs = self_subst.unwrap_or_else(Substs::empty); let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver); - let substs = Ty::substs_from_path(&ctx, path, typable); + let substs = Ty::substs_from_path(&ctx, path, typable, true); let full_substs = Substs::builder(substs.len()) .use_parent_substs(&parent_substs) .fill(substs.0[parent_substs.len()..].iter().cloned()) @@ -141,6 +141,7 @@ impl<'a> InferenceContext<'a> { def, resolved_segment, remaining_segments_for_ty, + true, ); if let Ty::Unknown = ty { return None; diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs index a05cbd7fc..42713928f 100644 --- a/crates/ra_hir_ty/src/lower.rs +++ b/crates/ra_hir_ty/src/lower.rs @@ -323,6 +323,7 @@ impl Ty { resolution: TypeNs, resolved_segment: PathSegment<'_>, remaining_segments: PathSegments<'_>, + infer_args: bool, ) -> (Ty, Option) { let ty = match resolution { TypeNs::TraitId(trait_) => { @@ -400,9 +401,15 @@ impl Ty { ctx.db.ty(adt.into()).subst(&substs) } - TypeNs::AdtId(it) => Ty::from_hir_path_inner(ctx, resolved_segment, it.into()), - TypeNs::BuiltinType(it) => Ty::from_hir_path_inner(ctx, resolved_segment, it.into()), - TypeNs::TypeAliasId(it) => Ty::from_hir_path_inner(ctx, resolved_segment, it.into()), + TypeNs::AdtId(it) => { + Ty::from_hir_path_inner(ctx, resolved_segment, it.into(), infer_args) + } + TypeNs::BuiltinType(it) => { + Ty::from_hir_path_inner(ctx, resolved_segment, it.into(), infer_args) + } + TypeNs::TypeAliasId(it) => { + Ty::from_hir_path_inner(ctx, resolved_segment, it.into(), infer_args) + } // FIXME: report error TypeNs::EnumVariantId(_) => return (Ty::Unknown, None), }; @@ -428,7 +435,13 @@ impl Ty { ), Some(i) => (path.segments().get(i - 1).unwrap(), path.segments().skip(i)), }; - Ty::from_partly_resolved_hir_path(ctx, resolution, resolved_segment, remaining_segments) + Ty::from_partly_resolved_hir_path( + ctx, + resolution, + resolved_segment, + remaining_segments, + false, + ) } fn select_associated_type( @@ -474,13 +487,14 @@ impl Ty { ctx: &TyLoweringContext<'_>, segment: PathSegment<'_>, typable: TyDefId, + infer_args: bool, ) -> Ty { let generic_def = match typable { TyDefId::BuiltinType(_) => None, TyDefId::AdtId(it) => Some(it.into()), TyDefId::TypeAliasId(it) => Some(it.into()), }; - let substs = substs_from_path_segment(ctx, segment, generic_def, false); + let substs = substs_from_path_segment(ctx, segment, generic_def, infer_args); ctx.db.ty(typable).subst(&substs) } @@ -493,6 +507,7 @@ impl Ty { // `ValueTyDefId` is just a convenient way to pass generics and // special-case enum variants resolved: ValueTyDefId, + infer_args: bool, ) -> Substs { let last = path.segments().last().expect("path should have at least one segment"); let (segment, generic_def) = match resolved { @@ -515,22 +530,27 @@ impl Ty { (segment, Some(var.parent.into())) } }; - substs_from_path_segment(ctx, segment, generic_def, false) + substs_from_path_segment(ctx, segment, generic_def, infer_args) } } -pub(super) fn substs_from_path_segment( +fn substs_from_path_segment( ctx: &TyLoweringContext<'_>, segment: PathSegment<'_>, def_generic: Option, - _add_self_param: bool, + infer_args: bool, ) -> Substs { let mut substs = Vec::new(); let def_generics = def_generic.map(|def| generics(ctx.db.upcast(), def)); let (parent_params, self_params, type_params, impl_trait_params) = def_generics.map_or((0, 0, 0, 0), |g| g.provenance_split()); + let total_len = parent_params + self_params + type_params + impl_trait_params; + substs.extend(iter::repeat(Ty::Unknown).take(parent_params)); + + let mut had_explicit_args = false; + if let Some(generic_args) = &segment.args_and_bindings { if !generic_args.has_self_type { substs.extend(iter::repeat(Ty::Unknown).take(self_params)); @@ -542,31 +562,35 @@ pub(super) fn substs_from_path_segment( for arg in generic_args.args.iter().skip(skip).take(expected_num) { match arg { GenericArg::Type(type_ref) => { + had_explicit_args = true; let ty = Ty::from_hir(ctx, type_ref); substs.push(ty); } } } } - let total_len = parent_params + self_params + type_params + impl_trait_params; - // add placeholders for args that were not provided - for _ in substs.len()..total_len { - substs.push(Ty::Unknown); - } - assert_eq!(substs.len(), total_len); - // handle defaults - if let Some(def_generic) = def_generic { - let default_substs = ctx.db.generic_defaults(def_generic); - assert_eq!(substs.len(), default_substs.len()); + // handle defaults. In expression or pattern path segments without + // explicitly specified type arguments, missing type arguments are inferred + // (i.e. defaults aren't used). + if !infer_args || had_explicit_args { + if let Some(def_generic) = def_generic { + let default_substs = ctx.db.generic_defaults(def_generic); + assert_eq!(total_len, default_substs.len()); - for (i, default_ty) in default_substs.iter().enumerate() { - if substs[i] == Ty::Unknown { - substs[i] = default_ty.clone(); + for default_ty in default_substs.iter().skip(substs.len()) { + substs.push(default_ty.clone()); } } } + // add placeholders for args that were not provided + // FIXME: emit diagnostics in contexts where this is not allowed + for _ in substs.len()..total_len { + substs.push(Ty::Unknown); + } + assert_eq!(substs.len(), total_len); + Substs(substs.into()) } @@ -615,9 +639,7 @@ impl TraitRef { segment: PathSegment<'_>, resolved: TraitId, ) -> Substs { - let has_self_param = - segment.args_and_bindings.as_ref().map(|a| a.has_self_type).unwrap_or(false); - substs_from_path_segment(ctx, segment, Some(resolved.into()), !has_self_param) + substs_from_path_segment(ctx, segment, Some(resolved.into()), false) } pub(crate) fn from_type_bound( diff --git a/crates/ra_hir_ty/src/tests/display_source_code.rs b/crates/ra_hir_ty/src/tests/display_source_code.rs index 4088b1d22..5dfa0a014 100644 --- a/crates/ra_hir_ty/src/tests/display_source_code.rs +++ b/crates/ra_hir_ty/src/tests/display_source_code.rs @@ -29,7 +29,7 @@ fn omit_default_type_parameters() { //- /main.rs struct Foo { t: T } fn main() { - let foo = Foo { t: 5 }; + let foo = Foo { t: 5u8 }; foo<|>; } ", @@ -41,7 +41,7 @@ fn omit_default_type_parameters() { //- /main.rs struct Foo { k: K, t: T } fn main() { - let foo = Foo { k: 400, t: 5 }; + let foo = Foo { k: 400, t: 5u8 }; foo<|>; } ", diff --git a/crates/ra_hir_ty/src/tests/method_resolution.rs b/crates/ra_hir_ty/src/tests/method_resolution.rs index 558a70022..804297315 100644 --- a/crates/ra_hir_ty/src/tests/method_resolution.rs +++ b/crates/ra_hir_ty/src/tests/method_resolution.rs @@ -183,60 +183,6 @@ fn test() { ); } -#[test] -fn infer_associated_method_generics_with_default_param() { - assert_snapshot!( - infer(r#" -struct Gen { - val: T -} - -impl Gen { - pub fn make() -> Gen { - loop { } - } -} - -fn test() { - let a = Gen::make(); -} -"#), - @r###" - 80..104 '{ ... }': Gen - 90..98 'loop { }': ! - 95..98 '{ }': () - 118..146 '{ ...e(); }': () - 128..129 'a': Gen - 132..141 'Gen::make': fn make() -> Gen - 132..143 'Gen::make()': Gen - "### - ); -} - -#[test] -fn infer_associated_method_generics_with_default_tuple_param() { - let t = type_at( - r#" -//- /main.rs -struct Gen { - val: T -} - -impl Gen { - pub fn make() -> Gen { - loop { } - } -} - -fn test() { - let a = Gen::make(); - a.val<|>; -} -"#, - ); - assert_eq!(t, "()"); -} - #[test] fn infer_associated_method_generics_without_args() { assert_snapshot!( diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index 88309157b..8a5031756 100644 --- a/crates/ra_hir_ty/src/tests/simple.rs +++ b/crates/ra_hir_ty/src/tests/simple.rs @@ -1997,3 +1997,111 @@ fn foo() { "### ); } + +#[test] +fn generic_default() { + assert_snapshot!( + infer(r#" +struct Thing { t: T } +enum OtherThing { + One { t: T }, + Two(T), +} + +fn test(t1: Thing, t2: OtherThing, t3: Thing, t4: OtherThing) { + t1.t; + t3.t; + match t2 { + OtherThing::One { t } => { t; }, + OtherThing::Two(t) => { t; }, + } + match t4 { + OtherThing::One { t } => { t; }, + OtherThing::Two(t) => { t; }, + } +} +"#), + @r###" + 98..100 't1': Thing<()> + 109..111 't2': OtherThing<()> + 125..127 't3': Thing + 141..143 't4': OtherThing + 162..385 '{ ... } }': () + 168..170 't1': Thing<()> + 168..172 't1.t': () + 178..180 't3': Thing + 178..182 't3.t': i32 + 188..283 'match ... }': () + 194..196 't2': OtherThing<()> + 207..228 'OtherT... { t }': OtherThing<()> + 225..226 't': () + 232..238 '{ t; }': () + 234..235 't': () + 248..266 'OtherT...Two(t)': OtherThing<()> + 264..265 't': () + 270..276 '{ t; }': () + 272..273 't': () + 288..383 'match ... }': () + 294..296 't4': OtherThing + 307..328 'OtherT... { t }': OtherThing + 325..326 't': i32 + 332..338 '{ t; }': () + 334..335 't': i32 + 348..366 'OtherT...Two(t)': OtherThing + 364..365 't': i32 + 370..376 '{ t; }': () + 372..373 't': i32 + "### + ); +} + +#[test] +fn generic_default_in_struct_literal() { + assert_snapshot!( + infer(r#" +struct Thing { t: T } +enum OtherThing { + One { t: T }, + Two(T), +} + +fn test() { + let x = Thing { t: loop {} }; + let y = Thing { t: () }; + let z = Thing { t: 1i32 }; + if let Thing { t } = z { + t; + } + + let a = OtherThing::One { t: 1i32 }; + let b = OtherThing::Two(1i32); +} +"#), + @r###" + 100..320 '{ ...32); }': () + 110..111 'x': Thing + 114..134 'Thing ...p {} }': Thing + 125..132 'loop {}': ! + 130..132 '{}': () + 144..145 'y': Thing<()> + 148..163 'Thing { t: () }': Thing<()> + 159..161 '()': () + 173..174 'z': Thing + 177..194 'Thing ...1i32 }': Thing + 188..192 '1i32': i32 + 200..241 'if let... }': () + 207..218 'Thing { t }': Thing + 215..216 't': i32 + 221..222 'z': Thing + 223..241 '{ ... }': () + 233..234 't': i32 + 251..252 'a': OtherThing + 255..282 'OtherT...1i32 }': OtherThing + 276..280 '1i32': i32 + 292..293 'b': OtherThing + 296..311 'OtherThing::Two': Two(i32) -> OtherThing + 296..317 'OtherT...(1i32)': OtherThing + 312..316 '1i32': i32 + "### + ); +} diff --git a/crates/ra_hir_ty/src/tests/traits.rs b/crates/ra_hir_ty/src/tests/traits.rs index 0c538a62d..133fb5f39 100644 --- a/crates/ra_hir_ty/src/tests/traits.rs +++ b/crates/ra_hir_ty/src/tests/traits.rs @@ -1806,33 +1806,33 @@ fn test() { } "#), @r###" -65..69 'self': &Self -166..170 'self': Self -172..176 'args': Args -240..244 'self': &Foo -255..257 '{}': () -335..336 'f': F -355..357 '{}': () -444..690 '{ ...o(); }': () -454..459 'lazy1': Lazy T> -476..485 'Lazy::new': fn new T>(fn() -> T) -> Lazy T> -476..493 'Lazy::...| Foo)': Lazy T> -486..492 '|| Foo': || -> T -489..492 'Foo': Foo -503..505 'r1': {unknown} -508..513 'lazy1': Lazy T> -508..519 'lazy1.foo()': {unknown} -561..576 'make_foo_fn_ptr': fn() -> Foo -592..603 'make_foo_fn': fn make_foo_fn() -> Foo -613..618 'lazy2': Lazy T> -635..644 'Lazy::new': fn new T>(fn() -> T) -> Lazy T> -635..661 'Lazy::...n_ptr)': Lazy T> -645..660 'make_foo_fn_ptr': fn() -> Foo -671..673 'r2': {unknown} -676..681 'lazy2': Lazy T> -676..687 'lazy2.foo()': {unknown} -550..552 '{}': () -"### + 65..69 'self': &Self + 166..170 'self': Self + 172..176 'args': Args + 240..244 'self': &Foo + 255..257 '{}': () + 335..336 'f': F + 355..357 '{}': () + 444..690 '{ ...o(); }': () + 454..459 'lazy1': Lazy Foo> + 476..485 'Lazy::new': fn new Foo>(|| -> Foo) -> Lazy Foo> + 476..493 'Lazy::...| Foo)': Lazy Foo> + 486..492 '|| Foo': || -> Foo + 489..492 'Foo': Foo + 503..505 'r1': usize + 508..513 'lazy1': Lazy Foo> + 508..519 'lazy1.foo()': usize + 561..576 'make_foo_fn_ptr': fn() -> Foo + 592..603 'make_foo_fn': fn make_foo_fn() -> Foo + 613..618 'lazy2': Lazy Foo> + 635..644 'Lazy::new': fn new Foo>(fn() -> Foo) -> Lazy Foo> + 635..661 'Lazy::...n_ptr)': Lazy Foo> + 645..660 'make_foo_fn_ptr': fn() -> Foo + 671..673 'r2': {unknown} + 676..681 'lazy2': Lazy Foo> + 676..687 'lazy2.foo()': {unknown} + 550..552 '{}': () + "### ); } -- cgit v1.2.3