Auto merge of #14932 - HKalbasi:dev, r=HKalbasi

Lower const params with a bad id

cc #7434

This PR adds an `InTypeConstId` which is a `DefWithBodyId` and lower const generic parameters into bodies using it, and evaluate them with the mir interpreter. I think this is the last unimplemented const generic feature relative to rustc stable.

But there is a problem: The id used in the `InTypeConstId` is the raw `FileAstId`, which changes frequently. So these ids and their bodies will be invalidated very frequently, which is bad for incremental analysis.

Due this problem, I disabled lowering for local crates (in library crate the id is stable since files won't be changed). This might be overreacting (const generic expressions are usually small, maybe it would be better enabled with bad performance than disabled) but it makes motivation for doing it in the correct way, and it splits the potential panic and breakages that usually comes with const generic PRs in two steps.

Other than the id, I think (at least I hope) other parts are in the right direction.

Author: bors

crates/base-db/src/fixture.rs

@@ -215,7 +215,7 @@ impl ChangeFixture {
                 None,
                 default_cfg,
                 Default::default(),
-                Env::default(),
+                Env::new_for_test_fixture(),
                 false,
                 CrateOrigin::Local { repo: None, name: None },
                 default_target_data_layout
@@ -259,7 +259,7 @@ impl ChangeFixture {
                 None,
                 Default::default(),
                 Default::default(),
-                Env::default(),
+                Env::new_for_test_fixture(),
                 false,
                 CrateOrigin::Lang(LangCrateOrigin::Core),
                 target_layout.clone(),
@@ -298,7 +298,7 @@ impl ChangeFixture {
                 None,
                 Default::default(),
                 Default::default(),
-                Env::default(),
+                Env::new_for_test_fixture(),
                 true,
                 CrateOrigin::Local { repo: None, name: None },
                 target_layout,

crates/base-db/src/input.rs

@@ -151,6 +151,12 @@ pub enum CrateOrigin {
     Lang(LangCrateOrigin),
 }
 
+impl CrateOrigin {
+    pub fn is_local(&self) -> bool {
+        matches!(self, CrateOrigin::Local { .. })
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum LangCrateOrigin {
     Alloc,
@@ -333,6 +339,17 @@ pub struct Env {
     entries: FxHashMap<String, String>,
 }
 
+impl Env {
+    pub fn new_for_test_fixture() -> Self {
+        Env {
+            entries: FxHashMap::from_iter([(
+                String::from("__ra_is_test_fixture"),
+                String::from("__ra_is_test_fixture"),
+            )]),
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Dependency {
     pub crate_id: CrateId,
@@ -456,6 +473,10 @@ impl CrateGraph {
         self.arena.iter().map(|(idx, _)| idx)
     }
 
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = (CrateId, &mut CrateData)> + '_ {
+        self.arena.iter_mut()
+    }
+
     /// Returns an iterator over all transitive dependencies of the given crate,
     /// including the crate itself.
     pub fn transitive_deps(&self, of: CrateId) -> impl Iterator<Item = CrateId> {

crates/hir-def/src/body.rs

@@ -118,7 +118,7 @@ impl Body {
         let _p = profile::span("body_with_source_map_query");
         let mut params = None;
 
-        let (file_id, module, body, is_async_fn) = {
+        let (file_id, body, is_async_fn) = {
             match def {
                 DefWithBodyId::FunctionId(f) => {
                     let data = db.function_data(f);
@@ -138,31 +138,29 @@ impl Body {
                             }),
                         )
                     });
-                    (
-                        src.file_id,
-                        f.module(db),
-                        src.value.body().map(ast::Expr::from),
-                        data.has_async_kw(),
-                    )
+                    (src.file_id, src.value.body().map(ast::Expr::from), data.has_async_kw())
                 }
                 DefWithBodyId::ConstId(c) => {
                     let c = c.lookup(db);
                     let src = c.source(db);
-                    (src.file_id, c.module(db), src.value.body(), false)
+                    (src.file_id, src.value.body(), false)
                 }
                 DefWithBodyId::StaticId(s) => {
                     let s = s.lookup(db);
                     let src = s.source(db);
-                    (src.file_id, s.module(db), src.value.body(), false)
+                    (src.file_id, src.value.body(), false)
                 }
                 DefWithBodyId::VariantId(v) => {
-                    let e = v.parent.lookup(db);
                     let src = v.parent.child_source(db);
                     let variant = &src.value[v.local_id];
-                    (src.file_id, e.container, variant.expr(), false)
+                    (src.file_id, variant.expr(), false)
+                }
+                DefWithBodyId::InTypeConstId(c) => {
+                    (c.lookup(db).0.file_id, c.source(db).expr(), false)
                 }
             }
         };
+        let module = def.module(db);
         let expander = Expander::new(db, file_id, module);
         let (mut body, source_map) =
             Body::new(db, def, expander, params, body, module.krate, is_async_fn);

crates/hir-def/src/body/pretty.rs

@@ -40,6 +40,7 @@ pub(super) fn print_body_hir(db: &dyn DefDatabase, body: &Body, owner: DefWithBo
             };
             format!("const {name} = ")
         }
+        DefWithBodyId::InTypeConstId(_) => format!("In type const = "),
         DefWithBodyId::VariantId(it) => {
             let src = it.parent.child_source(db);
             let variant = &src.value[it.local_id];

crates/hir-def/src/db.rs

@@ -1,7 +1,7 @@
 //! Defines database & queries for name resolution.
 use base_db::{salsa, CrateId, SourceDatabase, Upcast};
 use either::Either;
-use hir_expand::{db::ExpandDatabase, HirFileId};
+use hir_expand::{db::ExpandDatabase, AstId, HirFileId};
 use intern::Interned;
 use la_arena::ArenaMap;
 use syntax::{ast, AstPtr};
@@ -22,11 +22,12 @@ use crate::{
     lang_item::{LangItem, LangItemTarget, LangItems},
     nameres::{diagnostics::DefDiagnostic, DefMap},
     visibility::{self, Visibility},
-    AnonymousConstId, AttrDefId, BlockId, BlockLoc, ConstId, ConstLoc, DefWithBodyId, EnumId,
-    EnumLoc, ExternBlockId, ExternBlockLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc,
-    LocalEnumVariantId, LocalFieldId, Macro2Id, Macro2Loc, MacroRulesId, MacroRulesLoc,
-    ProcMacroId, ProcMacroLoc, StaticId, StaticLoc, StructId, StructLoc, TraitAliasId,
-    TraitAliasLoc, TraitId, TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, VariantId,
+    AttrDefId, BlockId, BlockLoc, ConstBlockId, ConstId, ConstLoc, DefWithBodyId, EnumId, EnumLoc,
+    ExternBlockId, ExternBlockLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc,
+    InTypeConstId, LocalEnumVariantId, LocalFieldId, Macro2Id, Macro2Loc, MacroRulesId,
+    MacroRulesLoc, OpaqueInternableThing, ProcMacroId, ProcMacroLoc, StaticId, StaticLoc, StructId,
+    StructLoc, TraitAliasId, TraitAliasLoc, TraitId, TraitLoc, TypeAliasId, TypeAliasLoc,
+    TypeOwnerId, UnionId, UnionLoc, VariantId,
 };
 
 #[salsa::query_group(InternDatabaseStorage)]
@@ -62,7 +63,12 @@ pub trait InternDatabase: SourceDatabase {
     #[salsa::interned]
     fn intern_macro_rules(&self, loc: MacroRulesLoc) -> MacroRulesId;
     #[salsa::interned]
-    fn intern_anonymous_const(&self, id: (DefWithBodyId, ExprId)) -> AnonymousConstId;
+    fn intern_anonymous_const(&self, id: (DefWithBodyId, ExprId)) -> ConstBlockId;
+    #[salsa::interned]
+    fn intern_in_type_const(
+        &self,
+        id: (AstId<ast::ConstArg>, TypeOwnerId, Box<dyn OpaqueInternableThing>),
+    ) -> InTypeConstId;
 }
 
 #[salsa::query_group(DefDatabaseStorage)]

crates/hir-def/src/expander.rs

@@ -155,7 +155,7 @@ impl Expander {
     }
 
     pub(crate) fn parse_path(&mut self, db: &dyn DefDatabase, path: ast::Path) -> Option<Path> {
-        let ctx = LowerCtx::with_hygiene(db, &self.cfg_expander.hygiene);
+        let ctx = LowerCtx::new(db, &self.cfg_expander.hygiene, self.current_file_id);
         Path::from_src(path, &ctx)
     }
 

crates/hir-def/src/hir.rs

@@ -26,7 +26,7 @@ use crate::{
     builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint},
     path::{GenericArgs, Path},
     type_ref::{Mutability, Rawness, TypeRef},
-    AnonymousConstId, BlockId,
+    BlockId, ConstBlockId,
 };
 
 pub use syntax::ast::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, RangeOp, UnaryOp};
@@ -181,7 +181,7 @@ pub enum Expr {
         statements: Box<[Statement]>,
         tail: Option<ExprId>,
     },
-    Const(AnonymousConstId),
+    Const(ConstBlockId),
     Unsafe {
         id: Option<BlockId>,
         statements: Box<[Statement]>,

crates/hir-def/src/hir/type_ref.rs

@@ -118,7 +118,7 @@ pub enum TypeRef {
     Reference(Box<TypeRef>, Option<LifetimeRef>, Mutability),
     // FIXME: for full const generics, the latter element (length) here is going to have to be an
     // expression that is further lowered later in hir_ty.
-    Array(Box<TypeRef>, ConstRefOrPath),
+    Array(Box<TypeRef>, ConstRef),
     Slice(Box<TypeRef>),
     /// A fn pointer. Last element of the vector is the return type.
     Fn(Vec<(Option<Name>, TypeRef)>, bool /*varargs*/, bool /*is_unsafe*/),
@@ -186,11 +186,7 @@ impl TypeRef {
                 TypeRef::RawPtr(Box::new(inner_ty), mutability)
             }
             ast::Type::ArrayType(inner) => {
-                // FIXME: This is a hack. We should probably reuse the machinery of
-                // `hir_def::body::lower` to lower this into an `Expr` and then evaluate it at the
-                // `hir_ty` level, which would allow knowing the type of:
-                // let v: [u8; 2 + 2] = [0u8; 4];
-                let len = ConstRefOrPath::from_expr_opt(inner.expr());
+                let len = ConstRef::from_const_arg(ctx, inner.const_arg());
                 TypeRef::Array(Box::new(TypeRef::from_ast_opt(ctx, inner.ty())), len)
             }
             ast::Type::SliceType(inner) => {
@@ -380,73 +376,84 @@ impl TypeBound {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub enum ConstRefOrPath {
-    Scalar(ConstRef),
+pub enum ConstRef {
+    Scalar(LiteralConstRef),
     Path(Name),
+    Complex(AstId<ast::ConstArg>),
 }
 
-impl ConstRefOrPath {
-    pub(crate) fn from_expr_opt(expr: Option<ast::Expr>) -> Self {
-        match expr {
-            Some(x) => Self::from_expr(x),
-            None => Self::Scalar(ConstRef::Unknown),
+impl ConstRef {
+    pub(crate) fn from_const_arg(lower_ctx: &LowerCtx<'_>, arg: Option<ast::ConstArg>) -> Self {
+        if let Some(arg) = arg {
+            let ast_id = lower_ctx.ast_id(&arg);
+            if let Some(expr) = arg.expr() {
+                return Self::from_expr(expr, ast_id);
+            }
         }
+        Self::Scalar(LiteralConstRef::Unknown)
     }
 
     pub fn display<'a>(&'a self, db: &'a dyn ExpandDatabase) -> impl fmt::Display + 'a {
-        struct Display<'a>(&'a dyn ExpandDatabase, &'a ConstRefOrPath);
+        struct Display<'a>(&'a dyn ExpandDatabase, &'a ConstRef);
         impl fmt::Display for Display<'_> {
             fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                 match self.1 {
-                    ConstRefOrPath::Scalar(s) => s.fmt(f),
-                    ConstRefOrPath::Path(n) => n.display(self.0).fmt(f),
+                    ConstRef::Scalar(s) => s.fmt(f),
+                    ConstRef::Path(n) => n.display(self.0).fmt(f),
+                    ConstRef::Complex(_) => f.write_str("{const}"),
                 }
             }
         }
         Display(db, self)
     }
 
-    // FIXME: as per the comments on `TypeRef::Array`, this evaluation should not happen at this
-    // parse stage.
-    fn from_expr(expr: ast::Expr) -> Self {
+    // We special case literals and single identifiers, to speed up things.
+    fn from_expr(expr: ast::Expr, ast_id: Option<AstId<ast::ConstArg>>) -> Self {
+        fn is_path_ident(p: &ast::PathExpr) -> bool {
+            let Some(path) = p.path() else {
+                return false;
+            };
+            if path.coloncolon_token().is_some() {
+                return false;
+            }
+            if let Some(s) = path.segment() {
+                if s.coloncolon_token().is_some() || s.generic_arg_list().is_some() {
+                    return false;
+                }
+            }
+            true
+        }
         match expr {
-            ast::Expr::PathExpr(p) => {
+            ast::Expr::PathExpr(p) if is_path_ident(&p) => {
                 match p.path().and_then(|x| x.segment()).and_then(|x| x.name_ref()) {
                     Some(x) => Self::Path(x.as_name()),
-                    None => Self::Scalar(ConstRef::Unknown),
+                    None => Self::Scalar(LiteralConstRef::Unknown),
                 }
             }
-            ast::Expr::PrefixExpr(prefix_expr) => match prefix_expr.op_kind() {
-                Some(ast::UnaryOp::Neg) => {
-                    let unsigned = Self::from_expr_opt(prefix_expr.expr());
-                    // Add sign
-                    match unsigned {
-                        Self::Scalar(ConstRef::UInt(num)) => {
-                            Self::Scalar(ConstRef::Int(-(num as i128)))
-                        }
-                        other => other,
-                    }
-                }
-                _ => Self::from_expr_opt(prefix_expr.expr()),
-            },
             ast::Expr::Literal(literal) => Self::Scalar(match literal.kind() {
                 ast::LiteralKind::IntNumber(num) => {
-                    num.value().map(ConstRef::UInt).unwrap_or(ConstRef::Unknown)
+                    num.value().map(LiteralConstRef::UInt).unwrap_or(LiteralConstRef::Unknown)
                 }
                 ast::LiteralKind::Char(c) => {
-                    c.value().map(ConstRef::Char).unwrap_or(ConstRef::Unknown)
+                    c.value().map(LiteralConstRef::Char).unwrap_or(LiteralConstRef::Unknown)
                 }
-                ast::LiteralKind::Bool(f) => ConstRef::Bool(f),
-                _ => ConstRef::Unknown,
+                ast::LiteralKind::Bool(f) => LiteralConstRef::Bool(f),
+                _ => LiteralConstRef::Unknown,
             }),
-            _ => Self::Scalar(ConstRef::Unknown),
+            _ => {
+                if let Some(ast_id) = ast_id {
+                    Self::Complex(ast_id)
+                } else {
+                    Self::Scalar(LiteralConstRef::Unknown)
+                }
+            }
         }
     }
 }
 
-/// A concrete constant value
+/// A literal constant value
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub enum ConstRef {
+pub enum LiteralConstRef {
     Int(i128),
     UInt(u128),
     Bool(bool),
@@ -460,18 +467,20 @@ pub enum ConstRef {
     Unknown,
 }
 
-impl ConstRef {
+impl LiteralConstRef {
     pub fn builtin_type(&self) -> BuiltinType {
         match self {
-            ConstRef::UInt(_) | ConstRef::Unknown => BuiltinType::Uint(BuiltinUint::U128),
-            ConstRef::Int(_) => BuiltinType::Int(BuiltinInt::I128),
-            ConstRef::Char(_) => BuiltinType::Char,
-            ConstRef::Bool(_) => BuiltinType::Bool,
+            LiteralConstRef::UInt(_) | LiteralConstRef::Unknown => {
+                BuiltinType::Uint(BuiltinUint::U128)
+            }
+            LiteralConstRef::Int(_) => BuiltinType::Int(BuiltinInt::I128),
+            LiteralConstRef::Char(_) => BuiltinType::Char,
+            LiteralConstRef::Bool(_) => BuiltinType::Bool,
         }
     }
 }
 
-impl From<Literal> for ConstRef {
+impl From<Literal> for LiteralConstRef {
     fn from(literal: Literal) -> Self {
         match literal {
             Literal::Char(c) => Self::Char(c),
@@ -483,14 +492,14 @@ impl From<Literal> for ConstRef {
     }
 }
 
-impl std::fmt::Display for ConstRef {
+impl std::fmt::Display for LiteralConstRef {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
         match self {
-            ConstRef::Int(num) => num.fmt(f),
-            ConstRef::UInt(num) => num.fmt(f),
-            ConstRef::Bool(flag) => flag.fmt(f),
-            ConstRef::Char(c) => write!(f, "'{c}'"),
-            ConstRef::Unknown => f.write_char('_'),
+            LiteralConstRef::Int(num) => num.fmt(f),
+            LiteralConstRef::UInt(num) => num.fmt(f),
+            LiteralConstRef::Bool(flag) => flag.fmt(f),
+            LiteralConstRef::Char(c) => write!(f, "'{c}'"),
+            LiteralConstRef::Unknown => f.write_char('_'),
         }
     }
 }