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

feat(improve): Remove scan through globals #6282

Merged
merged 9 commits into from
Oct 11, 2024
1 change: 0 additions & 1 deletion compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,6 @@ impl<'context> Elaborator<'context> {
DefinitionKind::Local(None),
&mut parameter_idents,
true, // warn_if_unused
None,
);

parameters.push((pattern, typ.clone(), visibility));
Expand Down
56 changes: 10 additions & 46 deletions compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ impl<'context> Elaborator<'context> {
None,
&mut Vec::new(),
warn_if_unused,
None,
)
}

Expand All @@ -50,7 +49,6 @@ impl<'context> Elaborator<'context> {
definition_kind: DefinitionKind,
created_ids: &mut Vec<HirIdent>,
warn_if_unused: bool,
global_id: Option<GlobalId>,
) -> HirPattern {
self.elaborate_pattern_mut(
pattern,
Expand All @@ -59,7 +57,6 @@ impl<'context> Elaborator<'context> {
None,
created_ids,
warn_if_unused,
global_id,
)
}

Expand All @@ -72,7 +69,6 @@ impl<'context> Elaborator<'context> {
mutable: Option<Span>,
new_definitions: &mut Vec<HirIdent>,
warn_if_unused: bool,
global_id: Option<GlobalId>,
) -> HirPattern {
match pattern {
Pattern::Identifier(name) => {
Expand All @@ -82,7 +78,7 @@ impl<'context> Elaborator<'context> {
(Some(_), DefinitionKind::Local(_)) => DefinitionKind::Local(None),
(_, other) => other,
};
let ident = if let Some(global_id) = global_id {
let ident = if let DefinitionKind::Global(global_id) = definition {
// Globals don't need to be added to scope, they're already in the def_maps
let id = self.interner.get_global(global_id).definition_id;
let location = Location::new(name.span(), self.file);
Expand Down Expand Up @@ -112,7 +108,6 @@ impl<'context> Elaborator<'context> {
Some(span),
new_definitions,
warn_if_unused,
global_id,
);
let location = Location::new(span, self.file);
HirPattern::Mutable(Box::new(pattern), location)
Expand Down Expand Up @@ -144,7 +139,6 @@ impl<'context> Elaborator<'context> {
mutable,
new_definitions,
warn_if_unused,
global_id,
)
});
let location = Location::new(span, self.file);
Expand All @@ -168,7 +162,6 @@ impl<'context> Elaborator<'context> {
mutable,
new_definitions,
warn_if_unused,
global_id,
)
}
}
Expand Down Expand Up @@ -283,7 +276,6 @@ impl<'context> Elaborator<'context> {
mutable,
new_definitions,
true, // warn_if_unused
None,
);

if unseen_fields.contains(&field) {
Expand Down Expand Up @@ -329,8 +321,8 @@ impl<'context> Elaborator<'context> {
warn_if_unused: bool,
definition: DefinitionKind,
) -> HirIdent {
if definition.is_global() {
return self.add_global_variable_decl(name, definition);
if let DefinitionKind::Global(global_id) = definition {
return self.add_global_variable_decl(name, global_id);
}

let location = Location::new(name.span(), self.file);
Expand Down Expand Up @@ -375,47 +367,19 @@ impl<'context> Elaborator<'context> {
}
}

pub fn add_global_variable_decl(
&mut self,
name: Ident,
definition: DefinitionKind,
) -> HirIdent {
let comptime = self.in_comptime_context();
pub fn add_global_variable_decl(&mut self, name: Ident, global_id: GlobalId) -> HirIdent {
let scope = self.scopes.get_mut_scope();

// This check is necessary to maintain the same definition ids in the interner. Currently, each function uses a new resolver that has its own ScopeForest and thus global scope.
// We must first check whether an existing definition ID has been inserted as otherwise there will be multiple definitions for the same global statement.
// This leads to an error in evaluation where the wrong definition ID is selected when evaluating a statement using the global. The check below prevents this error.
let mut global_id = None;
let global = self.interner.get_all_globals();
for global_info in global {
if global_info.local_id == self.local_module && global_info.ident == name {
global_id = Some(global_info.id);
}
}

let (ident, resolver_meta) = if let Some(id) = global_id {
let global = self.interner.get_global(id);
let hir_ident = HirIdent::non_trait_method(global.definition_id, global.location);
let ident = hir_ident.clone();
let resolver_meta = ResolverMeta { num_times_used: 0, ident, warn_if_unused: true };
(hir_ident, resolver_meta)
} else {
let location = Location::new(name.span(), self.file);
let name = name.0.contents.clone();
let id = self.interner.push_definition(name, false, comptime, definition, location);
let ident = HirIdent::non_trait_method(id, location);
let resolver_meta =
ResolverMeta { num_times_used: 0, ident: ident.clone(), warn_if_unused: true };
(ident, resolver_meta)
};
let global = self.interner.get_global(global_id);
let ident = HirIdent::non_trait_method(global.definition_id, global.location);
let resolver_meta =
ResolverMeta { num_times_used: 0, ident: ident.clone(), warn_if_unused: true };

let old_global_value = scope.add_key_value(name.0.contents.clone(), resolver_meta);
if let Some(old_global_value) = old_global_value {
self.push_err(ResolverError::DuplicateDefinition {
name: name.0.contents.clone(),
first_span: old_global_value.ident.location.span,
second_span: name.span(),
name: name.0.contents,
first_span: old_global_value.ident.location.span,
});
}
ident
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ impl<'context> Elaborator<'context> {
definition,
&mut Vec::new(),
warn_if_unused,
global_id,
);

let attributes = let_stmt.attributes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2462,7 +2462,6 @@ fn function_def_set_parameters(
DefinitionKind::Local(None),
&mut parameter_idents,
true, // warn_if_unused
None,
)
});

Expand Down
Loading