Skip to content

Commit

Permalink
fix: error on duplicate struct field (#5585)
Browse files Browse the repository at this point in the history
# Description

## Problem

Resolves #5030

## Summary

We weren't checking for duplicate struct fields anywhere.

## Additional Context

None.

## 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\*

- [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.
  • Loading branch information
asterite authored Jul 23, 2024
1 parent 453ed59 commit 3aed671
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 1 deletion.
28 changes: 27 additions & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ impl<'a> ModCollector<'a> {
}

/// Collect any struct definitions declared within the ast.
/// Returns a vector of errors if any structs were already defined.
/// Returns a vector of errors if any structs were already defined,
/// or if a struct has duplicate fields in it.
fn collect_structs(
&mut self,
context: &mut Context,
Expand All @@ -271,6 +272,8 @@ impl<'a> ModCollector<'a> {
) -> Vec<(CompilationError, FileId)> {
let mut definition_errors = vec![];
for struct_definition in types {
self.check_duplicate_field_names(&struct_definition, &mut definition_errors);

let name = struct_definition.name.clone();

let unresolved = UnresolvedStruct {
Expand Down Expand Up @@ -330,6 +333,29 @@ impl<'a> ModCollector<'a> {
definition_errors
}

fn check_duplicate_field_names(
&self,
struct_definition: &NoirStruct,
definition_errors: &mut Vec<(CompilationError, FileId)>,
) {
let mut seen_field_names = std::collections::HashSet::new();
for (field_name, _) in &struct_definition.fields {
if seen_field_names.insert(field_name) {
continue;
}

let previous_field_name = *seen_field_names.get(field_name).unwrap();
definition_errors.push((
DefCollectorErrorKind::DuplicateField {
first_def: previous_field_name.clone(),
second_def: field_name.clone(),
}
.into(),
self.file_id,
));
}
}

/// Collect any type aliases definitions declared within the ast.
/// Returns a vector of errors if any type aliases were already defined.
fn collect_type_aliases(
Expand Down
19 changes: 19 additions & 0 deletions compiler/noirc_frontend/src/hir/def_collector/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub enum DuplicateType {
pub enum DefCollectorErrorKind {
#[error("duplicate {typ} found in namespace")]
Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident },
#[error("duplicate struct field {first_def}")]
DuplicateField { first_def: Ident, second_def: Ident },
#[error("unresolved import")]
UnresolvedModuleDecl { mod_name: Ident, expected_path: String, alternative_path: String },
#[error("overlapping imports")]
Expand Down Expand Up @@ -132,6 +134,23 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
diag
}
}
DefCollectorErrorKind::DuplicateField { first_def, second_def } => {
let primary_message = format!(
"Duplicate definitions of struct field with name {} found",
&first_def.0.contents
);
{
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let mut diag = Diagnostic::simple_error(
primary_message,
"First definition found here".to_string(),
first_span,
);
diag.add_secondary("Second definition found here".to_string(), second_span);
diag
}
}
DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path, alternative_path } => {
let span = mod_name.0.span();
let mod_name = &mod_name.0.contents;
Expand Down
28 changes: 28 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2494,3 +2494,31 @@ fn bit_not_on_untyped_integer() {
"#;
assert_no_errors(src);
}

#[test]
fn duplicate_struct_field() {
let src = r#"
struct Foo {
x: i32,
x: i32,
}
fn main() {}
"#;
let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::DefinitionError(DefCollectorErrorKind::DuplicateField {
first_def,
second_def,
}) = &errors[0].0
else {
panic!("Expected a duplicate field error, got {:?}", errors[0].0);
};

assert_eq!(first_def.to_string(), "x");
assert_eq!(second_def.to_string(), "x");

assert_eq!(first_def.span().start(), 26);
assert_eq!(second_def.span().start(), 42);
}

0 comments on commit 3aed671

Please sign in to comment.