diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index 14b1cedf5f3..9eb60ebede7 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -763,11 +763,11 @@ fn transform_event( HirExpression::Literal(HirLiteral::Str(signature)) if signature == SIGNATURE_PLACEHOLDER => { - let selector_literal_id = first_arg_id; + let selector_literal_id = *first_arg_id; let structure = interner.get_struct(struct_id); let signature = event_signature(&structure.borrow()); - interner.update_expression(*selector_literal_id, |expr| { + interner.update_expression(selector_literal_id, |expr| { *expr = HirExpression::Literal(HirLiteral::Str(signature.clone())); }); @@ -833,7 +833,7 @@ fn get_serialized_length( let serialized_trait_impl_kind = traits .iter() - .filter_map(|&trait_id| { + .find_map(|&trait_id| { let r#trait = interner.get_trait(trait_id); if r#trait.borrow().name.0.contents == "Serialize" && r#trait.borrow().generics.len() == 1 @@ -846,7 +846,6 @@ fn get_serialized_length( None } }) - .next() .ok_or(AztecMacroError::CouldNotAssignStorageSlots { secondary_message: Some("Stored data must implement Serialize trait".to_string()), })?; diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 5a22214095b..8e0dacc294b 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -31,7 +31,7 @@ pub struct LocalModuleId(pub Index); impl LocalModuleId { pub fn dummy_id() -> LocalModuleId { - LocalModuleId(Index(std::usize::MAX)) + LocalModuleId(Index::dummy()) } } diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index d4aae133b35..f05a69be7c2 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1463,7 +1463,7 @@ impl<'a> Resolver<'a> { // they're used in expressions. We must do this here since the type // checker does not check definition kinds and otherwise expects // parameters to already be typed. - if self.interner.id_type(hir_ident.id) == Type::Error { + if self.interner.definition_type(hir_ident.id) == Type::Error { let typ = Type::polymorphic_integer(self.interner); self.interner.push_definition_type(hir_ident.id, typ); } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 96a79152f69..26084931bd2 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -284,8 +284,9 @@ impl<'interner> TypeChecker<'interner> { Type::Tuple(vecmap(&elements, |elem| self.check_expression(elem))) } HirExpression::Lambda(lambda) => { - let captured_vars = - vecmap(lambda.captures, |capture| self.interner.id_type(capture.ident.id)); + let captured_vars = vecmap(lambda.captures, |capture| { + self.interner.definition_type(capture.ident.id) + }); let env_type: Type = if captured_vars.is_empty() { Type::Unit } else { Type::Tuple(captured_vars) }; @@ -308,7 +309,7 @@ impl<'interner> TypeChecker<'interner> { } }; - self.interner.push_expr_type(expr_id, typ.clone()); + self.interner.push_expr_type(*expr_id, typ.clone()); typ } @@ -459,7 +460,7 @@ impl<'interner> TypeChecker<'interner> { operator: UnaryOp::MutableReference, rhs: method_call.object, })); - self.interner.push_expr_type(&new_object, new_type); + self.interner.push_expr_type(new_object, new_type); self.interner.push_expr_location(new_object, location.span, location.file); new_object }); @@ -485,7 +486,7 @@ impl<'interner> TypeChecker<'interner> { operator: UnaryOp::Dereference { implicitly_added: true }, rhs: object, })); - self.interner.push_expr_type(&object, element.as_ref().clone()); + self.interner.push_expr_type(object, element.as_ref().clone()); self.interner.push_expr_location(object, location.span, location.file); // Recursively dereference to allow for converting &mut &mut T to T @@ -682,8 +683,8 @@ impl<'interner> TypeChecker<'interner> { operator: crate::UnaryOp::Dereference { implicitly_added: true }, rhs: old_lhs, })); - this.interner.push_expr_type(&old_lhs, lhs_type); - this.interner.push_expr_type(access_lhs, element); + this.interner.push_expr_type(old_lhs, lhs_type); + this.interner.push_expr_type(*access_lhs, element); let old_location = this.interner.id_location(old_lhs); this.interner.push_expr_location(*access_lhs, span, old_location.file); diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 4dcddbcca02..225f5756d7a 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -458,7 +458,7 @@ mod test { } fn local_module_id(&self) -> LocalModuleId { - LocalModuleId(arena::Index(0, generational_arena::Index::from_raw_parts(0, 0))) + LocalModuleId(arena::Index::unsafe_zeroed()) } fn module_id(&self) -> ModuleId { diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 03d61b93e0c..370b4ee7b17 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -192,7 +192,7 @@ impl<'interner> TypeChecker<'interner> { mutable = definition.mutable; } - let typ = self.interner.id_type(ident.id).instantiate(self.interner).0; + let typ = self.interner.definition_type(ident.id).instantiate(self.interner).0; typ.follow_bindings() }; diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 98b47f17cd4..d4d8a948460 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1672,11 +1672,10 @@ fn convert_array_expression_to_slice( interner.push_expr_location(call, location.span, location.file); interner.push_expr_location(func, location.span, location.file); - interner.push_expr_type(&call, target_type.clone()); - interner.push_expr_type( - &func, - Type::Function(vec![array_type], Box::new(target_type), Box::new(Type::Unit)), - ); + interner.push_expr_type(call, target_type.clone()); + + let func_type = Type::Function(vec![array_type], Box::new(target_type), Box::new(Type::Unit)); + interner.push_expr_type(func, func_type); } impl BinaryTypeOperator { diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index d36816e3d37..5837d67660a 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -143,7 +143,7 @@ impl<'interner> Monomorphizer<'interner> { let index_id = self.interner.push_expr(HirExpression::Literal( HirLiteral::Integer(field_index.into(), false), )); - self.interner.push_expr_type(&index_id, crate::Type::FieldElement); + self.interner.push_expr_type(index_id, crate::Type::FieldElement); self.interner.push_expr_location( index_id, call.location.span, @@ -171,7 +171,7 @@ impl<'interner> Monomorphizer<'interner> { fn intern_var_id(&mut self, var_id: DebugVarId, location: &Location) -> ExprId { let var_id_literal = HirLiteral::Integer((var_id.0 as u128).into(), false); let expr_id = self.interner.push_expr(HirExpression::Literal(var_id_literal)); - self.interner.push_expr_type(&expr_id, crate::Type::FieldElement); + self.interner.push_expr_type(expr_id, crate::Type::FieldElement); self.interner.push_expr_location(expr_id, location.span, location.file); expr_id } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index f691a0c9065..3f255bf6d0a 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -696,7 +696,7 @@ impl<'interner> Monomorphizer<'interner> { let mutable = definition.mutable; let definition = self.lookup_local(ident.id)?; - let typ = self.convert_type(&self.interner.id_type(ident.id)); + let typ = self.convert_type(&self.interner.definition_type(ident.id)); Some(ast::Ident { location: Some(ident.location), mutable, definition, name, typ }) } @@ -1038,7 +1038,7 @@ impl<'interner> Monomorphizer<'interner> { ) { match hir_argument { HirExpression::Ident(ident) => { - let typ = self.interner.id_type(ident.id); + let typ = self.interner.definition_type(ident.id); let typ: Type = typ.follow_bindings(); let is_fmt_str = match typ { // A format string has many different possible types that need to be handled. diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 42584ce0265..f0a006489f4 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -75,13 +75,14 @@ pub struct NodeInterner { // Type checking map // - // Notice that we use `Index` as the Key and not an ExprId or IdentId - // Therefore, If a raw index is passed in, then it is not safe to assume that it will have - // a Type, as not all Ids have types associated to them. - // Further note, that an ExprId and an IdentId will never have the same underlying Index - // Because we use one Arena to store all Definitions/Nodes + // This should only be used with indices from the `nodes` arena. + // Otherwise the indices used may overwrite other existing indices. + // Each type for each index is filled in during type checking. id_to_type: HashMap, + // Similar to `id_to_type` but maps definitions to their type + definition_to_type: HashMap, + // Struct map. // // Each struct definition is possibly shared across multiple type nodes. @@ -277,12 +278,6 @@ impl DefinitionId { } } -impl From for Index { - fn from(id: DefinitionId) -> Self { - Index(id.0) - } -} - /// An ID for a global value #[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] pub struct GlobalId(usize); @@ -302,7 +297,7 @@ impl StmtId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> StmtId { - StmtId(Index(std::usize::MAX)) + StmtId(Index::dummy()) } } @@ -311,7 +306,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index(0)) + ExprId(Index::unsafe_zeroed()) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -322,7 +317,7 @@ impl FuncId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> FuncId { - FuncId(Index(std::usize::MAX)) + FuncId(Index::dummy()) } } @@ -396,23 +391,9 @@ macro_rules! into_index { }; } -macro_rules! partialeq { - ($id_type:ty) => { - impl PartialEq for &$id_type { - fn eq(&self, other: &usize) -> bool { - let index = self.0 .0; - index == *other - } - } - }; -} - into_index!(ExprId); into_index!(StmtId); -partialeq!(ExprId); -partialeq!(StmtId); - /// A Definition enum specifies anything that we can intern in the NodeInterner /// We use one Arena for all types that can be interned as that has better cache locality /// This data structure is never accessed directly, so API wise there is no difference between using @@ -496,6 +477,7 @@ impl Default for NodeInterner { id_to_location: HashMap::new(), definitions: vec![], id_to_type: HashMap::new(), + definition_to_type: HashMap::new(), structs: HashMap::new(), struct_attributes: HashMap::new(), type_aliases: Vec::new(), @@ -545,10 +527,15 @@ impl NodeInterner { } /// Store the type for an interned expression - pub fn push_expr_type(&mut self, expr_id: &ExprId, typ: Type) { + pub fn push_expr_type(&mut self, expr_id: ExprId, typ: Type) { self.id_to_type.insert(expr_id.into(), typ); } + /// Store the type for an interned expression + pub fn push_definition_type(&mut self, definition_id: DefinitionId, typ: Type) { + self.definition_to_type.insert(definition_id, typ); + } + pub fn push_empty_trait(&mut self, type_id: TraitId, unresolved_trait: &UnresolvedTrait) { let self_type_typevar_id = self.next_type_variable_id(); @@ -660,11 +647,6 @@ impl NodeInterner { } } - /// Store the type for an interned Identifier - pub fn push_definition_type(&mut self, definition_id: DefinitionId, typ: Type) { - self.id_to_type.insert(definition_id.into(), typ); - } - /// Store [Location] of [Type] reference pub fn push_type_ref_location(&mut self, typ: Type, location: Location) { self.type_ref_locations.push((typ, location)); @@ -980,8 +962,13 @@ impl NodeInterner { self.id_to_type.get(&index.into()).cloned().unwrap_or(Type::Error) } + /// Returns the type of the definition or `Type::Error` if it was not found. + pub fn definition_type(&self, id: DefinitionId) -> Type { + self.definition_to_type.get(&id).cloned().unwrap_or(Type::Error) + } + pub fn id_type_substitute_trait_as_type(&self, def_id: DefinitionId) -> Type { - let typ = self.id_type(def_id); + let typ = self.definition_type(def_id); if let Type::Function(args, ret, env) = &typ { let def = self.definition(def_id); if let Type::TraitAsType(..) = ret.as_ref() { diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index cf834ed7d59..ec9b1d9a837 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -4,7 +4,26 @@ #![warn(clippy::semicolon_if_nothing_returned)] #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)] -pub struct Index(pub usize); +pub struct Index(usize); + +impl Index { + #[cfg(test)] + pub fn test_new(index: usize) -> Index { + Self(index) + } + + /// Return a dummy index (max value internally). + /// This should be avoided over `Option` if possible. + pub fn dummy() -> Self { + Self(usize::MAX) + } + + /// Return the zeroed index. This is unsafe since we don't know + /// if this is a valid index for any particular map yet. + pub fn unsafe_zeroed() -> Self { + Self(0) + } +} // #[derive(Debug, Eq, PartialEq, Hash, Clone)] #[derive(Debug, Clone)]