Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove dependency on generational-arena #4207

Merged
merged 29 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f27b439
wip: remove dependency on generational-arena, add minimal vec-based a…
michaeljklein Jan 30, 2024
7f95c26
cargo fmt
michaeljklein Jan 30, 2024
648c00c
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Jan 30, 2024
f761a97
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Jan 30, 2024
da7e219
cargo clippy
michaeljklein Jan 30, 2024
f836981
revert warnings for unused crates
michaeljklein Jan 30, 2024
257ca9d
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Jan 30, 2024
10f161a
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Jan 30, 2024
9203fe5
rename ix -> index
michaeljklein Jan 30, 2024
ad7958e
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Jan 31, 2024
00531d2
make Index a tuple struct
michaeljklein Jan 31, 2024
b8098c8
Update compiler/noirc_frontend/src/hir/type_check/mod.rs
kevaundray Feb 1, 2024
d8581f4
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Feb 5, 2024
151a23c
added back generational_arena, added copy of generational_arena::{Are…
michaeljklein Feb 5, 2024
86d120e
wip debugging failing test
michaeljklein Feb 6, 2024
5d0a77b
Merge remote-tracking branch 'origin/master' into michaeljklein/vec-a…
kevaundray Feb 12, 2024
cd279cd
Fix compile errors from aztec_macros
jfecher Feb 12, 2024
ed62fd7
Fmt
jfecher Feb 12, 2024
e1aa513
Merge branch 'master' into michaeljklein/vec-arena
jfecher Feb 12, 2024
0f4f32d
Fix frontend test
jfecher Feb 12, 2024
ec1c5c4
Merge branch 'master' into michaeljklein/vec-arena
kevaundray Feb 13, 2024
948447c
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Feb 13, 2024
aa35b34
remove generational arena test stubs, indices, etc
michaeljklein Feb 13, 2024
9dbb84c
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Feb 13, 2024
68d7cb6
cargo fmt
michaeljklein Feb 13, 2024
b8d5b2e
Merge branch 'master' into michaeljklein/vec-arena
kevaundray Feb 15, 2024
e49fde4
fix: DefinitionIds should not be stored in id_to_type (#4397)
jfecher Feb 16, 2024
7885574
cleanup debugging, remove generational arena note from deny.toml
michaeljklein Feb 16, 2024
881373d
Merge branch 'master' into michaeljklein/vec-arena
michaeljklein Feb 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -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"
Expand Down
9 changes: 4 additions & 5 deletions aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ fn collect_traits(context: &HirContext) -> Vec<TraitId> {
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)
Expand Down Expand Up @@ -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()));
});

Expand Down Expand Up @@ -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
Expand All @@ -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()),
})?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::dummy())
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
15 changes: 8 additions & 7 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
Expand All @@ -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
}

Expand Down Expand Up @@ -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
});
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ mod test {
}

fn local_module_id(&self) -> LocalModuleId {
LocalModuleId(arena::Index::from_raw_parts(0, 0))
LocalModuleId(arena::Index::unsafe_zeroed())
}

fn module_id(&self) -> ModuleId {
Expand Down Expand Up @@ -509,7 +509,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));

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/type_check/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
};

Expand Down
9 changes: 4 additions & 5 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/monomorphization/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
}
Expand Down Expand Up @@ -1040,7 +1040,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.
Expand Down
57 changes: 22 additions & 35 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Index, Type>,

// Similar to `id_to_type` but maps definitions to their type
definition_to_type: HashMap<DefinitionId, Type>,

// Struct map.
//
// Each struct definition is possibly shared across multiple type nodes.
Expand Down Expand Up @@ -277,12 +278,6 @@ impl DefinitionId {
}
}

impl From<DefinitionId> for Index {
fn from(id: DefinitionId) -> Self {
Index::from_raw_parts(id.0, u64::MAX)
jfecher marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// An ID for a global value
#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)]
pub struct GlobalId(usize);
Expand All @@ -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::from_raw_parts(std::usize::MAX, 0))
StmtId(Index::dummy())
}
}

Expand All @@ -311,7 +306,7 @@ pub struct ExprId(Index);

impl ExprId {
pub fn empty_block_id() -> ExprId {
ExprId(Index::from_raw_parts(0, 0))
ExprId(Index::unsafe_zeroed())
}
}
#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)]
Expand All @@ -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::from_raw_parts(std::usize::MAX, 0))
FuncId(Index::dummy())
}
}

Expand Down Expand Up @@ -396,23 +391,9 @@ macro_rules! into_index {
};
}

macro_rules! partialeq {
($id_type:ty) => {
impl PartialEq<usize> for &$id_type {
fn eq(&self, other: &usize) -> bool {
let (index, _) = self.0.into_raw_parts();
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
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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() {
Expand Down
5 changes: 0 additions & 5 deletions compiler/utils/arena/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Loading
Loading