From a4695788caa4370136d3df38f9f20d0bb12060fa Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Mon, 12 Jun 2023 00:37:11 +0330 Subject: [PATCH] Add a bunch of fixme comments --- crates/hir-def/src/db.rs | 6 +++--- crates/hir-def/src/hir.rs | 4 ++-- crates/hir-def/src/lib.rs | 39 ++++++++++++++++++++++++++++------ crates/hir-ty/src/consteval.rs | 2 +- crates/hir-ty/src/infer.rs | 1 + 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/crates/hir-def/src/db.rs b/crates/hir-def/src/db.rs index b1a2d2028a8..e6a6eb03497 100644 --- a/crates/hir-def/src/db.rs +++ b/crates/hir-def/src/db.rs @@ -22,8 +22,8 @@ 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, + 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, @@ -63,7 +63,7 @@ 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, diff --git a/crates/hir-def/src/hir.rs b/crates/hir-def/src/hir.rs index 4ad8a7aa8eb..77d879a77ba 100644 --- a/crates/hir-def/src/hir.rs +++ b/crates/hir-def/src/hir.rs @@ -26,7 +26,7 @@ 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, }, - Const(AnonymousConstId), + Const(ConstBlockId), Unsafe { id: Option, statements: Box<[Statement]>, diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 69e4afe94f5..4f68093bcbe 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -482,8 +482,8 @@ pub enum ModuleDefId { /// Id of the anonymous const block expression and patterns. This is very similar to `ClosureId` and /// shouldn't be a `DefWithBodyId` since its type inference is dependent on its parent. #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] -pub struct AnonymousConstId(InternId); -impl_intern_key!(AnonymousConstId); +pub struct ConstBlockId(InternId); +impl_intern_key!(ConstBlockId); #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub enum TypeOwnerId { @@ -563,7 +563,10 @@ fn from(value: GenericDefId) -> Self { } } -/// A thing that we want to store in interned ids, but we don't know its type in `hir-def` +/// A thing that we want to store in interned ids, but we don't know its type in `hir-def`. This is +/// currently only used in `InTypeConstId` for storing the type (which has type `Ty` defined in +/// the `hir-ty` crate) of the constant in its id, which is a temporary hack so we may want +/// to remove this after removing that. pub trait OpaqueInternableThing: std::any::Any + std::fmt::Debug + Sync + Send + UnwindSafe + RefUnwindSafe { @@ -594,6 +597,28 @@ fn clone(&self) -> Self { } } +// FIXME(const-generic-body): Use an stable id for in type consts. +// +// The current id uses `AstId` which will be changed by every change in the code. Ideally +// we should use an id which is relative to the type owner, so that every change will only invalidate the +// id if it happens inside of the type owner. +// +// The solution probably is to have some query on `TypeOwnerId` to traverse its constant children and store +// their `AstId` in a list (vector or arena), and use the index of that list in the id here. That query probably +// needs name resolution, and might go far and handles the whole path lowering or type lowering for a `TypeOwnerId`. +// +// Whatever path the solution takes, it should answer 3 questions at the same time: +// * Is the id stable enough? +// * How to find a constant id using an ast node / position in the source code? This is needed when we want to +// provide ide functionalities inside an in type const (which we currently don't support) e.g. go to definition +// for a local defined there. A complex id might have some trouble in this reverse mapping. +// * How to find the return type of a constant using its id? We have this data when we are doing type lowering +// and the name of the struct that contains this constant is resolved, so a query that only traverses the +// type owner by its syntax tree might have a hard time here. + +/// A constant in a type as a substitution for const generics (like `Foo<{ 2 + 2 }>`) or as an array +/// length (like `[u8; 2 + 2]`). These constants are body owner and are a variant of `DefWithBodyId`. These +/// are not called `AnonymousConstId` to prevent confusion with [`ConstBlockId`]. #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub struct InTypeConstId(InternId); type InTypeConstLoc = (AstId, TypeOwnerId, Box); @@ -613,17 +638,17 @@ pub fn source(&self, db: &dyn db::DefDatabase) -> ast::ConstArg { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum GeneralConstId { ConstId(ConstId), - AnonymousConstId(AnonymousConstId), + ConstBlockId(ConstBlockId), InTypeConstId(InTypeConstId), } -impl_from!(ConstId, AnonymousConstId, InTypeConstId for GeneralConstId); +impl_from!(ConstId, ConstBlockId, InTypeConstId for GeneralConstId); impl GeneralConstId { pub fn generic_def(self, db: &dyn db::DefDatabase) -> Option { match self { GeneralConstId::ConstId(x) => Some(x.into()), - GeneralConstId::AnonymousConstId(x) => { + GeneralConstId::ConstBlockId(x) => { let (parent, _) = db.lookup_intern_anonymous_const(x); parent.as_generic_def_id() } @@ -643,7 +668,7 @@ pub fn name(self, db: &dyn db::DefDatabase) -> String { .and_then(|x| x.as_str()) .unwrap_or("_") .to_owned(), - GeneralConstId::AnonymousConstId(id) => format!("{{anonymous const {id:?}}}"), + GeneralConstId::ConstBlockId(id) => format!("{{anonymous const {id:?}}}"), GeneralConstId::InTypeConstId(id) => format!("{{in type const {id:?}}}"), } } diff --git a/crates/hir-ty/src/consteval.rs b/crates/hir-ty/src/consteval.rs index 0b762d96ae7..7a7c12e5b44 100644 --- a/crates/hir-ty/src/consteval.rs +++ b/crates/hir-ty/src/consteval.rs @@ -215,7 +215,7 @@ pub(crate) fn const_eval_query( GeneralConstId::ConstId(c) => { db.monomorphized_mir_body(c.into(), subst, db.trait_environment(c.into()))? } - GeneralConstId::AnonymousConstId(c) => { + GeneralConstId::ConstBlockId(c) => { let (def, root) = db.lookup_intern_anonymous_const(c); let body = db.body(def); let infer = db.infer(def); diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 6399fa052bc..2506ae5bb6f 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -108,6 +108,7 @@ pub(crate) fn infer_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc { + // FIXME(const-generic-body): We should not get the return type in this way. ctx.return_ty = c.lookup(db.upcast()).2.box_any().downcast::().unwrap().0; }