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.

Sign up for GitHub

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: Refactor DefCollector duplicate errors #2231

Merged
merged 3 commits into from
Aug 10, 2023
Merged

chore: Refactor DefCollector duplicate errors #2231

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
936e761
chore: Refactor DefCollector duplicate errors
ymadzhunkov Aug 9, 2023
ca531e6
Better message
ymadzhunkov Aug 9, 2023
0edf978
Convert duplicate type to enum
ymadzhunkov Aug 10, 2023
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
13 changes: 10 additions & 3 deletions crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ impl DefCollector {
.import(name.clone(), ns);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateImport { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: "import".to_string(),
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(root_file_id));
}
}
Expand Down Expand Up @@ -256,8 +260,11 @@ fn collect_impls(
let result = module.declare_function(method.name_ident().clone(), *method_id);

if let Err((first_def, second_def)) = result {
let err =
DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: "function".to_string(),
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(unresolved.file_id));
}
}
Expand Down
30 changes: 25 additions & 5 deletions crates/noirc_frontend/src/hir/def_collector/dc_mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ impl<'a> ModCollector<'a> {
self.def_collector.def_map.modules[self.module_id.0].declare_global(name, stmt_id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateGlobal { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: "global".to_string(),
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
}

Expand Down Expand Up @@ -142,7 +146,11 @@ impl<'a> ModCollector<'a> {
.declare_function(name, func_id);

if let Err((first_def, second_def)) = result {
let error = DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let error = DefCollectorErrorKind::Duplicate {
typ: "function".to_string(),
first_def,
second_def,
};
errors.push(error.into_file_diagnostic(self.file_id));
}
}
Expand Down Expand Up @@ -172,7 +180,11 @@ impl<'a> ModCollector<'a> {
self.def_collector.def_map.modules[self.module_id.0].declare_struct(name, id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: "type definition".to_string(),
jfecher marked this conversation as resolved.
Show resolved Hide resolved
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
}

Expand Down Expand Up @@ -211,7 +223,11 @@ impl<'a> ModCollector<'a> {
.declare_type_alias(name, type_alias_id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: "function".to_string(),
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
}

Expand Down Expand Up @@ -323,7 +339,11 @@ impl<'a> ModCollector<'a> {
if let Err((first_def, second_def)) =
modules[self.module_id.0].declare_child_module(mod_name.to_owned(), mod_id)
{
let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def };
let err = DefCollectorErrorKind::Duplicate {
typ: "module".to_string(),
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
return None;
}
Expand Down
79 changes: 20 additions & 59 deletions crates/noirc_frontend/src/hir/def_collector/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,8 @@ use thiserror::Error;

#[derive(Error, Debug)]
pub enum DefCollectorErrorKind {
#[error("duplicate function found in namespace")]
DuplicateFunction { first_def: Ident, second_def: Ident },
#[error("duplicate function found in namespace")]
DuplicateModuleDecl { first_def: Ident, second_def: Ident },
#[error("duplicate import")]
DuplicateImport { first_def: Ident, second_def: Ident },
#[error("duplicate global found in namespace")]
DuplicateGlobal { first_def: Ident, second_def: Ident },
#[error("duplicate {typ} found in namespace")]
Duplicate { typ: String, first_def: Ident, second_def: Ident },
yordanmadzhunkov marked this conversation as resolved.
Show resolved Hide resolved
#[error("unresolved import")]
UnresolvedModuleDecl { mod_name: Ident },
#[error("path resolution error")]
Expand All @@ -35,57 +29,24 @@ impl DefCollectorErrorKind {
impl From<DefCollectorErrorKind> for Diagnostic {
fn from(error: DefCollectorErrorKind) -> Diagnostic {
match error {
DefCollectorErrorKind::DuplicateFunction { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let func_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("duplicate definitions of {func_name} function found"),
"first definition found here".to_string(),
first_span,
);
diag.add_secondary("second definition found here".to_string(), second_span);
diag
}
DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let mod_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("module {mod_name} has been declared twice"),
"first declaration found here".to_string(),
first_span,
);
diag.add_secondary("second declaration found here".to_string(), second_span);
diag
}
DefCollectorErrorKind::DuplicateImport { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let import_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("the name `{import_name}` is defined multiple times"),
"first import found here".to_string(),
first_span,
);
diag.add_secondary("second import found here".to_string(), second_span);
diag
}
DefCollectorErrorKind::DuplicateGlobal { first_def, second_def } => {
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let import_name = &first_def.0.contents;

let mut diag = Diagnostic::simple_error(
format!("the name `{import_name}` is defined multiple times"),
"first global declaration found here".to_string(),
first_span,
);
diag.add_secondary("second global declaration found here".to_string(), second_span);
diag
DefCollectorErrorKind::Duplicate { typ, first_def, second_def } => {
let primary_message =
format!("duplicate definitions of {} with name {} found", &typ, &first_def.0.contents);
{
let duplicate_type: &str = &typ;
let first_span = first_def.0.span();
let second_span = second_def.0.span();
let mut diag = Diagnostic::simple_error(
primary_message,
format!("first {} found here", duplicate_type),
first_span,
);
diag.add_secondary(
format!("second {} found here", duplicate_type),
second_span,
);
diag
}
}
DefCollectorErrorKind::UnresolvedModuleDecl { mod_name } => {
let span = mod_name.0.span();
Expand Down