From 52b43927240ca8c77ef24b92a580b9e9291e17f8 Mon Sep 17 00:00:00 2001 From: ponyii Date: Fri, 30 Jun 2023 17:31:04 +0400 Subject: [PATCH 1/6] the "add missing members" assists: implemented substitution of default values of const params --- crates/hir-def/src/generics.rs | 8 +-- crates/hir-def/src/hir/type_ref.rs | 11 +++ crates/hir-ty/src/lib.rs | 12 +++- crates/hir-ty/src/lower.rs | 71 +++++++++---------- crates/hir/src/lib.rs | 23 ++++-- .../src/handlers/add_missing_impl_members.rs | 39 +++++++++- crates/ide-db/src/path_transform.rs | 10 ++- crates/syntax/src/ast/make.rs | 5 ++ 8 files changed, 127 insertions(+), 52 deletions(-) diff --git a/crates/hir-def/src/generics.rs b/crates/hir-def/src/generics.rs index 540fa115a00..531a503c1b6 100644 --- a/crates/hir-def/src/generics.rs +++ b/crates/hir-def/src/generics.rs @@ -25,7 +25,7 @@ use crate::{ lower::LowerCtx, nameres::{DefMap, MacroSubNs}, src::{HasChildSource, HasSource}, - type_ref::{LifetimeRef, TypeBound, TypeRef}, + type_ref::{ConstRef, LifetimeRef, TypeBound, TypeRef}, AdtId, ConstParamId, GenericDefId, HasModule, LifetimeParamId, LocalLifetimeParamId, LocalTypeOrConstParamId, Lookup, TypeOrConstParamId, TypeParamId, }; @@ -49,7 +49,7 @@ pub struct LifetimeParamData { pub struct ConstParamData { pub name: Name, pub ty: Interned, - pub has_default: bool, + pub default: Option, } #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] @@ -76,7 +76,7 @@ impl TypeOrConstParamData { pub fn has_default(&self) -> bool { match self { TypeOrConstParamData::TypeParamData(it) => it.default.is_some(), - TypeOrConstParamData::ConstParamData(it) => it.has_default, + TypeOrConstParamData::ConstParamData(it) => it.default.is_some(), } } @@ -307,7 +307,7 @@ impl GenericParams { let param = ConstParamData { name, ty: Interned::new(ty), - has_default: const_param.default_val().is_some(), + default: ConstRef::from_default_param_value(lower_ctx, const_param), }; let idx = self.type_or_consts.alloc(param.into()); add_param_attrs(idx.into(), ast::GenericParam::ConstParam(const_param)); diff --git a/crates/hir-def/src/hir/type_ref.rs b/crates/hir-def/src/hir/type_ref.rs index 57f023ef35d..e2d24625cea 100644 --- a/crates/hir-def/src/hir/type_ref.rs +++ b/crates/hir-def/src/hir/type_ref.rs @@ -393,6 +393,17 @@ impl ConstRef { Self::Scalar(LiteralConstRef::Unknown) } + pub(crate) fn from_default_param_value( + _: &LowerCtx<'_>, + param: ast::ConstParam, + ) -> Option { + if let Some(expr) = param.default_val() { + // FIXME: pass the `ast_id` arg to recognize complex expressions + return Some(Self::from_expr(expr, None)); + } + None + } + pub fn display<'a>(&'a self, db: &'a dyn ExpandDatabase) -> impl fmt::Display + 'a { struct Display<'a>(&'a dyn ExpandDatabase, &'a ConstRef); impl fmt::Display for Display<'_> { diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index b3ca2a22258..1595622d2b2 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -57,7 +57,8 @@ use triomphe::Arc; use utils::Generics; use crate::{ - consteval::unknown_const, db::HirDatabase, infer::unify::InferenceTable, utils::generics, + consteval::unknown_const, db::HirDatabase, display::HirDisplay, infer::unify::InferenceTable, + utils::generics, }; pub use autoderef::autoderef; @@ -719,3 +720,12 @@ where value.visit_with(&mut collector, DebruijnIndex::INNERMOST); collector.placeholders.into_iter().collect() } + +pub fn known_const_to_string(konst: &Const, db: &dyn HirDatabase) -> Option { + if let ConstValue::Concrete(c) = &konst.interned().value { + if c.interned == ConstScalar::Unknown { + return None; + } + } + Some(konst.display(db).to_string()) +} diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index 2837f400bce..419a50ebe5f 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -213,6 +213,19 @@ impl<'a> TyLoweringContext<'a> { self.lower_ty_ext(type_ref).0 } + pub fn lower_const(&self, const_ref: &ConstRef, const_type: Ty) -> Const { + const_or_path_to_chalk( + self.db, + self.resolver, + self.owner, + const_type, + const_ref, + self.type_param_mode, + || self.generics(), + self.in_binders, + ) + } + fn generics(&self) -> Generics { generics( self.db.upcast(), @@ -242,17 +255,7 @@ impl<'a> TyLoweringContext<'a> { } TypeRef::Array(inner, len) => { let inner_ty = self.lower_ty(inner); - let const_len = const_or_path_to_chalk( - self.db, - self.resolver, - self.owner, - TyBuilder::usize(), - len, - self.type_param_mode, - || self.generics(), - self.in_binders, - ); - + let const_len = self.lower_const(len, TyBuilder::usize()); TyKind::Array(inner_ty, const_len).intern(Interner) } TypeRef::Slice(inner) => { @@ -847,18 +850,7 @@ impl<'a> TyLoweringContext<'a> { arg, &mut (), |_, type_ref| self.lower_ty(type_ref), - |_, c, ty| { - const_or_path_to_chalk( - self.db, - self.resolver, - self.owner, - ty, - c, - self.type_param_mode, - || self.generics(), - self.in_binders, - ) - }, + |_, const_ref, ty| self.lower_const(const_ref, ty), ) { had_explicit_args = true; substs.push(x); @@ -1604,24 +1596,31 @@ pub(crate) fn generic_defaults_query( .iter() .enumerate() .map(|(idx, (id, p))| { - let p = match p { - TypeOrConstParamData::TypeParamData(p) => p, - TypeOrConstParamData::ConstParamData(_) => { - // FIXME: implement const generic defaults - let val = unknown_const_as_generic( + match p { + TypeOrConstParamData::TypeParamData(p) => { + let mut ty = p + .default + .as_ref() + .map_or(TyKind::Error.intern(Interner), |t| ctx.lower_ty(t)); + // Each default can only refer to previous parameters. + // Type variable default referring to parameter coming + // after it is forbidden (FIXME: report diagnostic) + ty = fallback_bound_vars(ty, idx, parent_start_idx); + return crate::make_binders(db, &generic_params, ty.cast(Interner)); + } + TypeOrConstParamData::ConstParamData(p) => { + let unknown = unknown_const_as_generic( db.const_param_ty(ConstParamId::from_unchecked(id)), ); + let val = p.default.as_ref().map_or(unknown, |c| { + let c = ctx.lower_const(c, ctx.lower_ty(&p.ty)); + chalk_ir::GenericArg::new(Interner, GenericArgData::Const(c)) + }); + // FIXME: check if complex default values refer to + // previous parameters they should not. return make_binders(db, &generic_params, val); } }; - let mut ty = - p.default.as_ref().map_or(TyKind::Error.intern(Interner), |t| ctx.lower_ty(t)); - - // Each default can only refer to previous parameters. - // Type variable default referring to parameter coming - // after it is forbidden (FIXME: report diagnostic) - ty = fallback_bound_vars(ty, idx, parent_start_idx); - crate::make_binders(db, &generic_params, ty.cast(Interner)) }) // FIXME: use `Arc::from_iter` when it becomes available .collect::>(), diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index de60c88844d..b577b3fb327 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -62,13 +62,13 @@ use hir_expand::{name::name, MacroCallKind}; use hir_ty::{ all_super_traits, autoderef, consteval::{try_const_usize, unknown_const_as_generic, ConstEvalError, ConstExt}, - diagnostics::BodyValidationDiagnostic, + diagnostics::BodyValidationDiagnostic, known_const_to_string, layout::{Layout as TyLayout, RustcEnumVariantIdx, TagEncoding}, method_resolution::{self, TyFingerprint}, mir::{self, interpret_mir}, primitive::UintTy, traits::FnTrait, - AliasTy, CallableDefId, CallableSig, Canonical, CanonicalVarKinds, Cast, ClosureId, + AliasTy, CallableDefId, CallableSig, Canonical, CanonicalVarKinds, Cast, ClosureId, GenericArg, GenericArgData, Interner, ParamKind, QuantifiedWhereClause, Scalar, Substitution, TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyDefId, TyExt, TyKind, ValueTyDefId, WhereClause, @@ -3142,12 +3142,8 @@ impl TypeParam { } pub fn default(self, db: &dyn HirDatabase) -> Option { - let params = db.generic_defaults(self.id.parent()); - let local_idx = hir_ty::param_idx(db, self.id.into())?; + let ty = generic_arg_from_param(db, self.id.into())?; let resolver = self.id.parent().resolver(db.upcast()); - let ty = params.get(local_idx)?.clone(); - let subst = TyBuilder::placeholder_subst(db, self.id.parent()); - let ty = ty.substitute(Interner, &subst); match ty.data(Interner) { GenericArgData::Ty(it) => { Some(Type::new_with_resolver_inner(db, &resolver, it.clone())) @@ -3209,6 +3205,19 @@ impl ConstParam { pub fn ty(self, db: &dyn HirDatabase) -> Type { Type::new(db, self.id.parent(), db.const_param_ty(self.id)) } + + pub fn default(self, db: &dyn HirDatabase) -> Option { + let arg = generic_arg_from_param(db, self.id.into())?; + known_const_to_string(arg.constant(Interner)?, db) + } +} + +fn generic_arg_from_param(db: &dyn HirDatabase, id: TypeOrConstParamId) -> Option { + let params = db.generic_defaults(id.parent); + let local_idx = hir_ty::param_idx(db, id)?; + let ty = params.get(local_idx)?.clone(); + let subst = TyBuilder::placeholder_subst(db, id.parent); + Some(ty.substitute(Interner, &subst)) } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] 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 6aca716bb60..ea659a2295d 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -422,7 +422,7 @@ impl<'x, 'y, T, V, U: Default> Trait<'x, 'y, T, V, U> for () { check_assist( add_missing_default_members, r#" -struct Bar { +struct Bar { bar: [i32, N] } @@ -439,7 +439,7 @@ impl Foo for S { $0 }"#, r#" -struct Bar { +struct Bar { bar: [i32, N] } @@ -483,6 +483,41 @@ impl Foo<42, {20 + 22}, X> for () { ) } + #[test] + fn test_const_substitution_with_defaults() { + check_assist( + add_missing_default_members, + r#" +trait Foo { + fn get_n(&self) -> usize { N } + fn get_m(&self) -> bool { M } + fn get_p(&self) -> char { P } + fn get_array(&self, arg: &T) -> [bool; N] { [M; N] } +} + +impl Foo for () { + $0 +}"#, + r#" +trait Foo { + fn get_n(&self) -> usize { N } + fn get_m(&self) -> bool { M } + fn get_p(&self) -> char { P } + fn get_array(&self, arg: &T) -> [bool; N] { [M; N] } +} + +impl Foo for () { + $0fn get_n(&self) -> usize { 42 } + + fn get_m(&self) -> bool { false } + + fn get_p(&self) -> char { 'a' } + + fn get_array(&self, arg: &X) -> [bool; 42] { [false; 42] } +}"#, + ); + } + #[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 1d0cb426a57..cb04a0381bf 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -158,8 +158,14 @@ impl<'a> PathTransform<'a> { const_substs.insert(k, expr.syntax().clone()); } } - (Either::Left(_), None) => (), // FIXME: get default const value - _ => (), // ignore mismatching params + (Either::Left(k), None) => { + if let Some(default) = k.default(db) { + let default = ast::make::expr_const_value(&default); + const_substs.insert(k, default.syntax().clone_for_update()); + // FIXME: transform the default value + } + } + _ => (), // ignore mismatching params }); let lifetime_substs: FxHashMap<_, _> = self .generic_def diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 4c6db0ef06c..1eefd949050 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -503,11 +503,16 @@ pub fn hacky_block_expr( pub fn expr_unit() -> ast::Expr { expr_from_text("()") } + pub fn expr_literal(text: &str) -> ast::Literal { assert_eq!(text.trim(), text); ast_from_text(&format!("fn f() {{ let _ = {text}; }}")) } +pub fn expr_const_value(text: &str) -> ast::Expr { + ast_from_text(&format!("trait Foo {{}}")) +} + pub fn expr_empty_block() -> ast::Expr { expr_from_text("{}") } From 4ebdc6f0528365f32db4a529407d26a169c36617 Mon Sep 17 00:00:00 2001 From: ponyii Date: Mon, 10 Jul 2023 17:56:07 +0400 Subject: [PATCH 2/6] syntax update: the default value of `ConstParam` turned from `Expr` into `ConstArg` --- crates/hir-def/src/generics.rs | 2 +- crates/hir-def/src/hir/type_ref.rs | 12 ++++++------ crates/hir-ty/src/lib.rs | 4 ++++ crates/hir/src/lib.rs | 3 ++- .../src/handlers/extract_function.rs | 2 +- crates/ide-db/src/path_transform.rs | 7 ++++--- crates/parser/src/grammar/generic_params.rs | 2 +- ...0022_recover_from_missing_const_default.rast | 6 ++++-- .../ok/0188_const_param_default_path.rast | 15 ++++++++------- .../ok/0199_const_param_default_expression.rast | 17 +++++++++-------- .../ok/0200_const_param_default_literal.rast | 9 +++++---- crates/syntax/rust.ungram | 2 +- crates/syntax/src/ast/generated/nodes.rs | 2 +- crates/syntax/src/ast/make.rs | 2 +- 14 files changed, 48 insertions(+), 37 deletions(-) diff --git a/crates/hir-def/src/generics.rs b/crates/hir-def/src/generics.rs index 531a503c1b6..7ee27d26709 100644 --- a/crates/hir-def/src/generics.rs +++ b/crates/hir-def/src/generics.rs @@ -307,7 +307,7 @@ impl GenericParams { let param = ConstParamData { name, ty: Interned::new(ty), - default: ConstRef::from_default_param_value(lower_ctx, const_param), + default: ConstRef::from_const_param(lower_ctx, const_param), }; let idx = self.type_or_consts.alloc(param.into()); add_param_attrs(idx.into(), ast::GenericParam::ConstParam(const_param)); diff --git a/crates/hir-def/src/hir/type_ref.rs b/crates/hir-def/src/hir/type_ref.rs index e2d24625cea..c518f1b75b3 100644 --- a/crates/hir-def/src/hir/type_ref.rs +++ b/crates/hir-def/src/hir/type_ref.rs @@ -393,15 +393,15 @@ impl ConstRef { Self::Scalar(LiteralConstRef::Unknown) } - pub(crate) fn from_default_param_value( - _: &LowerCtx<'_>, + pub(crate) fn from_const_param( + lower_ctx: &LowerCtx<'_>, param: ast::ConstParam, ) -> Option { - if let Some(expr) = param.default_val() { - // FIXME: pass the `ast_id` arg to recognize complex expressions - return Some(Self::from_expr(expr, None)); + let default = param.default_val(); + match default { + Some(_) => Some(Self::from_const_arg(lower_ctx, default)), + None => None, } - None } pub fn display<'a>(&'a self, db: &'a dyn ExpandDatabase) -> impl fmt::Display + 'a { diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index 1595622d2b2..14346c27941 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -723,6 +723,10 @@ where pub fn known_const_to_string(konst: &Const, db: &dyn HirDatabase) -> Option { if let ConstValue::Concrete(c) = &konst.interned().value { + if let ConstScalar::UnevaluatedConst(GeneralConstId::InTypeConstId(_), _) = &c.interned { + // FIXME: stringify the block expression + return None; + } if c.interned == ConstScalar::Unknown { return None; } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index b577b3fb327..136b1b08533 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -62,7 +62,8 @@ use hir_expand::{name::name, MacroCallKind}; use hir_ty::{ all_super_traits, autoderef, consteval::{try_const_usize, unknown_const_as_generic, ConstEvalError, ConstExt}, - diagnostics::BodyValidationDiagnostic, known_const_to_string, + diagnostics::BodyValidationDiagnostic, + known_const_to_string, layout::{Layout as TyLayout, RustcEnumVariantIdx, TagEncoding}, method_resolution::{self, TyFingerprint}, mir::{self, interpret_mir}, diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index 7f61a68284c..1340681ccab 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -810,7 +810,7 @@ impl FunctionBody { (true, konst.body(), Some(sema.to_def(&konst)?.ty(sema.db))) }, ast::ConstParam(cp) => { - (true, cp.default_val(), Some(sema.to_def(&cp)?.ty(sema.db))) + (true, cp.default_val()?.expr(), Some(sema.to_def(&cp)?.ty(sema.db))) }, ast::ConstBlockPat(cbp) => { let expr = cbp.block_expr().map(ast::Expr::BlockExpr); diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index cb04a0381bf..507200ea3ba 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -160,9 +160,10 @@ impl<'a> PathTransform<'a> { } (Either::Left(k), None) => { if let Some(default) = k.default(db) { - let default = ast::make::expr_const_value(&default); - const_substs.insert(k, default.syntax().clone_for_update()); - // FIXME: transform the default value + if let Some(default) = ast::make::expr_const_value(&default).expr() { + const_substs.insert(k, default.syntax().clone_for_update()); + // FIXME: transform the default value + } } } _ => (), // ignore mismatching params diff --git a/crates/parser/src/grammar/generic_params.rs b/crates/parser/src/grammar/generic_params.rs index 8ed1c84c4c6..29d9b05d3f3 100644 --- a/crates/parser/src/grammar/generic_params.rs +++ b/crates/parser/src/grammar/generic_params.rs @@ -88,7 +88,7 @@ fn const_param(p: &mut Parser<'_>, m: Marker) { // test const_param_default_path // struct A; - generic_args::const_arg_expr(p); + generic_args::const_arg(p); } m.complete(p, CONST_PARAM); diff --git a/crates/parser/test_data/parser/inline/err/0022_recover_from_missing_const_default.rast b/crates/parser/test_data/parser/inline/err/0022_recover_from_missing_const_default.rast index 809ad1b8d5b..49f163b164a 100644 --- a/crates/parser/test_data/parser/inline/err/0022_recover_from_missing_const_default.rast +++ b/crates/parser/test_data/parser/inline/err/0022_recover_from_missing_const_default.rast @@ -20,7 +20,8 @@ SOURCE_FILE IDENT "i32" WHITESPACE " " EQ "=" - WHITESPACE " " + WHITESPACE " " + CONST_ARG COMMA "," WHITESPACE " " CONST_PARAM @@ -37,8 +38,9 @@ SOURCE_FILE IDENT "i32" WHITESPACE " " EQ "=" + CONST_ARG R_ANGLE ">" SEMICOLON ";" WHITESPACE "\n" -error 23: expected a generic const argument +error 24: expected a generic const argument error 40: expected a generic const argument diff --git a/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rast b/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rast index 11002bf98d0..3f5fb47d287 100644 --- a/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rast +++ b/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rast @@ -21,16 +21,17 @@ SOURCE_FILE WHITESPACE " " EQ "=" WHITESPACE " " - PATH_EXPR - PATH + CONST_ARG + PATH_EXPR PATH + PATH + PATH_SEGMENT + NAME_REF + IDENT "i32" + COLON2 "::" PATH_SEGMENT NAME_REF - IDENT "i32" - COLON2 "::" - PATH_SEGMENT - NAME_REF - IDENT "MAX" + IDENT "MAX" R_ANGLE ">" SEMICOLON ";" WHITESPACE "\n" diff --git a/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rast b/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rast index 0607ff54fbb..d6501137498 100644 --- a/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rast +++ b/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rast @@ -21,14 +21,15 @@ SOURCE_FILE WHITESPACE " " EQ "=" WHITESPACE " " - BLOCK_EXPR - STMT_LIST - L_CURLY "{" - WHITESPACE " " - LITERAL - INT_NUMBER "1" - WHITESPACE " " - R_CURLY "}" + CONST_ARG + BLOCK_EXPR + STMT_LIST + L_CURLY "{" + WHITESPACE " " + LITERAL + INT_NUMBER "1" + WHITESPACE " " + R_CURLY "}" R_ANGLE ">" SEMICOLON ";" WHITESPACE "\n" diff --git a/crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rast b/crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rast index 8e52313651c..6de10353bf0 100644 --- a/crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rast +++ b/crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rast @@ -21,10 +21,11 @@ SOURCE_FILE WHITESPACE " " EQ "=" WHITESPACE " " - PREFIX_EXPR - MINUS "-" - LITERAL - INT_NUMBER "1" + CONST_ARG + PREFIX_EXPR + MINUS "-" + LITERAL + INT_NUMBER "1" R_ANGLE ">" SEMICOLON ";" WHITESPACE "\n" diff --git a/crates/syntax/rust.ungram b/crates/syntax/rust.ungram index 138ddd20897..ea7ebd85b33 100644 --- a/crates/syntax/rust.ungram +++ b/crates/syntax/rust.ungram @@ -296,7 +296,7 @@ TypeParam = ConstParam = Attr* 'const' Name ':' Type - ('=' default_val:Expr)? + ('=' default_val:ConstArg)? LifetimeParam = Attr* Lifetime (':' TypeBoundList?)? diff --git a/crates/syntax/src/ast/generated/nodes.rs b/crates/syntax/src/ast/generated/nodes.rs index 0b27faa535d..16448db04f8 100644 --- a/crates/syntax/src/ast/generated/nodes.rs +++ b/crates/syntax/src/ast/generated/nodes.rs @@ -709,7 +709,7 @@ impl ConstParam { pub fn colon_token(&self) -> Option { support::token(&self.syntax, T![:]) } pub fn ty(&self) -> Option { support::child(&self.syntax) } pub fn eq_token(&self) -> Option { support::token(&self.syntax, T![=]) } - pub fn default_val(&self) -> Option { support::child(&self.syntax) } + pub fn default_val(&self) -> Option { support::child(&self.syntax) } } #[derive(Debug, Clone, PartialEq, Eq, Hash)] diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 1eefd949050..217134385af 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -509,7 +509,7 @@ pub fn expr_literal(text: &str) -> ast::Literal { ast_from_text(&format!("fn f() {{ let _ = {text}; }}")) } -pub fn expr_const_value(text: &str) -> ast::Expr { +pub fn expr_const_value(text: &str) -> ast::ConstArg { ast_from_text(&format!("trait Foo {{}}")) } From 4e2be8e959179f8b7f9614e1d147b878bbd2f071 Mon Sep 17 00:00:00 2001 From: ponyii Date: Tue, 11 Jul 2023 18:46:39 +0400 Subject: [PATCH 3/6] the "add missing members" assists: implemented the transformation of const param default values --- crates/hir-ty/src/lower.rs | 6 ++-- .../src/handlers/add_missing_impl_members.rs | 31 +++++++++++++++++++ crates/ide-db/src/path_transform.rs | 23 +++++++++----- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index 419a50ebe5f..a885c32e7f9 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -1612,12 +1612,12 @@ pub(crate) fn generic_defaults_query( let unknown = unknown_const_as_generic( db.const_param_ty(ConstParamId::from_unchecked(id)), ); - let val = p.default.as_ref().map_or(unknown, |c| { + let mut val = p.default.as_ref().map_or(unknown, |c| { let c = ctx.lower_const(c, ctx.lower_ty(&p.ty)); chalk_ir::GenericArg::new(Interner, GenericArgData::Const(c)) }); - // FIXME: check if complex default values refer to - // previous parameters they should not. + // Each default can only refer to previous parameters, see above. + val = fallback_bound_vars(val, idx, parent_start_idx); return make_binders(db, &generic_params, val); } }; 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 ea659a2295d..d9faf3a7f6c 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -518,6 +518,37 @@ impl Foo for () { ); } + #[test] + fn test_const_substitution_with_defaults_2() { + check_assist( + add_missing_impl_members, + r#" +mod m { + pub const LEN: usize = 42; + pub trait Foo { + fn get_t(&self) -> T; + } +} + +impl m::Foo for () { + $0 +}"#, + r#" +mod m { + pub const LEN: usize = 42; + pub trait Foo { + fn get_t(&self) -> T; + } +} + +impl m::Foo for () { + fn get_t(&self) -> [bool; m::LEN] { + ${0:todo!()} + } +}"#, + ) + } + #[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 507200ea3ba..efe13f06040 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -21,6 +21,7 @@ enum TypeOrConst { } type LifetimeName = String; +type DefaultedParam = Either; /// `PathTransform` substitutes path in SyntaxNodes in bulk. /// @@ -115,7 +116,7 @@ impl<'a> PathTransform<'a> { }; let mut type_substs: FxHashMap = Default::default(); let mut const_substs: FxHashMap = Default::default(); - let mut default_types: Vec = Default::default(); + let mut defaulted_params: Vec = Default::default(); self.generic_def .into_iter() .flat_map(|it| it.type_params(db)) @@ -139,7 +140,7 @@ impl<'a> PathTransform<'a> { &default.display_source_code(db, source_module.into(), false).ok() { type_substs.insert(k, ast::make::ty(default).clone_for_update()); - default_types.push(k); + defaulted_params.push(Either::Left(k)); } } } @@ -162,7 +163,7 @@ impl<'a> PathTransform<'a> { if let Some(default) = k.default(db) { if let Some(default) = ast::make::expr_const_value(&default).expr() { const_substs.insert(k, default.syntax().clone_for_update()); - // FIXME: transform the default value + defaulted_params.push(Either::Right(k)); } } } @@ -182,7 +183,7 @@ impl<'a> PathTransform<'a> { target_module, source_scope: self.source_scope, }; - ctx.transform_default_type_substs(default_types); + ctx.transform_default_values(defaulted_params); ctx } } @@ -219,13 +220,19 @@ impl Ctx<'_> { }); } - fn transform_default_type_substs(&self, default_types: Vec) { - for k in default_types { - let v = self.type_substs.get(&k).unwrap(); + fn transform_default_values(&self, defaulted_params: Vec) { + // By now the default values are simply copied from where they are declared + // and should be transformed. As any value is allowed to refer to previous + // generic (both type and const) parameters, they should be all iterated left-to-right. + for param in defaulted_params { + let value = match param { + Either::Left(k) => self.type_substs.get(&k).unwrap().syntax(), + Either::Right(k) => self.const_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::>(); + let paths = postorder(value).filter_map(ast::Path::cast).collect::>(); for path in paths { self.transform_path(path); } From 61cabe029fcd74aae6a2811768bc7f46f22354fe Mon Sep 17 00:00:00 2001 From: ponyii Date: Wed, 12 Jul 2023 18:31:40 +0400 Subject: [PATCH 4/6] the "add missing members" assists: supported bracketed default const values --- crates/hir-ty/src/lib.rs | 13 +++---- .../src/handlers/add_missing_impl_members.rs | 35 +++++++++++++++++++ crates/ide-db/src/path_transform.rs | 1 + 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index 14346c27941..d0bb16c7808 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -52,6 +52,7 @@ use hir_expand::name; use la_arena::{Arena, Idx}; use mir::{MirEvalError, VTableMap}; use rustc_hash::FxHashSet; +use syntax::AstNode; use traits::FnTrait; use triomphe::Arc; use utils::Generics; @@ -723,12 +724,12 @@ where pub fn known_const_to_string(konst: &Const, db: &dyn HirDatabase) -> Option { if let ConstValue::Concrete(c) = &konst.interned().value { - if let ConstScalar::UnevaluatedConst(GeneralConstId::InTypeConstId(_), _) = &c.interned { - // FIXME: stringify the block expression - return None; - } - if c.interned == ConstScalar::Unknown { - return None; + match c.interned { + ConstScalar::UnevaluatedConst(GeneralConstId::InTypeConstId(cid), _) => { + return Some(cid.source(db.upcast()).syntax().to_string()); + } + ConstScalar::Unknown => return None, + _ => (), } } Some(konst.display(db).to_string()) 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 d9faf3a7f6c..c0e5429a22c 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -549,6 +549,41 @@ impl m::Foo for () { ) } + #[test] + fn test_const_substitution_with_defaults_3() { + check_assist( + add_missing_default_members, + r#" +mod m { + pub const VAL: usize = 0; + + pub trait Foo { + fn get_n(&self) -> usize { N } + fn get_m(&self) -> usize { M } + } +} + +impl m::Foo for () { + $0 +}"#, + r#" +mod m { + pub const VAL: usize = 0; + + pub trait Foo { + fn get_n(&self) -> usize { N } + fn get_m(&self) -> usize { M } + } +} + +impl m::Foo for () { + $0fn get_n(&self) -> usize { {40 + 2} } + + fn get_m(&self) -> usize { {m::VAL + 1} } +}"#, + ) + } + #[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 efe13f06040..4bd4f1e845f 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -156,6 +156,7 @@ impl<'a> PathTransform<'a> { // 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 + // (default values in curly brackets can cause the same problem) const_substs.insert(k, expr.syntax().clone()); } } From e4c45427dc398f7fc8f984d2d1b454f36d718fd9 Mon Sep 17 00:00:00 2001 From: ponyii Date: Tue, 8 Aug 2023 21:51:59 +0400 Subject: [PATCH 5/6] refactoring --- crates/hir-ty/src/lib.rs | 8 +++---- crates/hir-ty/src/lower.rs | 22 ++++++++++++-------- crates/hir/src/lib.rs | 6 +++--- crates/ide-db/src/imports/import_assets.rs | 4 ++-- crates/ide-db/src/path_transform.rs | 13 ++++++------ crates/ide-db/src/use_trivial_constructor.rs | 16 +++++++------- 6 files changed, 35 insertions(+), 34 deletions(-) diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index d0bb16c7808..405bb001b5d 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -52,7 +52,7 @@ use hir_expand::name; use la_arena::{Arena, Idx}; use mir::{MirEvalError, VTableMap}; use rustc_hash::FxHashSet; -use syntax::AstNode; +use syntax::ast::{make, ConstArg}; use traits::FnTrait; use triomphe::Arc; use utils::Generics; @@ -722,15 +722,15 @@ where collector.placeholders.into_iter().collect() } -pub fn known_const_to_string(konst: &Const, db: &dyn HirDatabase) -> Option { +pub fn known_const_to_ast(konst: &Const, db: &dyn HirDatabase) -> Option { if let ConstValue::Concrete(c) = &konst.interned().value { match c.interned { ConstScalar::UnevaluatedConst(GeneralConstId::InTypeConstId(cid), _) => { - return Some(cid.source(db.upcast()).syntax().to_string()); + return Some(cid.source(db.upcast())); } ConstScalar::Unknown => return None, _ => (), } } - Some(konst.display(db).to_string()) + Some(make::expr_const_value(konst.display(db).to_string().as_str())) } diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index a885c32e7f9..da5ee6fc428 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -1606,21 +1606,25 @@ pub(crate) fn generic_defaults_query( // Type variable default referring to parameter coming // after it is forbidden (FIXME: report diagnostic) ty = fallback_bound_vars(ty, idx, parent_start_idx); - return crate::make_binders(db, &generic_params, ty.cast(Interner)); + crate::make_binders(db, &generic_params, ty.cast(Interner)) } TypeOrConstParamData::ConstParamData(p) => { - let unknown = unknown_const_as_generic( - db.const_param_ty(ConstParamId::from_unchecked(id)), + let mut val = p.default.as_ref().map_or_else( + || { + unknown_const_as_generic( + db.const_param_ty(ConstParamId::from_unchecked(id)), + ) + }, + |c| { + let c = ctx.lower_const(c, ctx.lower_ty(&p.ty)); + c.cast(Interner) + }, ); - let mut val = p.default.as_ref().map_or(unknown, |c| { - let c = ctx.lower_const(c, ctx.lower_ty(&p.ty)); - chalk_ir::GenericArg::new(Interner, GenericArgData::Const(c)) - }); // Each default can only refer to previous parameters, see above. val = fallback_bound_vars(val, idx, parent_start_idx); - return make_binders(db, &generic_params, val); + make_binders(db, &generic_params, val) } - }; + } }) // FIXME: use `Arc::from_iter` when it becomes available .collect::>(), diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 136b1b08533..1d42e97aa3e 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -63,7 +63,7 @@ use hir_ty::{ all_super_traits, autoderef, consteval::{try_const_usize, unknown_const_as_generic, ConstEvalError, ConstExt}, diagnostics::BodyValidationDiagnostic, - known_const_to_string, + known_const_to_ast, layout::{Layout as TyLayout, RustcEnumVariantIdx, TagEncoding}, method_resolution::{self, TyFingerprint}, mir::{self, interpret_mir}, @@ -3207,9 +3207,9 @@ impl ConstParam { Type::new(db, self.id.parent(), db.const_param_ty(self.id)) } - pub fn default(self, db: &dyn HirDatabase) -> Option { + pub fn default(self, db: &dyn HirDatabase) -> Option { let arg = generic_arg_from_param(db, self.id.into())?; - known_const_to_string(arg.constant(Interner)?, db) + known_const_to_ast(arg.constant(Interner)?, db) } } diff --git a/crates/ide-db/src/imports/import_assets.rs b/crates/ide-db/src/imports/import_assets.rs index e52dc356775..e475c5cd66b 100644 --- a/crates/ide-db/src/imports/import_assets.rs +++ b/crates/ide-db/src/imports/import_assets.rs @@ -6,7 +6,7 @@ use hir::{ use itertools::Itertools; use rustc_hash::FxHashSet; use syntax::{ - ast::{self, HasName}, + ast::{self, make, HasName}, utils::path_to_string_stripping_turbo_fish, AstNode, SyntaxNode, }; @@ -607,7 +607,7 @@ impl ImportCandidate { fn for_name(sema: &Semantics<'_, RootDatabase>, name: &ast::Name) -> Option { if sema .scope(name.syntax())? - .speculative_resolve(&ast::make::ext::ident_path(&name.text())) + .speculative_resolve(&make::ext::ident_path(&name.text())) .is_some() { return None; diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index 4bd4f1e845f..fb75b5b4584 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -5,7 +5,7 @@ use either::Either; use hir::{AsAssocItem, HirDisplay, SemanticsScope}; use rustc_hash::FxHashMap; use syntax::{ - ast::{self, AstNode}, + ast::{self, make, AstNode}, ted, SyntaxNode, }; @@ -139,7 +139,7 @@ impl<'a> PathTransform<'a> { if let Some(default) = &default.display_source_code(db, source_module.into(), false).ok() { - type_substs.insert(k, ast::make::ty(default).clone_for_update()); + type_substs.insert(k, make::ty(default).clone_for_update()); defaulted_params.push(Either::Left(k)); } } @@ -162,7 +162,7 @@ impl<'a> PathTransform<'a> { } (Either::Left(k), None) => { if let Some(default) = k.default(db) { - if let Some(default) = ast::make::expr_const_value(&default).expr() { + if let Some(default) = default.expr() { const_substs.insert(k, default.syntax().clone_for_update()); defaulted_params.push(Either::Right(k)); } @@ -278,15 +278,14 @@ impl Ctx<'_> { hir::ModuleDef::Trait(trait_ref), false, )?; - match ast::make::ty_path(mod_path_to_ast(&found_path)) { + match make::ty_path(mod_path_to_ast(&found_path)) { ast::Type::PathType(path_ty) => Some(path_ty), _ => None, } }); - let segment = ast::make::path_segment_ty(subst.clone(), trait_ref); - let qualified = - ast::make::path_from_segments(std::iter::once(segment), false); + let segment = make::path_segment_ty(subst.clone(), trait_ref); + let qualified = make::path_from_segments(std::iter::once(segment), false); ted::replace(path.syntax(), qualified.clone_for_update().syntax()); } else if let Some(path_ty) = ast::PathType::cast(parent) { ted::replace( diff --git a/crates/ide-db/src/use_trivial_constructor.rs b/crates/ide-db/src/use_trivial_constructor.rs index f96ea29ae2f..a915391ad90 100644 --- a/crates/ide-db/src/use_trivial_constructor.rs +++ b/crates/ide-db/src/use_trivial_constructor.rs @@ -1,31 +1,29 @@ //! Functionality for generating trivial constructors use hir::StructKind; -use syntax::ast; +use syntax::ast::{make, Expr, Path}; /// given a type return the trivial constructor (if one exists) pub fn use_trivial_constructor( db: &crate::RootDatabase, - path: ast::Path, + path: Path, ty: &hir::Type, -) -> Option { +) -> Option { match ty.as_adt() { Some(hir::Adt::Enum(x)) => { if let &[variant] = &*x.variants(db) { if variant.kind(db) == hir::StructKind::Unit { - let path = ast::make::path_qualified( + let path = make::path_qualified( path, - syntax::ast::make::path_segment(ast::make::name_ref( - &variant.name(db).to_smol_str(), - )), + make::path_segment(make::name_ref(&variant.name(db).to_smol_str())), ); - return Some(syntax::ast::make::expr_path(path)); + return Some(make::expr_path(path)); } } } Some(hir::Adt::Struct(x)) if x.kind(db) == StructKind::Unit => { - return Some(syntax::ast::make::expr_path(path)); + return Some(make::expr_path(path)); } _ => {} } From 68e8379ec35f83ce22d3c57bad79e8a7bc7ea231 Mon Sep 17 00:00:00 2001 From: ponyii Date: Tue, 8 Aug 2023 22:16:28 +0400 Subject: [PATCH 6/6] fixed a merge-caused error --- crates/hir-def/src/generics.rs | 2 +- crates/hir-def/src/hir/type_ref.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/hir-def/src/generics.rs b/crates/hir-def/src/generics.rs index 7ee27d26709..1e2535a8a99 100644 --- a/crates/hir-def/src/generics.rs +++ b/crates/hir-def/src/generics.rs @@ -307,7 +307,7 @@ impl GenericParams { let param = ConstParamData { name, ty: Interned::new(ty), - default: ConstRef::from_const_param(lower_ctx, const_param), + default: ConstRef::from_const_param(lower_ctx, &const_param), }; let idx = self.type_or_consts.alloc(param.into()); add_param_attrs(idx.into(), ast::GenericParam::ConstParam(const_param)); diff --git a/crates/hir-def/src/hir/type_ref.rs b/crates/hir-def/src/hir/type_ref.rs index c518f1b75b3..75adf21abdc 100644 --- a/crates/hir-def/src/hir/type_ref.rs +++ b/crates/hir-def/src/hir/type_ref.rs @@ -395,7 +395,7 @@ impl ConstRef { pub(crate) fn from_const_param( lower_ctx: &LowerCtx<'_>, - param: ast::ConstParam, + param: &ast::ConstParam, ) -> Option { let default = param.default_val(); match default {