Skip to content

Commit

Permalink
Add a bunch of fixme comments
Browse files Browse the repository at this point in the history
  • Loading branch information
HKalbasi committed Jun 11, 2023
1 parent e83b567 commit a469578
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 13 deletions.
6 changes: 3 additions & 3 deletions crates/hir-def/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ 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,
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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -181,7 +181,7 @@ pub enum Expr {
statements: Box<[Statement]>,
tail: Option<ExprId>,
},
Const(AnonymousConstId),
Const(ConstBlockId),
Unsafe {
id: Option<BlockId>,
statements: Box<[Statement]>,
Expand Down
39 changes: 32 additions & 7 deletions crates/hir-def/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,8 @@ impl_from!(
/// 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 {
Expand Down Expand Up @@ -563,7 +563,10 @@ impl From<GenericDefId> for TypeOwnerId {
}
}

/// 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
{
Expand Down Expand Up @@ -594,6 +597,28 @@ impl Clone for Box<dyn OpaqueInternableThing> {
}
}

// FIXME(const-generic-body): Use an stable id for in type consts.
//
// The current id uses `AstId<ast::ConstArg>` 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<ast::ConstArg>, TypeOwnerId, Box<dyn OpaqueInternableThing>);
Expand All @@ -613,17 +638,17 @@ impl InTypeConstId {
#[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<GenericDefId> {
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()
}
Expand All @@ -643,7 +668,7 @@ impl GeneralConstId {
.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:?}}}"),
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/hir-ty/src/consteval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions crates/hir-ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ pub(crate) fn infer_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc<Infer
});
}
DefWithBodyId::InTypeConstId(c) => {
// 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::<InTypeConstIdMetadata>().unwrap().0;
}
Expand Down

0 comments on commit a469578

Please sign in to comment.