From f27b439786a4650bf8d1732187cccb8b7cd9d102 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 30 Jan 2024 11:04:40 -0500 Subject: [PATCH 01/16] wip: remove dependency on generational-arena, add minimal vec-based arena class --- Cargo.lock | 12 ---- .../noirc_frontend/src/hir/def_map/mod.rs | 2 +- .../noirc_frontend/src/hir/type_check/mod.rs | 4 +- compiler/noirc_frontend/src/node_interner.rs | 10 ++-- compiler/utils/arena/Cargo.toml | 5 -- compiler/utils/arena/src/lib.rs | 55 ++++++++++++++++++- 6 files changed, 60 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 93f1d25fc76..1dd62525954 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -213,9 +213,6 @@ checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" [[package]] name = "arena" version = "0.23.0" -dependencies = [ - "generational-arena", -] [[package]] name = "ark-bls12-381" @@ -1818,15 +1815,6 @@ dependencies = [ "byteorder", ] -[[package]] -name = "generational-arena" -version = "0.2.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "877e94aff08e743b651baaea359664321055749b398adff8740a7399af7796e7" -dependencies = [ - "cfg-if 1.0.0", -] - [[package]] name = "generic-array" version = "0.14.7" diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 8c985e88e0b..5fcc3101043 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::from_raw_parts(std::usize::MAX, std::u64::MAX)) + LocalModuleId(Index { ix: std::usize::MAX }) } } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 3c2a970ee84..392f9222bd1 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -454,7 +454,7 @@ mod test { } fn local_module_id(&self) -> LocalModuleId { - LocalModuleId(arena::Index::from_raw_parts(0, 0)) + LocalModuleId(arena::Index { ix: 0 }) } fn module_id(&self) -> ModuleId { @@ -505,7 +505,7 @@ mod test { let mut def_maps = BTreeMap::new(); let file = FileId::default(); - let mut modules = arena::Arena::new(); + let mut modules = arena::Arena::default(); let location = Location::new(Default::default(), file); modules.insert(ModuleData::new(None, location, false)); diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index b856b54f6ca..d2e6134fd29 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -242,7 +242,7 @@ impl DefinitionId { impl From for Index { fn from(id: DefinitionId) -> Self { - Index::from_raw_parts(id.0, u64::MAX) + Index { ix: id.0 } } } @@ -254,7 +254,7 @@ impl StmtId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> StmtId { - StmtId(Index::from_raw_parts(std::usize::MAX, 0)) + StmtId(Index { ix: std::usize::MAX }) } } @@ -263,7 +263,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index::from_raw_parts(0, 0)) + ExprId(Index { ix: 0 }) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -274,7 +274,7 @@ impl FuncId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> FuncId { - FuncId(Index::from_raw_parts(std::usize::MAX, 0)) + FuncId(Index { ix: std::usize::MAX }) } } @@ -352,7 +352,7 @@ macro_rules! partialeq { ($id_type:ty) => { impl PartialEq for &$id_type { fn eq(&self, other: &usize) -> bool { - let (index, _) = self.0.into_raw_parts(); + let index = self.0.ix; index == *other } } diff --git a/compiler/utils/arena/Cargo.toml b/compiler/utils/arena/Cargo.toml index e82201a2cf4..41c6ebc9a8b 100644 --- a/compiler/utils/arena/Cargo.toml +++ b/compiler/utils/arena/Cargo.toml @@ -4,8 +4,3 @@ version.workspace = true authors.workspace = true edition.workspace = true license.workspace = true - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] -generational-arena = "0.2.8" diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index fc19f44ab6e..f15191639a9 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -1,7 +1,56 @@ #![forbid(unsafe_code)] -#![warn(unused_crate_dependencies, unused_extern_crates)] #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] -// For now we use a wrapper around generational-arena -pub use generational_arena::{Arena, Index}; +#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)] +pub struct Index { + pub ix: usize, +} + +#[derive(Debug, Eq, PartialEq, Hash, Clone)] +pub struct Arena { + vec: Vec +} + +impl core::ops::Index for Arena { + type Output = T; + + fn index(&self, index: Index) -> &Self::Output { + self.vec.index(index.ix) + } +} + +impl core::ops::IndexMut for Arena { + fn index_mut(&mut self, index: Index) -> &mut Self::Output { + self.vec.index_mut(index.ix) + } +} + +impl Arena { + pub fn default() -> Self { + Self { + vec: Vec::new() + } + } + + pub fn insert(&mut self, item: T) -> Index { + let ix = self.vec.len(); + self.vec.push(item); + Index { ix } + } + + pub fn get(&self, index: Index) -> Option<&T> { + self.vec.get(index.ix) + } + + pub fn get_mut(&mut self, index: Index) -> Option<&mut T> { + self.vec.get_mut(index.ix) + } + + pub fn iter(&self) -> impl Iterator { + self.vec.iter().enumerate().map(|(ix, item)| { + (Index { ix: ix }, item) + }) + } +} + From 7f95c26908d3beb533e55332413a5ac7e77b7392 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 30 Jan 2024 11:06:38 -0500 Subject: [PATCH 02/16] cargo fmt --- compiler/utils/arena/src/lib.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index f15191639a9..1336ce75384 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -9,7 +9,7 @@ pub struct Index { #[derive(Debug, Eq, PartialEq, Hash, Clone)] pub struct Arena { - vec: Vec + vec: Vec, } impl core::ops::Index for Arena { @@ -28,9 +28,7 @@ impl core::ops::IndexMut for Arena { impl Arena { pub fn default() -> Self { - Self { - vec: Vec::new() - } + Self { vec: Vec::new() } } pub fn insert(&mut self, item: T) -> Index { @@ -48,9 +46,6 @@ impl Arena { } pub fn iter(&self) -> impl Iterator { - self.vec.iter().enumerate().map(|(ix, item)| { - (Index { ix: ix }, item) - }) + self.vec.iter().enumerate().map(|(ix, item)| (Index { ix: ix }, item)) } } - From da7e21999caf5648d568f833b874e05603ff6c6e Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 30 Jan 2024 11:21:04 -0500 Subject: [PATCH 03/16] cargo clippy --- compiler/utils/arena/src/lib.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 1336ce75384..910a7723fbe 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -12,6 +12,12 @@ pub struct Arena { vec: Vec, } +impl Default for Arena { + fn default() -> Self { + Self { vec: Vec::new() } + } +} + impl core::ops::Index for Arena { type Output = T; @@ -27,10 +33,6 @@ impl core::ops::IndexMut for Arena { } impl Arena { - pub fn default() -> Self { - Self { vec: Vec::new() } - } - pub fn insert(&mut self, item: T) -> Index { let ix = self.vec.len(); self.vec.push(item); @@ -46,6 +48,6 @@ impl Arena { } pub fn iter(&self) -> impl Iterator { - self.vec.iter().enumerate().map(|(ix, item)| (Index { ix: ix }, item)) + self.vec.iter().enumerate().map(|(ix, item)| (Index { ix }, item)) } } From f83698162e3c9284512fd098cd63a0c50ddf2e7b Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 30 Jan 2024 12:54:51 -0500 Subject: [PATCH 04/16] revert warnings for unused crates --- compiler/utils/arena/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 910a7723fbe..615d9b59b14 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -1,4 +1,5 @@ #![forbid(unsafe_code)] +#![warn(unused_crate_dependencies, unused_extern_crates)] #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] From 9203fe5bf4186c4534b689a0ffc8af5809e65719 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 30 Jan 2024 16:22:30 -0500 Subject: [PATCH 05/16] rename ix -> index --- compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 10 +++++----- compiler/utils/arena/src/lib.rs | 16 ++++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 5fcc3101043..3a4fca41d79 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 { ix: std::usize::MAX }) + LocalModuleId(Index { index: std::usize::MAX }) } } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index d2e6134fd29..f990c330ed4 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -242,7 +242,7 @@ impl DefinitionId { impl From for Index { fn from(id: DefinitionId) -> Self { - Index { ix: id.0 } + Index { index: id.0 } } } @@ -254,7 +254,7 @@ impl StmtId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> StmtId { - StmtId(Index { ix: std::usize::MAX }) + StmtId(Index { index: std::usize::MAX }) } } @@ -263,7 +263,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index { ix: 0 }) + ExprId(Index { index: 0 }) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -274,7 +274,7 @@ impl FuncId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> FuncId { - FuncId(Index { ix: std::usize::MAX }) + FuncId(Index { index: std::usize::MAX }) } } @@ -352,7 +352,7 @@ macro_rules! partialeq { ($id_type:ty) => { impl PartialEq for &$id_type { fn eq(&self, other: &usize) -> bool { - let index = self.0.ix; + let index = self.0.index; index == *other } } diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 615d9b59b14..bfea3f11431 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -5,7 +5,7 @@ #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)] pub struct Index { - pub ix: usize, + pub index: usize, } #[derive(Debug, Eq, PartialEq, Hash, Clone)] @@ -23,32 +23,32 @@ impl core::ops::Index for Arena { type Output = T; fn index(&self, index: Index) -> &Self::Output { - self.vec.index(index.ix) + self.vec.index(index.index) } } impl core::ops::IndexMut for Arena { fn index_mut(&mut self, index: Index) -> &mut Self::Output { - self.vec.index_mut(index.ix) + self.vec.index_mut(index.index) } } impl Arena { pub fn insert(&mut self, item: T) -> Index { - let ix = self.vec.len(); + let index = self.vec.len(); self.vec.push(item); - Index { ix } + Index { index } } pub fn get(&self, index: Index) -> Option<&T> { - self.vec.get(index.ix) + self.vec.get(index.index) } pub fn get_mut(&mut self, index: Index) -> Option<&mut T> { - self.vec.get_mut(index.ix) + self.vec.get_mut(index.index) } pub fn iter(&self) -> impl Iterator { - self.vec.iter().enumerate().map(|(ix, item)| (Index { ix }, item)) + self.vec.iter().enumerate().map(|(index, item)| (Index { index }, item)) } } From 00531d218756e9f98748b904e373fa8b151f1d84 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 31 Jan 2024 13:46:56 -0500 Subject: [PATCH 06/16] make Index a tuple struct --- compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 10 +++++----- compiler/utils/arena/src/lib.rs | 16 +++++++--------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 3a4fca41d79..5a22214095b 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 { index: std::usize::MAX }) + LocalModuleId(Index(std::usize::MAX)) } } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index f990c330ed4..013cc04c2cd 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -242,7 +242,7 @@ impl DefinitionId { impl From for Index { fn from(id: DefinitionId) -> Self { - Index { index: id.0 } + Index(id.0) } } @@ -254,7 +254,7 @@ impl StmtId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> StmtId { - StmtId(Index { index: std::usize::MAX }) + StmtId(Index(std::usize::MAX)) } } @@ -263,7 +263,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index { index: 0 }) + ExprId(Index(0)) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -274,7 +274,7 @@ impl FuncId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> FuncId { - FuncId(Index { index: std::usize::MAX }) + FuncId(Index(std::usize::MAX)) } } @@ -352,7 +352,7 @@ macro_rules! partialeq { ($id_type:ty) => { impl PartialEq for &$id_type { fn eq(&self, other: &usize) -> bool { - let index = self.0.index; + let index = self.0 .0; index == *other } } diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index bfea3f11431..a999e98c1c6 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -4,9 +4,7 @@ #![warn(clippy::semicolon_if_nothing_returned)] #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)] -pub struct Index { - pub index: usize, -} +pub struct Index(pub usize); #[derive(Debug, Eq, PartialEq, Hash, Clone)] pub struct Arena { @@ -23,13 +21,13 @@ impl core::ops::Index for Arena { type Output = T; fn index(&self, index: Index) -> &Self::Output { - self.vec.index(index.index) + self.vec.index(index.0) } } impl core::ops::IndexMut for Arena { fn index_mut(&mut self, index: Index) -> &mut Self::Output { - self.vec.index_mut(index.index) + self.vec.index_mut(index.0) } } @@ -37,18 +35,18 @@ impl Arena { pub fn insert(&mut self, item: T) -> Index { let index = self.vec.len(); self.vec.push(item); - Index { index } + Index(index) } pub fn get(&self, index: Index) -> Option<&T> { - self.vec.get(index.index) + self.vec.get(index.0) } pub fn get_mut(&mut self, index: Index) -> Option<&mut T> { - self.vec.get_mut(index.index) + self.vec.get_mut(index.0) } pub fn iter(&self) -> impl Iterator { - self.vec.iter().enumerate().map(|(index, item)| (Index { index }, item)) + self.vec.iter().enumerate().map(|(index, item)| (Index(index), item)) } } From b8098c8f4bdb471d248a4b8f96b6ec1fc651ffa6 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 1 Feb 2024 12:29:31 +0000 Subject: [PATCH 07/16] Update compiler/noirc_frontend/src/hir/type_check/mod.rs --- compiler/noirc_frontend/src/hir/type_check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 392f9222bd1..d84ae304af4 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -454,7 +454,7 @@ mod test { } fn local_module_id(&self) -> LocalModuleId { - LocalModuleId(arena::Index { ix: 0 }) + LocalModuleId(arena::Index(0)) } fn module_id(&self) -> ModuleId { From 151a23c15083618ce2c9d23380d825d12f97cde3 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 5 Feb 2024 17:30:40 -0500 Subject: [PATCH 08/16] added back generational_arena, added copy of generational_arena::{Arena, Index} to {Arena, Index}, added tests for parity between arenas to class, added missing instances, found inconsistency, wip debugging --- Cargo.lock | 13 ++ compiler/noirc_frontend/Cargo.toml | 1 + .../src/hir/def_collector/dc_mod.rs | 6 + .../src/hir/def_map/item_scope.rs | 2 +- .../noirc_frontend/src/hir/def_map/mod.rs | 2 +- .../src/hir/def_map/module_data.rs | 2 +- compiler/noirc_frontend/src/hir_def/expr.rs | 30 ++--- .../noirc_frontend/src/hir_def/function.rs | 2 +- compiler/noirc_frontend/src/hir_def/stmt.rs | 12 +- compiler/noirc_frontend/src/node_interner.rs | 10 +- compiler/utils/arena/Cargo.toml | 3 + compiler/utils/arena/src/lib.rs | 119 +++++++++++++++--- 12 files changed, 155 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e7f757776be..09cf969a686 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -213,6 +213,9 @@ checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" [[package]] name = "arena" version = "0.23.0" +dependencies = [ + "generational-arena", +] [[package]] name = "ark-bls12-381" @@ -1815,6 +1818,15 @@ dependencies = [ "byteorder", ] +[[package]] +name = "generational-arena" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "877e94aff08e743b651baaea359664321055749b398adff8740a7399af7796e7" +dependencies = [ + "cfg-if 1.0.0", +] + [[package]] name = "generic-array" version = "0.14.7" @@ -2969,6 +2981,7 @@ dependencies = [ "arena", "chumsky", "fm", + "generational-arena", "iter-extended", "noirc_errors", "noirc_printable_type", diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index 80d767f7f2c..e63305885d7 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -23,6 +23,7 @@ rustc-hash = "1.1.0" small-ord-set = "0.1.3" regex = "1.9.1" tracing.workspace = true +generational-arena = "0.2.8" [dev-dependencies] strum = "0.24" diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 3cd60c33b8b..f6d861c3484 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -235,6 +235,9 @@ impl<'a> ModCollector<'a> { unresolved_functions.push_fn(self.module_id, func_id, function); // Add function to scope/ns of the module + // TODO: remove before merge + let _ = self.def_collector.def_map.modules._arena[self.module_id.0.1] + .declare_function(name.clone(), func_id); let result = self.def_collector.def_map.modules[self.module_id.0] .declare_function(name, func_id); @@ -408,6 +411,9 @@ impl<'a> ModCollector<'a> { .def_interner .push_function_definition(func_id, modifiers, trait_id.0, location); + // TODO: remove before merge + let _ = self.def_collector.def_map.modules._arena[trait_id.0.local_id.0.1] + .declare_function(name.clone(), func_id); match self.def_collector.def_map.modules[trait_id.0.local_id.0] .declare_function(name.clone(), func_id) { diff --git a/compiler/noirc_frontend/src/hir/def_map/item_scope.rs b/compiler/noirc_frontend/src/hir/def_map/item_scope.rs index 523def89518..50f1f50cbe5 100644 --- a/compiler/noirc_frontend/src/hir/def_map/item_scope.rs +++ b/compiler/noirc_frontend/src/hir/def_map/item_scope.rs @@ -12,7 +12,7 @@ pub enum Visibility { Public, } -#[derive(Default, Debug, PartialEq, Eq)] +#[derive(Clone, Default, Debug, PartialEq, Eq)] pub struct ItemScope { types: HashMap, values: HashMap, diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 5a22214095b..65835eaa086 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(std::usize::MAX, generational_arena::Index::from_raw_parts(std::usize::MAX, 0))) } } diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index fbb5e5cf741..0f752d530bb 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -11,7 +11,7 @@ use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs}; /// Contains the actual contents of a module: its parent (if one exists), /// children, and scope with all definitions defined within the scope. -#[derive(Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ModuleData { pub parent: Option, pub children: HashMap, diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 75ed68af0f6..70f53a24532 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -14,7 +14,7 @@ use super::types::{StructType, Type}; /// references other HIR nodes indirectly via IDs rather than directly via /// boxing. Variables in HirExpressions are tagged with their DefinitionId /// from the definition that refers to them so there is no ambiguity with names. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum HirExpression { Ident(HirIdent), Literal(HirLiteral), @@ -109,7 +109,7 @@ impl HirBinaryOp { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum HirLiteral { Array(HirArrayLiteral), Bool(bool), @@ -119,19 +119,19 @@ pub enum HirLiteral { Unit, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum HirArrayLiteral { Standard(Vec), Repeated { repeated_element: ExprId, length: Type }, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirPrefixExpression { pub operator: UnaryOp, pub rhs: ExprId, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirInfixExpression { pub lhs: ExprId, pub operator: HirBinaryOp, @@ -146,7 +146,7 @@ pub struct HirInfixExpression { /// This is always a struct field access `my_struct.field` /// and never a method call. The later is represented by HirMethodCallExpression. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirMemberAccess { pub lhs: ExprId, // This field is not an IdentId since the rhs of a field @@ -160,7 +160,7 @@ pub struct HirMemberAccess { pub is_offset: bool, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirIfExpression { pub condition: ExprId, pub consequence: ExprId, @@ -168,13 +168,13 @@ pub struct HirIfExpression { } // `lhs as type` in the source code -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirCastExpression { pub lhs: ExprId, pub r#type: Type, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirCallExpression { pub func: ExprId, pub arguments: Vec, @@ -185,7 +185,7 @@ pub struct HirCallExpression { /// lowered into HirCallExpression nodes /// after type checking resolves the object /// type and the method it calls. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirMethodCallExpression { pub method: Ident, pub object: ExprId, @@ -239,7 +239,7 @@ impl HirMethodCallExpression { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirConstructorExpression { pub r#type: Shared, pub struct_generics: Vec, @@ -253,13 +253,13 @@ pub struct HirConstructorExpression { } /// Indexing, as in `array[index]` -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirIndexExpression { pub collection: ExprId, pub index: ExprId, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirBlockExpression(pub Vec); impl HirBlockExpression { @@ -269,7 +269,7 @@ impl HirBlockExpression { } /// A variable captured inside a closure -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirCapturedVar { pub ident: HirIdent, @@ -283,7 +283,7 @@ pub struct HirCapturedVar { pub transitive_capture_index: Option, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirLambda { pub parameters: Vec<(HirPattern, Type)>, pub return_type: Type, diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index 9fff301f5f7..81de6c65e6f 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -10,7 +10,7 @@ use crate::{Distinctness, FunctionReturnType, Type, Visibility}; /// A Hir function is a block expression /// with a list of statements -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirFunction(ExprId); impl HirFunction { diff --git a/compiler/noirc_frontend/src/hir_def/stmt.rs b/compiler/noirc_frontend/src/hir_def/stmt.rs index 4d946690151..d9cfbbce416 100644 --- a/compiler/noirc_frontend/src/hir_def/stmt.rs +++ b/compiler/noirc_frontend/src/hir_def/stmt.rs @@ -8,7 +8,7 @@ use noirc_errors::Span; /// the Statement AST node. Unlike the AST node, any nested nodes /// are referred to indirectly via ExprId or StmtId, which can be /// used to retrieve the relevant node via the NodeInterner. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum HirStatement { Let(HirLetStatement), Constrain(HirConstrainStatement), @@ -19,7 +19,7 @@ pub enum HirStatement { Error, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirLetStatement { pub pattern: HirPattern, pub r#type: Type, @@ -35,7 +35,7 @@ impl HirLetStatement { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirForStatement { pub identifier: HirIdent, pub start_range: ExprId, @@ -44,7 +44,7 @@ pub struct HirForStatement { } /// Corresponds to `lvalue = expression;` in the source code -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirAssignStatement { pub lvalue: HirLValue, pub expression: ExprId, @@ -54,10 +54,10 @@ pub struct HirAssignStatement { /// This node also contains the FileId of the file the constrain /// originates from. This is used later in the SSA pass to issue /// an error if a constrain is found to be always false. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct HirConstrainStatement(pub ExprId, pub FileId, pub Option); -#[derive(Debug, Clone, Hash)] +#[derive(Debug, Clone, Hash, PartialEq)] pub enum HirPattern { Identifier(HirIdent), Mutable(Box, Span), diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 013cc04c2cd..9822eec7ee5 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -242,7 +242,7 @@ impl DefinitionId { impl From for Index { fn from(id: DefinitionId) -> Self { - Index(id.0) + Index(id.0, generational_arena::Index::from_raw_parts(0, 0)) } } @@ -254,7 +254,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(std::usize::MAX, generational_arena::Index::from_raw_parts(std::usize::MAX, 0))) } } @@ -263,7 +263,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index(0)) + ExprId(Index(0, generational_arena::Index::from_raw_parts(0, 0))) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -274,7 +274,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(std::usize::MAX, generational_arena::Index::from_raw_parts(std::usize::MAX, 0))) } } @@ -369,7 +369,7 @@ partialeq!(StmtId); /// 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 /// Multiple arenas and a single Arena -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub(crate) enum Node { Function(HirFunction), Statement(HirStatement), diff --git a/compiler/utils/arena/Cargo.toml b/compiler/utils/arena/Cargo.toml index 41c6ebc9a8b..ba4bbd0fa40 100644 --- a/compiler/utils/arena/Cargo.toml +++ b/compiler/utils/arena/Cargo.toml @@ -4,3 +4,6 @@ version.workspace = true authors.workspace = true edition.workspace = true license.workspace = true + +[dependencies] +generational-arena = "0.2.8" diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index a999e98c1c6..8cf0910c685 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -4,49 +4,134 @@ #![warn(clippy::semicolon_if_nothing_returned)] #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)] -pub struct Index(pub usize); +// pub struct Index(pub usize); +pub struct Index(pub usize, pub generational_arena::Index); -#[derive(Debug, Eq, PartialEq, Hash, Clone)] +// #[derive(Debug, Eq, PartialEq, Hash, Clone)] +#[derive(Debug, Clone)] pub struct Arena { - vec: Vec, + pub vec: Vec, + pub _arena: generational_arena::Arena, } impl Default for Arena { fn default() -> Self { - Self { vec: Vec::new() } + Self { + vec: Vec::new(), + _arena: generational_arena::Arena::default(), + } } } -impl core::ops::Index for Arena { + +impl core::ops::Index for Arena +// TODO: remove +where + T: PartialEq + std::fmt::Debug, +{ type Output = T; fn index(&self, index: Index) -> &Self::Output { - self.vec.index(index.0) + // self.vec.index(index.0) + let vec_out = self.vec.index(index.0); + let arena_out = self._arena.index(index.1); + + if vec_out == arena_out { + vec_out + } else { + panic!("Arena::index: vec_out != arena_out:\n Index: {:?}\n \n {:?} != {:?}", index, vec_out, arena_out) + } } } -impl core::ops::IndexMut for Arena { +impl core::ops::IndexMut for Arena +// TODO: remove +where + T: PartialEq + std::fmt::Debug, +{ fn index_mut(&mut self, index: Index) -> &mut Self::Output { - self.vec.index_mut(index.0) + // self.vec.index_mut(index.0) + let vec_out = self.vec.get(index.0); + let arena_out = self._arena.get(index.1); + + if vec_out == arena_out { + self.vec.index_mut(index.0) + } else { + let vec_iter = self.vec.iter().map(|x| format!("{:?}", x)).collect::>().join("\n\n"); + let arena_iter = self._arena.iter().map(|x| format!("{:?}", x.1)).collect::>().join("\n\n"); + panic!("Arena::index_mut: vec_out != arena_out:\n Index: {:?}\n \n {:?}\n !=\n {:?}\n\n Vec:\n{}\n\n Arena:\n{}", index, self.vec.get(index.0), self._arena.get(index.1), vec_iter, arena_iter) + } } } +// TODO: remove Clone impl Arena { - pub fn insert(&mut self, item: T) -> Index { + pub fn insert(&mut self, item: T) -> Index + where + T: Clone + std::fmt::Debug, + { let index = self.vec.len(); - self.vec.push(item); - Index(index) + self.vec.push(item.clone()); + + let index_arena = self._arena.insert(item); + if index == index_arena.into_raw_parts().0 { + Index(index, index_arena) + } else { + panic!("Arena::insert: index != index_arena: {:?} != {:?}", index, index_arena) + } + + // Index(index) } - pub fn get(&self, index: Index) -> Option<&T> { - self.vec.get(index.0) + pub fn get(&self, index: Index) -> Option<&T> + where + T: PartialEq + std::fmt::Debug, + { + // self.vec.get(index.0) + let item = self.vec.get(index.0); + let item_arena = self._arena.get(index.1); + if item == item_arena { + item + } else { + panic!("Arena::get: {:?} != {:?}", item, item_arena) + } } - pub fn get_mut(&mut self, index: Index) -> Option<&mut T> { - self.vec.get_mut(index.0) + pub fn get_mut(&mut self, index: Index) -> Option<&mut T> + where + T: PartialEq + std::fmt::Debug, + { + // self.vec.get_mut(index.0) + let item = self.vec.get_mut(index.0); + let item_arena = self._arena.get_mut(index.1); + if item == item_arena { + item + } else { + panic!("Arena::get: {:?} != {:?}", item, item_arena) + } } - pub fn iter(&self) -> impl Iterator { - self.vec.iter().enumerate().map(|(index, item)| (Index(index), item)) + pub fn iter(&self) -> impl Iterator + where + T: PartialEq + std::fmt::Debug, + { + // self.vec.iter().enumerate().map(|(index, item)| (Index(index), item)) + self.vec + .iter() + .enumerate() + .map(|(index, item)| (Index(index, generational_arena::Index::from_raw_parts(index, 0)), item)) + .zip(self._arena.iter()) + .map(|(item, item_arena)| { + if item.0.0 == item_arena.0.into_raw_parts().0 && item.1 == item_arena.1 { + Ok(item) + } else { + Err(format!("Arena::iter: {:?} != {:?}", item, item_arena)) + } + }) + .collect::, _>>() + .unwrap() + .into_iter() } + } + From 86d120eb5aec6d450c76f6ed62f1d47450d731ae Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 6 Feb 2024 15:48:41 -0500 Subject: [PATCH 09/16] wip debugging failing test --- .../src/hir/def_collector/dc_mod.rs | 6 -- .../noirc_frontend/src/hir/def_map/mod.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 8 +- compiler/utils/arena/src/lib.rs | 90 +++---------------- 4 files changed, 16 insertions(+), 90 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index f6d861c3484..3cd60c33b8b 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -235,9 +235,6 @@ impl<'a> ModCollector<'a> { unresolved_functions.push_fn(self.module_id, func_id, function); // Add function to scope/ns of the module - // TODO: remove before merge - let _ = self.def_collector.def_map.modules._arena[self.module_id.0.1] - .declare_function(name.clone(), func_id); let result = self.def_collector.def_map.modules[self.module_id.0] .declare_function(name, func_id); @@ -411,9 +408,6 @@ impl<'a> ModCollector<'a> { .def_interner .push_function_definition(func_id, modifiers, trait_id.0, location); - // TODO: remove before merge - let _ = self.def_collector.def_map.modules._arena[trait_id.0.local_id.0.1] - .declare_function(name.clone(), func_id); match self.def_collector.def_map.modules[trait_id.0.local_id.0] .declare_function(name.clone(), func_id) { diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 65835eaa086..aac19bd1a07 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, generational_arena::Index::from_raw_parts(std::usize::MAX, 0))) + LocalModuleId(Index(std::usize::MAX, generational_arena::Index::from_raw_parts(0,0))) } } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 9822eec7ee5..7dbf2ffcdac 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -242,7 +242,7 @@ impl DefinitionId { impl From for Index { fn from(id: DefinitionId) -> Self { - Index(id.0, generational_arena::Index::from_raw_parts(0, 0)) + Index(id.0, generational_arena::Index::from_raw_parts(0,0)) } } @@ -254,7 +254,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, generational_arena::Index::from_raw_parts(std::usize::MAX, 0))) + StmtId(Index(std::usize::MAX, generational_arena::Index::from_raw_parts(0,0))) } } @@ -263,7 +263,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index(0, generational_arena::Index::from_raw_parts(0, 0))) + ExprId(Index(0, generational_arena::Index::from_raw_parts(0,0))) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -274,7 +274,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, generational_arena::Index::from_raw_parts(std::usize::MAX, 0))) + FuncId(Index(std::usize::MAX, generational_arena::Index::from_raw_parts(0,0))) } } diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 8cf0910c685..8de117e6223 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -24,43 +24,17 @@ impl Default for Arena { } -impl core::ops::Index for Arena -// TODO: remove -where - T: PartialEq + std::fmt::Debug, -{ +impl core::ops::Index for Arena { type Output = T; fn index(&self, index: Index) -> &Self::Output { - // self.vec.index(index.0) - let vec_out = self.vec.index(index.0); - let arena_out = self._arena.index(index.1); - - if vec_out == arena_out { - vec_out - } else { - panic!("Arena::index: vec_out != arena_out:\n Index: {:?}\n \n {:?} != {:?}", index, vec_out, arena_out) - } + self.vec.index(index.0) } } -impl core::ops::IndexMut for Arena -// TODO: remove -where - T: PartialEq + std::fmt::Debug, -{ +impl core::ops::IndexMut for Arena { fn index_mut(&mut self, index: Index) -> &mut Self::Output { - // self.vec.index_mut(index.0) - let vec_out = self.vec.get(index.0); - let arena_out = self._arena.get(index.1); - - if vec_out == arena_out { - self.vec.index_mut(index.0) - } else { - let vec_iter = self.vec.iter().map(|x| format!("{:?}", x)).collect::>().join("\n\n"); - let arena_iter = self._arena.iter().map(|x| format!("{:?}", x.1)).collect::>().join("\n\n"); - panic!("Arena::index_mut: vec_out != arena_out:\n Index: {:?}\n \n {:?}\n !=\n {:?}\n\n Vec:\n{}\n\n Arena:\n{}", index, self.vec.get(index.0), self._arena.get(index.1), vec_iter, arena_iter) - } + self.vec.index_mut(index.0) } } @@ -68,69 +42,27 @@ where impl Arena { pub fn insert(&mut self, item: T) -> Index where - T: Clone + std::fmt::Debug, + T: Clone, { let index = self.vec.len(); self.vec.push(item.clone()); - let index_arena = self._arena.insert(item); - if index == index_arena.into_raw_parts().0 { - Index(index, index_arena) - } else { - panic!("Arena::insert: index != index_arena: {:?} != {:?}", index, index_arena) - } - - // Index(index) + Index(index, index_arena) } - pub fn get(&self, index: Index) -> Option<&T> - where - T: PartialEq + std::fmt::Debug, - { - // self.vec.get(index.0) - let item = self.vec.get(index.0); - let item_arena = self._arena.get(index.1); - if item == item_arena { - item - } else { - panic!("Arena::get: {:?} != {:?}", item, item_arena) - } + pub fn get(&self, index: Index) -> Option<&T> { + self.vec.get(index.0) } - pub fn get_mut(&mut self, index: Index) -> Option<&mut T> - where - T: PartialEq + std::fmt::Debug, - { - // self.vec.get_mut(index.0) - let item = self.vec.get_mut(index.0); - let item_arena = self._arena.get_mut(index.1); - if item == item_arena { - item - } else { - panic!("Arena::get: {:?} != {:?}", item, item_arena) - } + pub fn get_mut(&mut self, index: Index) -> Option<&mut T> { + self.vec.get_mut(index.0) } - pub fn iter(&self) -> impl Iterator - where - T: PartialEq + std::fmt::Debug, - { - // self.vec.iter().enumerate().map(|(index, item)| (Index(index), item)) + pub fn iter(&self) -> impl Iterator { self.vec .iter() .enumerate() .map(|(index, item)| (Index(index, generational_arena::Index::from_raw_parts(index, 0)), item)) - .zip(self._arena.iter()) - .map(|(item, item_arena)| { - if item.0.0 == item_arena.0.into_raw_parts().0 && item.1 == item_arena.1 { - Ok(item) - } else { - Err(format!("Arena::iter: {:?} != {:?}", item, item_arena)) - } - }) - .collect::, _>>() - .unwrap() - .into_iter() } } From cd279cd0d096261cbe82aa6160fd384e7bb0aefe Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 13:20:30 -0600 Subject: [PATCH 10/16] Fix compile errors from aztec_macros --- aztec_macros/src/lib.rs | 2 +- compiler/utils/arena/src/lib.rs | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index 7ca4b79eed8..82a9744de5d 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -696,7 +696,7 @@ fn collect_traits(context: &HirContext) -> Vec { crates .flat_map(|crate_id| context.def_map(&crate_id).map(|def_map| def_map.modules())) .flatten() - .flat_map(|(_, module)| { + .flat_map(|module| { module.type_definitions().filter_map(|typ| { if let ModuleDefId::TraitId(struct_id) = typ { Some(struct_id) diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 8de117e6223..354105a56b6 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -38,6 +38,26 @@ impl core::ops::IndexMut for Arena { } } +impl IntoIterator for Arena { + type Item = T; + + type IntoIter = as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.vec.into_iter() + } +} + +impl<'a, T> IntoIterator for &'a Arena { + type Item = &'a T; + + type IntoIter = <&'a Vec as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.vec.iter() + } +} + // TODO: remove Clone impl Arena { pub fn insert(&mut self, item: T) -> Index From ed62fd790856bf72adba4ef300d4b2d550cd5768 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 13:53:08 -0600 Subject: [PATCH 11/16] Fmt --- compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 8 ++++---- compiler/utils/arena/src/lib.rs | 15 ++++----------- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index aac19bd1a07..5fa44b3dd15 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, generational_arena::Index::from_raw_parts(0,0))) + LocalModuleId(Index(std::usize::MAX, generational_arena::Index::from_raw_parts(0, 0))) } } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 62f91c11939..46381b59cd0 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -278,7 +278,7 @@ impl DefinitionId { impl From for Index { fn from(id: DefinitionId) -> Self { - Index(id.0, generational_arena::Index::from_raw_parts(0,0)) + Index(id.0, generational_arena::Index::from_raw_parts(0, 0)) } } @@ -301,7 +301,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, generational_arena::Index::from_raw_parts(0,0))) + StmtId(Index(std::usize::MAX, generational_arena::Index::from_raw_parts(0, 0))) } } @@ -310,7 +310,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index(0, generational_arena::Index::from_raw_parts(0,0))) + ExprId(Index(0, generational_arena::Index::from_raw_parts(0, 0))) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -321,7 +321,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, generational_arena::Index::from_raw_parts(0,0))) + FuncId(Index(std::usize::MAX, generational_arena::Index::from_raw_parts(0, 0))) } } diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 354105a56b6..75593e26f63 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -16,14 +16,10 @@ pub struct Arena { impl Default for Arena { fn default() -> Self { - Self { - vec: Vec::new(), - _arena: generational_arena::Arena::default(), - } + Self { vec: Vec::new(), _arena: generational_arena::Arena::default() } } } - impl core::ops::Index for Arena { type Output = T; @@ -79,11 +75,8 @@ impl Arena { } pub fn iter(&self) -> impl Iterator { - self.vec - .iter() - .enumerate() - .map(|(index, item)| (Index(index, generational_arena::Index::from_raw_parts(index, 0)), item)) + self.vec.iter().enumerate().map(|(index, item)| { + (Index(index, generational_arena::Index::from_raw_parts(index, 0)), item) + }) } - } - From 0f4f32df3309b2abedd8eb646fe90686541d3fe4 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Feb 2024 14:30:06 -0600 Subject: [PATCH 12/16] Fix frontend test --- compiler/noirc_frontend/src/hir/type_check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index b6edb74609b..4dcddbcca02 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)) + LocalModuleId(arena::Index(0, generational_arena::Index::from_raw_parts(0, 0))) } fn module_id(&self) -> ModuleId { From aa35b34b92c92bb9446b26c553e09da07aca0a22 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 13 Feb 2024 17:46:21 -0500 Subject: [PATCH 13/16] remove generational arena test stubs, indices, etc --- Cargo.lock | 13 ------------- Cargo.toml | 4 ---- compiler/noirc_frontend/Cargo.toml | 1 - compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 8 ++++---- compiler/utils/arena/Cargo.toml | 3 --- compiler/utils/arena/src/lib.rs | 11 ++++------- 7 files changed, 9 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ddbecdd822e..4d8b12d5379 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -213,9 +213,6 @@ checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" [[package]] name = "arena" version = "0.24.0" -dependencies = [ - "generational-arena", -] [[package]] name = "ark-bls12-381" @@ -1842,15 +1839,6 @@ dependencies = [ "byteorder", ] -[[package]] -name = "generational-arena" -version = "0.2.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "877e94aff08e743b651baaea359664321055749b398adff8740a7399af7796e7" -dependencies = [ - "cfg-if 1.0.0", -] - [[package]] name = "generic-array" version = "0.14.7" @@ -3004,7 +2992,6 @@ dependencies = [ "arena", "chumsky", "fm", - "generational-arena", "iter-extended", "noirc_errors", "noirc_printable_type", diff --git a/Cargo.toml b/Cargo.toml index 4f95e3b0821..77058554aff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,12 +69,10 @@ noirc_errors = { path = "compiler/noirc_errors" } noirc_evaluator = { path = "compiler/noirc_evaluator" } noirc_frontend = { path = "compiler/noirc_frontend" } noirc_printable_type = { path = "compiler/noirc_printable_type" } -noir_wasm = { path = "compiler/wasm" } # Noir tooling workspace dependencies nargo = { path = "tooling/nargo" } nargo_fmt = { path = "tooling/nargo_fmt" } -nargo_cli = { path = "tooling/nargo_cli" } nargo_toml = { path = "tooling/nargo_toml" } noir_lsp = { path = "tooling/lsp" } noir_debugger = { path = "tooling/debugger" } @@ -97,8 +95,6 @@ getrandom = "0.2" # Debugger dap = "0.4.1-alpha1" - -cfg-if = "1.0.0" clap = { version = "4.3.19", features = ["derive", "env"] } codespan = { version = "0.11.1", features = ["serialization"] } codespan-lsp = "0.11.1" diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index 40bb44b9455..a3a8d460572 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -23,7 +23,6 @@ rustc-hash = "1.1.0" small-ord-set = "0.1.3" regex = "1.9.1" tracing.workspace = true -generational-arena = "0.2.8" petgraph = "0.6" [dev-dependencies] diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 5fa44b3dd15..5a22214095b 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, generational_arena::Index::from_raw_parts(0, 0))) + LocalModuleId(Index(std::usize::MAX)) } } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index abd39c665de..87542b1b9f8 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -278,7 +278,7 @@ impl DefinitionId { impl From for Index { fn from(id: DefinitionId) -> Self { - Index(id.0, generational_arena::Index::from_raw_parts(0, 0)) + Index(id.0) } } @@ -301,7 +301,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, generational_arena::Index::from_raw_parts(0, 0))) + StmtId(Index(std::usize::MAX)) } } @@ -310,7 +310,7 @@ pub struct ExprId(Index); impl ExprId { pub fn empty_block_id() -> ExprId { - ExprId(Index(0, generational_arena::Index::from_raw_parts(0, 0))) + ExprId(Index(0)) } } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] @@ -321,7 +321,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, generational_arena::Index::from_raw_parts(0, 0))) + FuncId(Index(std::usize::MAX)) } } diff --git a/compiler/utils/arena/Cargo.toml b/compiler/utils/arena/Cargo.toml index ba4bbd0fa40..41c6ebc9a8b 100644 --- a/compiler/utils/arena/Cargo.toml +++ b/compiler/utils/arena/Cargo.toml @@ -4,6 +4,3 @@ version.workspace = true authors.workspace = true edition.workspace = true license.workspace = true - -[dependencies] -generational-arena = "0.2.8" diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 75593e26f63..158c9c1fa9d 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -4,19 +4,17 @@ #![warn(clippy::semicolon_if_nothing_returned)] #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)] -// pub struct Index(pub usize); -pub struct Index(pub usize, pub generational_arena::Index); +pub struct Index(pub usize); // #[derive(Debug, Eq, PartialEq, Hash, Clone)] #[derive(Debug, Clone)] pub struct Arena { pub vec: Vec, - pub _arena: generational_arena::Arena, } impl Default for Arena { fn default() -> Self { - Self { vec: Vec::new(), _arena: generational_arena::Arena::default() } + Self { vec: Vec::new() } } } @@ -62,8 +60,7 @@ impl Arena { { let index = self.vec.len(); self.vec.push(item.clone()); - let index_arena = self._arena.insert(item); - Index(index, index_arena) + Index(index) } pub fn get(&self, index: Index) -> Option<&T> { @@ -76,7 +73,7 @@ impl Arena { pub fn iter(&self) -> impl Iterator { self.vec.iter().enumerate().map(|(index, item)| { - (Index(index, generational_arena::Index::from_raw_parts(index, 0)), item) + (Index(index), item) }) } } From 68d7cb6088795b8e919b4e20a7f5adc2262b9c91 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 13 Feb 2024 17:52:24 -0500 Subject: [PATCH 14/16] cargo fmt --- compiler/utils/arena/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index 158c9c1fa9d..cf834ed7d59 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -72,8 +72,6 @@ impl Arena { } pub fn iter(&self) -> impl Iterator { - self.vec.iter().enumerate().map(|(index, item)| { - (Index(index), item) - }) + self.vec.iter().enumerate().map(|(index, item)| (Index(index), item)) } } From e49fde456eb10468abcef132e4bdcbdde038c0e4 Mon Sep 17 00:00:00 2001 From: jfecher Date: Fri, 16 Feb 2024 13:06:59 -0600 Subject: [PATCH 15/16] fix: DefinitionIds should not be stored in id_to_type (#4397) # Description ## Problem\* Resolves the underlying issue with #4207 where definition/expr types are being overwritten by other exprs/definitions because DefinitionIds are converted into Indexes which may clash with those from expressions. ## Summary\* Separates out `id_to_type` into another map for only definitions: `definition_to_type` to resolve the clash. I've also removed the `impl From for Index` to prevent this issue in the future. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- aztec_macros/src/lib.rs | 7 +-- .../noirc_frontend/src/hir/def_map/mod.rs | 2 +- .../src/hir/resolution/resolver.rs | 2 +- .../noirc_frontend/src/hir/type_check/expr.rs | 15 ++--- .../noirc_frontend/src/hir/type_check/mod.rs | 2 +- .../noirc_frontend/src/hir/type_check/stmt.rs | 2 +- compiler/noirc_frontend/src/hir_def/types.rs | 9 ++- .../src/monomorphization/debug.rs | 4 +- .../src/monomorphization/mod.rs | 4 +- compiler/noirc_frontend/src/node_interner.rs | 57 +++++++------------ compiler/utils/arena/src/lib.rs | 21 ++++++- 11 files changed, 65 insertions(+), 60 deletions(-) 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)] From 7885574494cc3ecc058eac0f5a0cc2ead123a54d Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Fri, 16 Feb 2024 14:28:56 -0500 Subject: [PATCH 16/16] cleanup debugging, remove generational arena note from deny.toml --- .../src/hir/def_map/item_scope.rs | 2 +- .../src/hir/def_map/module_data.rs | 2 +- compiler/noirc_frontend/src/hir_def/expr.rs | 30 +++++++++---------- .../noirc_frontend/src/hir_def/function.rs | 2 +- compiler/noirc_frontend/src/hir_def/stmt.rs | 12 ++++---- compiler/noirc_frontend/src/node_interner.rs | 2 +- compiler/utils/arena/src/lib.rs | 11 ++----- deny.toml | 2 +- 8 files changed, 29 insertions(+), 34 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/item_scope.rs b/compiler/noirc_frontend/src/hir/def_map/item_scope.rs index 50f1f50cbe5..523def89518 100644 --- a/compiler/noirc_frontend/src/hir/def_map/item_scope.rs +++ b/compiler/noirc_frontend/src/hir/def_map/item_scope.rs @@ -12,7 +12,7 @@ pub enum Visibility { Public, } -#[derive(Clone, Default, Debug, PartialEq, Eq)] +#[derive(Default, Debug, PartialEq, Eq)] pub struct ItemScope { types: HashMap, values: HashMap, diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index 6284cf252c1..309618dd011 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -11,7 +11,7 @@ use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs}; /// Contains the actual contents of a module: its parent (if one exists), /// children, and scope with all definitions defined within the scope. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq)] pub struct ModuleData { pub parent: Option, pub children: HashMap, diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 70f53a24532..75ed68af0f6 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -14,7 +14,7 @@ use super::types::{StructType, Type}; /// references other HIR nodes indirectly via IDs rather than directly via /// boxing. Variables in HirExpressions are tagged with their DefinitionId /// from the definition that refers to them so there is no ambiguity with names. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub enum HirExpression { Ident(HirIdent), Literal(HirLiteral), @@ -109,7 +109,7 @@ impl HirBinaryOp { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub enum HirLiteral { Array(HirArrayLiteral), Bool(bool), @@ -119,19 +119,19 @@ pub enum HirLiteral { Unit, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub enum HirArrayLiteral { Standard(Vec), Repeated { repeated_element: ExprId, length: Type }, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirPrefixExpression { pub operator: UnaryOp, pub rhs: ExprId, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirInfixExpression { pub lhs: ExprId, pub operator: HirBinaryOp, @@ -146,7 +146,7 @@ pub struct HirInfixExpression { /// This is always a struct field access `my_struct.field` /// and never a method call. The later is represented by HirMethodCallExpression. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirMemberAccess { pub lhs: ExprId, // This field is not an IdentId since the rhs of a field @@ -160,7 +160,7 @@ pub struct HirMemberAccess { pub is_offset: bool, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirIfExpression { pub condition: ExprId, pub consequence: ExprId, @@ -168,13 +168,13 @@ pub struct HirIfExpression { } // `lhs as type` in the source code -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirCastExpression { pub lhs: ExprId, pub r#type: Type, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirCallExpression { pub func: ExprId, pub arguments: Vec, @@ -185,7 +185,7 @@ pub struct HirCallExpression { /// lowered into HirCallExpression nodes /// after type checking resolves the object /// type and the method it calls. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirMethodCallExpression { pub method: Ident, pub object: ExprId, @@ -239,7 +239,7 @@ impl HirMethodCallExpression { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirConstructorExpression { pub r#type: Shared, pub struct_generics: Vec, @@ -253,13 +253,13 @@ pub struct HirConstructorExpression { } /// Indexing, as in `array[index]` -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirIndexExpression { pub collection: ExprId, pub index: ExprId, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirBlockExpression(pub Vec); impl HirBlockExpression { @@ -269,7 +269,7 @@ impl HirBlockExpression { } /// A variable captured inside a closure -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirCapturedVar { pub ident: HirIdent, @@ -283,7 +283,7 @@ pub struct HirCapturedVar { pub transitive_capture_index: Option, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirLambda { pub parameters: Vec<(HirPattern, Type)>, pub return_type: Type, diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index 0b0365a66f6..d3ab2a9393b 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -10,7 +10,7 @@ use crate::{Distinctness, FunctionReturnType, Type, Visibility}; /// A Hir function is a block expression /// with a list of statements -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirFunction(ExprId); impl HirFunction { diff --git a/compiler/noirc_frontend/src/hir_def/stmt.rs b/compiler/noirc_frontend/src/hir_def/stmt.rs index fbca33c4912..b910be1fdda 100644 --- a/compiler/noirc_frontend/src/hir_def/stmt.rs +++ b/compiler/noirc_frontend/src/hir_def/stmt.rs @@ -8,7 +8,7 @@ use noirc_errors::{Location, Span}; /// the Statement AST node. Unlike the AST node, any nested nodes /// are referred to indirectly via ExprId or StmtId, which can be /// used to retrieve the relevant node via the NodeInterner. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub enum HirStatement { Let(HirLetStatement), Constrain(HirConstrainStatement), @@ -19,7 +19,7 @@ pub enum HirStatement { Error, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirLetStatement { pub pattern: HirPattern, pub r#type: Type, @@ -35,7 +35,7 @@ impl HirLetStatement { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirForStatement { pub identifier: HirIdent, pub start_range: ExprId, @@ -44,7 +44,7 @@ pub struct HirForStatement { } /// Corresponds to `lvalue = expression;` in the source code -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirAssignStatement { pub lvalue: HirLValue, pub expression: ExprId, @@ -54,10 +54,10 @@ pub struct HirAssignStatement { /// This node also contains the FileId of the file the constrain /// originates from. This is used later in the SSA pass to issue /// an error if a constrain is found to be always false. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct HirConstrainStatement(pub ExprId, pub FileId, pub Option); -#[derive(Debug, Clone, Hash, PartialEq)] +#[derive(Debug, Clone, Hash)] pub enum HirPattern { Identifier(HirIdent), Mutable(Box, Location), diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index f0a006489f4..a791112926e 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -398,7 +398,7 @@ into_index!(StmtId); /// 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 /// Multiple arenas and a single Arena -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub(crate) enum Node { Function(HirFunction), Statement(HirStatement), diff --git a/compiler/utils/arena/src/lib.rs b/compiler/utils/arena/src/lib.rs index ec9b1d9a837..2d117304e16 100644 --- a/compiler/utils/arena/src/lib.rs +++ b/compiler/utils/arena/src/lib.rs @@ -25,8 +25,7 @@ impl Index { } } -// #[derive(Debug, Eq, PartialEq, Hash, Clone)] -#[derive(Debug, Clone)] +#[derive(Clone, Debug)] pub struct Arena { pub vec: Vec, } @@ -71,14 +70,10 @@ impl<'a, T> IntoIterator for &'a Arena { } } -// TODO: remove Clone impl Arena { - pub fn insert(&mut self, item: T) -> Index - where - T: Clone, - { + pub fn insert(&mut self, item: T) -> Index { let index = self.vec.len(); - self.vec.push(item.clone()); + self.vec.push(item); Index(index) } diff --git a/deny.toml b/deny.toml index a3e506984c9..72150f08a3c 100644 --- a/deny.toml +++ b/deny.toml @@ -54,7 +54,7 @@ allow = [ "LicenseRef-ring", # https://github.com/rustls/webpki/blob/main/LICENSE ISC Style "LicenseRef-rustls-webpki", - # bitmaps 2.1.0, generational-arena 0.2.9,im 15.1.0 + # bitmaps 2.1.0, im 15.1.0 "MPL-2.0", # Boost Software License "BSL-1.0",