the "add missing members" assists: implemented substitution of default values of const params

ponyii · ponyii · commit 52b43927240c · 2023-08-08T21:57:54.000+04:00
diff --git 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<TypeRef>,
-    pub has_default: bool,
+    pub default: Option<ConstRef>,
 }
 
 #[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
@@ -393,6 +393,17 @@ impl ConstRef {
         Self::Scalar(LiteralConstRef::Unknown)
     }
 
+    pub(crate) fn from_default_param_value(
+        _: &LowerCtx<'_>,
+        param: ast::ConstParam,
+    ) -> Option<Self> {
+        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
@@ -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<String> {
+    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
@@ -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::<Vec<_>>(),
diff --git 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<Type> {
-        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<String> {
+        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<GenericArg> {
+    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
@@ -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<const: N: bool> {
+struct Bar<const N: usize> {
     bar: [i32, N]
 }
 
@@ -439,7 +439,7 @@ impl<const X: usize, Y, Z> Foo<X, Z> for S<Y> {
     $0
 }"#,
             r#"
-struct Bar<const: N: bool> {
+struct Bar<const N: usize> {
     bar: [i32, N]
 }
 
@@ -483,6 +483,41 @@ impl<X> Foo<42, {20 + 22}, X> for () {
         )
     }
 
+    #[test]
+    fn test_const_substitution_with_defaults() {
+        check_assist(
+            add_missing_default_members,
+            r#"
+trait Foo<T, const N: usize = 42, const M: bool = false, const P: char = 'a'> {
+    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<X> Foo<X> for () {
+    $0
+}"#,
+            r#"
+trait Foo<T, const N: usize = 42, const M: bool = false, const P: char = 'a'> {
+    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<X> Foo<X> 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
@@ -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
@@ -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<const N: usize = {text}> {{}}"))
+}
+
 pub fn expr_empty_block() -> ast::Expr {
     expr_from_text("{}")
 }

Original file line number	Diff line number	Diff line change
`@@ -503,11 +503,16 @@ pub fn hacky_block_expr(`
`503`	`503`	`pub fn expr_unit() -> ast::Expr {`
`504`	`504`	`expr_from_text("()")`
`505`	`505`	`}`
	`506`	`+`
`506`	`507`	`pub fn expr_literal(text: &str) -> ast::Literal {`
`507`	`508`	`assert_eq!(text.trim(), text);`
`508`	`509`	`ast_from_text(&format!("fn f() {{ let _ = {text}; }}"))`
`509`	`510`	`}`
`510`	`511`
	`512`	`+pub fn expr_const_value(text: &str) -> ast::Expr {`
	`513`	`+ ast_from_text(&format!("trait Foo<const N: usize = {text}> {{}}"))`
	`514`	`+}`
	`515`	`+`
`511`	`516`	`pub fn expr_empty_block() -> ast::Expr {`
`512`	`517`	`expr_from_text("{}")`
`513`	`518`	`}`