Skip to content

Commit

Permalink
chore(refactor): Remove optional global_id from methods that can get …
Browse files Browse the repository at this point in the history
…it from the definition (#6283)

# Description

## Problem\*

Followup to #6282

## Summary\*
Some methods in the `Elaborator` received a `global_id:
Option<GlobalId>` and a `definition: DefinitionKind` parameter. We found
that `Elaborator::elaborate_let` actually created `definition` to carry
the same global ID as it was given, so `global_id: Some(id)` and
`definition: DefinitionKind::Global(id)` always carried the same value.
We added an
[assertion](1d2bd36)
that checked that they are never different.

The next logical step is to remove the possibility of confusion by only
carrying the ID in the definition.

## Additional Context

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
aakoshh committed Oct 11, 2024
1 parent 5f552d1 commit d1c3189
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 14 deletions.
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
12 changes: 1 addition & 11 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,9 +78,7 @@ impl<'context> Elaborator<'context> {
(Some(_), DefinitionKind::Local(_)) => DefinitionKind::Local(None),
(_, other) => other,
};
let ident = if let Some(global_id) = global_id {
// Sanity check that we don't have conflicting globals.
assert_eq!(definition, DefinitionKind::Global(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 @@ -114,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 @@ -146,7 +139,6 @@ impl<'context> Elaborator<'context> {
mutable,
new_definitions,
warn_if_unused,
global_id,
)
});
let location = Location::new(span, self.file);
Expand All @@ -170,7 +162,6 @@ impl<'context> Elaborator<'context> {
mutable,
new_definitions,
warn_if_unused,
global_id,
)
}
}
Expand Down Expand Up @@ -285,7 +276,6 @@ impl<'context> Elaborator<'context> {
mutable,
new_definitions,
true, // warn_if_unused
None,
);

if unseen_fields.contains(&field) {
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

0 comments on commit d1c3189

Please sign in to comment.