From 5ce65a1d922fd7f6440f998c39d91a1763d6453f Mon Sep 17 00:00:00 2001 From: ponyii Date: Mon, 12 Jun 2023 20:16:22 +0400 Subject: [PATCH 1/4] the "implement missing members" assist's const transformation implemented --- .../src/handlers/add_missing_impl_members.rs | 31 ++++++++++ crates/ide-db/src/path_transform.rs | 56 ++++++++++++------- 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/crates/ide-assists/src/handlers/add_missing_impl_members.rs b/crates/ide-assists/src/handlers/add_missing_impl_members.rs index ae5118e950a..4d572c604b0 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -418,6 +418,37 @@ impl<'x, 'y, T, V, U: Default> Trait<'x, 'y, T, V, U> for () { ); } + #[test] + fn test_const_substitution() { + check_assist( + add_missing_default_members, + r#" +trait Foo { + fn get_n_sq(&self, arg: &T) -> usize { N * N } +} + +struct S { + wrapped: T +} + +impl Foo for S { + $0 +}"#, + r#" +trait Foo { + fn get_n_sq(&self, arg: &T) -> usize { N * N } +} + +struct S { + wrapped: T +} + +impl Foo for S { + $0fn get_n_sq(&self, arg: &Z) -> usize { X * X } +}"#, + ) + } + #[test] fn test_cursor_after_empty_impl_def() { check_assist( diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index 6afd61e84dc..887c2feee16 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -11,7 +11,9 @@ use syntax::{ #[derive(Default)] struct AstSubsts { - types: Vec, + // ast::TypeArgs stands in fact for both type and const params + // as consts declared elsewhere look just like type params. + types_and_consts: Vec, lifetimes: Vec, } @@ -108,7 +110,7 @@ impl<'a> PathTransform<'a> { Some(hir::GenericDef::Trait(_)) => 1, _ => 0, }; - let type_substs: FxHashMap<_, _> = self + let type_and_const_substs: FxHashMap<_, _> = self .generic_def .into_iter() .flat_map(|it| it.type_params(db)) @@ -119,19 +121,17 @@ impl<'a> PathTransform<'a> { // can still hit those trailing values and check if they actually have // a default type. If they do, go for that type from `hir` to `ast` so // the resulting change can be applied correctly. - .zip(self.substs.types.iter().map(Some).chain(std::iter::repeat(None))) - .filter_map(|(k, v)| match k.split(db) { - Either::Left(_) => None, // FIXME: map const types too - Either::Right(t) => match v { - Some(v) => Some((k, v.ty()?.clone())), - None => { - let default = t.default(db)?; - let v = ast::make::ty( - &default.display_source_code(db, source_module.into(), false).ok()?, - ); - Some((k, v)) - } - }, + .zip(self.substs.types_and_consts.iter().map(Some).chain(std::iter::repeat(None))) + .filter_map(|(k, v)| match (k.split(db), v) { + (_, Some(v)) => Some((k, v.ty()?.clone())), + (Either::Right(t), None) => { + let default = t.default(db)?; + let v = ast::make::ty( + &default.display_source_code(db, source_module.into(), false).ok()?, + ); + Some((k, v)) + } + (Either::Left(_), None) => None, // FIXME: get default const value }) .collect(); let lifetime_substs: FxHashMap<_, _> = self @@ -141,12 +141,17 @@ impl<'a> PathTransform<'a> { .zip(self.substs.lifetimes.clone()) .filter_map(|(k, v)| Some((k.name(db).display(db.upcast()).to_string(), v.lifetime()?))) .collect(); - Ctx { type_substs, lifetime_substs, target_module, source_scope: self.source_scope } + Ctx { + type_and_const_substs, + lifetime_substs, + target_module, + source_scope: self.source_scope, + } } } struct Ctx<'a> { - type_substs: FxHashMap, + type_and_const_substs: FxHashMap, lifetime_substs: FxHashMap, target_module: hir::Module, source_scope: &'a SemanticsScope<'a>, @@ -203,7 +208,7 @@ impl<'a> Ctx<'a> { match resolution { hir::PathResolution::TypeParam(tp) => { - if let Some(subst) = self.type_substs.get(&tp.merge()) { + if let Some(subst) = self.type_and_const_substs.get(&tp.merge()) { let parent = path.syntax().parent()?; if let Some(parent) = ast::Path::cast(parent.clone()) { // Path inside path means that there is an associated @@ -270,8 +275,12 @@ impl<'a> Ctx<'a> { } ted::replace(path.syntax(), res.syntax()) } + hir::PathResolution::ConstParam(cp) => { + if let Some(subst) = self.type_and_const_substs.get(&cp.merge()) { + ted::replace(path.syntax(), subst.clone_subtree().clone_for_update().syntax()); + } + } hir::PathResolution::Local(_) - | hir::PathResolution::ConstParam(_) | hir::PathResolution::SelfType(_) | hir::PathResolution::Def(_) | hir::PathResolution::BuiltinAttr(_) @@ -298,9 +307,14 @@ fn get_syntactic_substs(impl_def: ast::Impl) -> Option { fn get_type_args_from_arg_list(generic_arg_list: ast::GenericArgList) -> Option { let mut result = AstSubsts::default(); generic_arg_list.generic_args().for_each(|generic_arg| match generic_arg { - ast::GenericArg::TypeArg(type_arg) => result.types.push(type_arg), + // Const params are marked as consts on definition only, + // being passed to the trait they are indistguishable from type params; + // anyway, we don't really need to distinguish them here. + ast::GenericArg::TypeArg(type_or_const_arg) => { + result.types_and_consts.push(type_or_const_arg) + } ast::GenericArg::LifetimeArg(l_arg) => result.lifetimes.push(l_arg), - _ => (), // FIXME: don't filter out const params + _ => (), }); Some(result) From b07490ffe9bdf2b4d71006bee09d2b0dc6ab0a19 Mon Sep 17 00:00:00 2001 From: ponyii Date: Wed, 14 Jun 2023 17:37:34 +0400 Subject: [PATCH 2/4] made the `add_missing_impl_members` and `add_missing_default_members` assists transform default generic types --- .../src/handlers/add_missing_impl_members.rs | 109 ++++++++++++++++++ crates/ide-db/src/path_transform.rs | 80 ++++++++----- 2 files changed, 157 insertions(+), 32 deletions(-) diff --git a/crates/ide-assists/src/handlers/add_missing_impl_members.rs b/crates/ide-assists/src/handlers/add_missing_impl_members.rs index 4d572c604b0..e827f277b1e 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -830,6 +830,115 @@ impl Foo for S { ) } + #[test] + fn test_qualify_generic_default_parameter() { + check_assist( + add_missing_impl_members, + r#" +mod m { + pub struct S; + pub trait Foo { + fn bar(&self, other: &T); + } +} + +struct S; +impl m::Foo for S { $0 }"#, + r#" +mod m { + pub struct S; + pub trait Foo { + fn bar(&self, other: &T); + } +} + +struct S; +impl m::Foo for S { + fn bar(&self, other: &m::S) { + ${0:todo!()} + } +}"#, + ) + } + + #[test] + fn test_qualify_generic_default_parameter_2() { + check_assist( + add_missing_impl_members, + r#" +mod m { + pub struct Wrapper { + one: T, + another: V + }; + pub struct S; + pub trait Foo> { + fn bar(&self, other: &T); + } +} + +struct S; +impl m::Foo for S { $0 }"#, + r#" +mod m { + pub struct Wrapper { + one: T, + another: V + }; + pub struct S; + pub trait Foo> { + fn bar(&self, other: &T); + } +} + +struct S; +impl m::Foo for S { + fn bar(&self, other: &m::Wrapper) { + ${0:todo!()} + } +}"#, + ); + } + + #[test] + fn test_qualify_generic_default_parameter_3() { + check_assist( + add_missing_impl_members, + r#" +mod m { + pub struct Wrapper { + one: T, + another: V + }; + pub struct S; + pub trait Foo> { + fn bar(&self, other: &V); + } +} + +struct S; +impl m::Foo for S { $0 }"#, + r#" +mod m { + pub struct Wrapper { + one: T, + another: V + }; + pub struct S; + pub trait Foo> { + fn bar(&self, other: &V); + } +} + +struct S; +impl m::Foo for S { + fn bar(&self, other: &m::Wrapper) { + ${0:todo!()} + } +}"#, + ); + } + #[test] fn test_assoc_type_bounds_are_removed() { check_assist( diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index 887c2feee16..f990fbd0e8f 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -17,6 +17,11 @@ struct AstSubsts { lifetimes: Vec, } +struct TypeOrConstSubst { + subst: ast::Type, + to_be_transformed: bool, +} + type LifetimeName = String; /// `PathTransform` substitutes path in SyntaxNodes in bulk. @@ -123,13 +128,21 @@ impl<'a> PathTransform<'a> { // the resulting change can be applied correctly. .zip(self.substs.types_and_consts.iter().map(Some).chain(std::iter::repeat(None))) .filter_map(|(k, v)| match (k.split(db), v) { - (_, Some(v)) => Some((k, v.ty()?.clone())), + (_, Some(v)) => { + let subst = + TypeOrConstSubst { subst: v.ty()?.clone(), to_be_transformed: false }; + Some((k, subst)) + } (Either::Right(t), None) => { let default = t.default(db)?; - let v = ast::make::ty( - &default.display_source_code(db, source_module.into(), false).ok()?, - ); - Some((k, v)) + let subst = TypeOrConstSubst { + subst: ast::make::ty( + &default.display_source_code(db, source_module.into(), false).ok()?, + ) + .clone_for_update(), + to_be_transformed: true, + }; + Some((k, subst)) } (Either::Left(_), None) => None, // FIXME: get default const value }) @@ -151,45 +164,44 @@ impl<'a> PathTransform<'a> { } struct Ctx<'a> { - type_and_const_substs: FxHashMap, + type_and_const_substs: FxHashMap, lifetime_substs: FxHashMap, target_module: hir::Module, source_scope: &'a SemanticsScope<'a>, } +fn preorder(item: &SyntaxNode) -> impl Iterator { + item.preorder().filter_map(|event| match event { + syntax::WalkEvent::Enter(_) => None, + syntax::WalkEvent::Leave(node) => Some(node), + }) +} + impl<'a> Ctx<'a> { fn apply(&self, item: &SyntaxNode) { + for (_, subst) in &self.type_and_const_substs { + if subst.to_be_transformed { + let paths = + preorder(&subst.subst.syntax()).filter_map(ast::Path::cast).collect::>(); + for path in paths { + self.transform_path(path); + } + } + } + // `transform_path` may update a node's parent and that would break the // tree traversal. Thus all paths in the tree are collected into a vec // so that such operation is safe. - let paths = item - .preorder() - .filter_map(|event| match event { - syntax::WalkEvent::Enter(_) => None, - syntax::WalkEvent::Leave(node) => Some(node), - }) - .filter_map(ast::Path::cast) - .collect::>(); - + let paths = preorder(item).filter_map(ast::Path::cast).collect::>(); for path in paths { self.transform_path(path); } - item.preorder() - .filter_map(|event| match event { - syntax::WalkEvent::Enter(_) => None, - syntax::WalkEvent::Leave(node) => Some(node), - }) - .filter_map(ast::Lifetime::cast) - .for_each(|lifetime| { - if let Some(subst) = self.lifetime_substs.get(&lifetime.syntax().text().to_string()) - { - ted::replace( - lifetime.syntax(), - subst.clone_subtree().clone_for_update().syntax(), - ); - } - }); + preorder(item).filter_map(ast::Lifetime::cast).for_each(|lifetime| { + if let Some(subst) = self.lifetime_substs.get(&lifetime.syntax().text().to_string()) { + ted::replace(lifetime.syntax(), subst.clone_subtree().clone_for_update().syntax()); + } + }); } fn transform_path(&self, path: ast::Path) -> Option<()> { @@ -208,7 +220,9 @@ impl<'a> Ctx<'a> { match resolution { hir::PathResolution::TypeParam(tp) => { - if let Some(subst) = self.type_and_const_substs.get(&tp.merge()) { + if let Some(TypeOrConstSubst { subst, .. }) = + self.type_and_const_substs.get(&tp.merge()) + { let parent = path.syntax().parent()?; if let Some(parent) = ast::Path::cast(parent.clone()) { // Path inside path means that there is an associated @@ -276,7 +290,9 @@ impl<'a> Ctx<'a> { ted::replace(path.syntax(), res.syntax()) } hir::PathResolution::ConstParam(cp) => { - if let Some(subst) = self.type_and_const_substs.get(&cp.merge()) { + if let Some(TypeOrConstSubst { subst, .. }) = + self.type_and_const_substs.get(&cp.merge()) + { ted::replace(path.syntax(), subst.clone_subtree().clone_for_update().syntax()); } } From 8a3c21442e7000cb3b70b92c220f71ceeed1abe1 Mon Sep 17 00:00:00 2001 From: ponyii Date: Thu, 15 Jun 2023 17:56:08 +0400 Subject: [PATCH 3/4] refactoring --- crates/ide-db/src/path_transform.rs | 96 ++++++++++++++--------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index f990fbd0e8f..d280185c4ce 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -17,11 +17,6 @@ struct AstSubsts { lifetimes: Vec, } -struct TypeOrConstSubst { - subst: ast::Type, - to_be_transformed: bool, -} - type LifetimeName = String; /// `PathTransform` substitutes path in SyntaxNodes in bulk. @@ -115,8 +110,10 @@ impl<'a> PathTransform<'a> { Some(hir::GenericDef::Trait(_)) => 1, _ => 0, }; - let type_and_const_substs: FxHashMap<_, _> = self - .generic_def + let mut type_substs: FxHashMap = Default::default(); + let mut const_substs: FxHashMap = Default::default(); + let mut default_types: Vec = Default::default(); + self.generic_def .into_iter() .flat_map(|it| it.type_params(db)) .skip(skip) @@ -127,26 +124,29 @@ impl<'a> PathTransform<'a> { // a default type. If they do, go for that type from `hir` to `ast` so // the resulting change can be applied correctly. .zip(self.substs.types_and_consts.iter().map(Some).chain(std::iter::repeat(None))) - .filter_map(|(k, v)| match (k.split(db), v) { - (_, Some(v)) => { - let subst = - TypeOrConstSubst { subst: v.ty()?.clone(), to_be_transformed: false }; - Some((k, subst)) + .for_each(|(k, v)| match (k.split(db), v) { + (Either::Right(t), Some(v)) => { + if let Some(ty) = v.ty() { + type_substs.insert(t, ty.clone()); + } } (Either::Right(t), None) => { - let default = t.default(db)?; - let subst = TypeOrConstSubst { - subst: ast::make::ty( - &default.display_source_code(db, source_module.into(), false).ok()?, - ) - .clone_for_update(), - to_be_transformed: true, - }; - Some((k, subst)) + if let Some(default) = t.default(db) { + if let Some(default) = + &default.display_source_code(db, source_module.into(), false).ok() + { + type_substs.insert(t, ast::make::ty(default).clone_for_update()); + default_types.push(t); + } + } } - (Either::Left(_), None) => None, // FIXME: get default const value - }) - .collect(); + (Either::Left(c), Some(v)) => { + if let Some(ty) = v.ty() { + const_substs.insert(c, ty.syntax().clone()); + } + } + (Either::Left(_), None) => (), // FIXME: get default const value + }); let lifetime_substs: FxHashMap<_, _> = self .generic_def .into_iter() @@ -154,23 +154,27 @@ impl<'a> PathTransform<'a> { .zip(self.substs.lifetimes.clone()) .filter_map(|(k, v)| Some((k.name(db).display(db.upcast()).to_string(), v.lifetime()?))) .collect(); - Ctx { - type_and_const_substs, + let ctx = Ctx { + type_substs, + const_substs, lifetime_substs, target_module, source_scope: self.source_scope, - } + }; + ctx.transform_default_type_substs(default_types); + ctx } } struct Ctx<'a> { - type_and_const_substs: FxHashMap, + type_substs: FxHashMap, + const_substs: FxHashMap, lifetime_substs: FxHashMap, target_module: hir::Module, source_scope: &'a SemanticsScope<'a>, } -fn preorder(item: &SyntaxNode) -> impl Iterator { +fn postorder(item: &SyntaxNode) -> impl Iterator { item.preorder().filter_map(|event| match event { syntax::WalkEvent::Enter(_) => None, syntax::WalkEvent::Leave(node) => Some(node), @@ -179,31 +183,31 @@ fn preorder(item: &SyntaxNode) -> impl Iterator { impl<'a> Ctx<'a> { fn apply(&self, item: &SyntaxNode) { - for (_, subst) in &self.type_and_const_substs { - if subst.to_be_transformed { - let paths = - preorder(&subst.subst.syntax()).filter_map(ast::Path::cast).collect::>(); - for path in paths { - self.transform_path(path); - } - } - } - // `transform_path` may update a node's parent and that would break the // tree traversal. Thus all paths in the tree are collected into a vec // so that such operation is safe. - let paths = preorder(item).filter_map(ast::Path::cast).collect::>(); + let paths = postorder(item).filter_map(ast::Path::cast).collect::>(); for path in paths { self.transform_path(path); } - preorder(item).filter_map(ast::Lifetime::cast).for_each(|lifetime| { + postorder(item).filter_map(ast::Lifetime::cast).for_each(|lifetime| { if let Some(subst) = self.lifetime_substs.get(&lifetime.syntax().text().to_string()) { ted::replace(lifetime.syntax(), subst.clone_subtree().clone_for_update().syntax()); } }); } + fn transform_default_type_substs(&self, default_types: Vec) { + for k in default_types { + let v = self.type_substs.get(&k).unwrap(); + let paths = postorder(&v.syntax()).filter_map(ast::Path::cast).collect::>(); + for path in paths { + self.transform_path(path); + } + } + } + fn transform_path(&self, path: ast::Path) -> Option<()> { if path.qualifier().is_some() { return None; @@ -220,9 +224,7 @@ impl<'a> Ctx<'a> { match resolution { hir::PathResolution::TypeParam(tp) => { - if let Some(TypeOrConstSubst { subst, .. }) = - self.type_and_const_substs.get(&tp.merge()) - { + if let Some(subst) = self.type_substs.get(&tp) { let parent = path.syntax().parent()?; if let Some(parent) = ast::Path::cast(parent.clone()) { // Path inside path means that there is an associated @@ -290,10 +292,8 @@ impl<'a> Ctx<'a> { ted::replace(path.syntax(), res.syntax()) } hir::PathResolution::ConstParam(cp) => { - if let Some(TypeOrConstSubst { subst, .. }) = - self.type_and_const_substs.get(&cp.merge()) - { - ted::replace(path.syntax(), subst.clone_subtree().clone_for_update().syntax()); + if let Some(subst) = self.const_substs.get(&cp) { + ted::replace(path.syntax(), subst.clone_subtree().clone_for_update()); } } hir::PathResolution::Local(_) From 7e08933a26453b3075bc870814fa35bd263ef9c2 Mon Sep 17 00:00:00 2001 From: ponyii Date: Thu, 15 Jun 2023 19:04:59 +0400 Subject: [PATCH 4/4] the "implement missing members" assist's const transformation patched --- .../src/handlers/add_missing_impl_members.rs | 35 ++++++++++++++ .../ide-assists/src/handlers/inline_call.rs | 3 +- crates/ide-db/src/path_transform.rs | 47 ++++++++++++++----- 3 files changed, 70 insertions(+), 15 deletions(-) diff --git a/crates/ide-assists/src/handlers/add_missing_impl_members.rs b/crates/ide-assists/src/handlers/add_missing_impl_members.rs index e827f277b1e..b61f052a1e0 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -423,8 +423,13 @@ impl<'x, 'y, T, V, U: Default> Trait<'x, 'y, T, V, U> for () { check_assist( add_missing_default_members, r#" +struct Bar { + bar: [i32, N] +} + trait Foo { fn get_n_sq(&self, arg: &T) -> usize { N * N } + fn get_array(&self, arg: Bar) -> [i32; N] { [1; N] } } struct S { @@ -435,8 +440,13 @@ impl Foo for S { $0 }"#, r#" +struct Bar { + bar: [i32, N] +} + trait Foo { fn get_n_sq(&self, arg: &T) -> usize { N * N } + fn get_array(&self, arg: Bar) -> [i32; N] { [1; N] } } struct S { @@ -445,6 +455,31 @@ struct S { impl Foo for S { $0fn get_n_sq(&self, arg: &Z) -> usize { X * X } + + fn get_array(&self, arg: Bar) -> [i32; X] { [1; X] } +}"#, + ) + } + + #[test] + fn test_const_substitution_2() { + check_assist( + add_missing_default_members, + r#" +trait Foo { + fn get_sum(&self, arg: &T) -> usize { N + M } +} + +impl Foo<42, {20 + 22}, X> for () { + $0 +}"#, + r#" +trait Foo { + fn get_sum(&self, arg: &T) -> usize { N + M } +} + +impl Foo<42, {20 + 22}, X> for () { + $0fn get_sum(&self, arg: &X) -> usize { 42 + {20 + 22} } }"#, ) } diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index 28d815e81b4..797180fa189 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -958,7 +958,6 @@ fn main() { ); } - // FIXME: const generics aren't being substituted, this is blocked on better support for them #[test] fn inline_substitutes_generics() { check_assist( @@ -982,7 +981,7 @@ fn foo() { fn bar() {} fn main() { - bar::(); + bar::(); } "#, ); diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index d280185c4ce..73e6a920ee4 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -11,12 +11,15 @@ use syntax::{ #[derive(Default)] struct AstSubsts { - // ast::TypeArgs stands in fact for both type and const params - // as consts declared elsewhere look just like type params. - types_and_consts: Vec, + types_and_consts: Vec, lifetimes: Vec, } +enum TypeOrConst { + Either(ast::TypeArg), // indistinguishable type or const param + Const(ast::ConstArg), +} + type LifetimeName = String; /// `PathTransform` substitutes path in SyntaxNodes in bulk. @@ -125,27 +128,38 @@ impl<'a> PathTransform<'a> { // the resulting change can be applied correctly. .zip(self.substs.types_and_consts.iter().map(Some).chain(std::iter::repeat(None))) .for_each(|(k, v)| match (k.split(db), v) { - (Either::Right(t), Some(v)) => { + (Either::Right(k), Some(TypeOrConst::Either(v))) => { if let Some(ty) = v.ty() { - type_substs.insert(t, ty.clone()); + type_substs.insert(k, ty.clone()); } } - (Either::Right(t), None) => { - if let Some(default) = t.default(db) { + (Either::Right(k), None) => { + if let Some(default) = k.default(db) { if let Some(default) = &default.display_source_code(db, source_module.into(), false).ok() { - type_substs.insert(t, ast::make::ty(default).clone_for_update()); - default_types.push(t); + type_substs.insert(k, ast::make::ty(default).clone_for_update()); + default_types.push(k); } } } - (Either::Left(c), Some(v)) => { + (Either::Left(k), Some(TypeOrConst::Either(v))) => { if let Some(ty) = v.ty() { - const_substs.insert(c, ty.syntax().clone()); + const_substs.insert(k, ty.syntax().clone()); + } + } + (Either::Left(k), Some(TypeOrConst::Const(v))) => { + if let Some(expr) = v.expr() { + // FIXME: expressions in curly brackets can cause ambiguity after insertion + // (e.g. `N * 2` -> `{1 + 1} * 2`; it's unclear whether `{1 + 1}` + // is a standalone statement or a part of another expresson) + // and sometimes require slight modifications; see + // https://doc.rust-lang.org/reference/statements.html#expression-statements + const_substs.insert(k, expr.syntax().clone()); } } (Either::Left(_), None) => (), // FIXME: get default const value + _ => (), // ignore mismatching params }); let lifetime_substs: FxHashMap<_, _> = self .generic_def @@ -201,6 +215,9 @@ impl<'a> Ctx<'a> { fn transform_default_type_substs(&self, default_types: Vec) { for k in default_types { let v = self.type_substs.get(&k).unwrap(); + // `transform_path` may update a node's parent and that would break the + // tree traversal. Thus all paths in the tree are collected into a vec + // so that such operation is safe. let paths = postorder(&v.syntax()).filter_map(ast::Path::cast).collect::>(); for path in paths { self.transform_path(path); @@ -326,8 +343,12 @@ fn get_type_args_from_arg_list(generic_arg_list: ast::GenericArgList) -> Option< // Const params are marked as consts on definition only, // being passed to the trait they are indistguishable from type params; // anyway, we don't really need to distinguish them here. - ast::GenericArg::TypeArg(type_or_const_arg) => { - result.types_and_consts.push(type_or_const_arg) + ast::GenericArg::TypeArg(type_arg) => { + result.types_and_consts.push(TypeOrConst::Either(type_arg)) + } + // Some const values are recognized correctly. + ast::GenericArg::ConstArg(const_arg) => { + result.types_and_consts.push(TypeOrConst::Const(const_arg)); } ast::GenericArg::LifetimeArg(l_arg) => result.lifetimes.push(l_arg), _ => (),