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 all 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
15 changes: 11 additions & 4 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
@@ -1,5 +1,5 @@
use super::dc_mod::collect_defs;
use super::errors::DefCollectorErrorKind;
use super::errors::{DefCollectorErrorKind, DuplicateType};
use crate::graph::CrateId;
use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
use crate::hir::resolution::errors::ResolverError;
Expand Down 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: DuplicateType::Import,
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: DuplicateType::Function,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(unresolved.file_id));
}
}
Expand Down
32 changes: 26 additions & 6 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 @@ -9,7 +9,7 @@ use crate::{

use super::{
dc_crate::{DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTypeAlias},
errors::DefCollectorErrorKind,
errors::{DefCollectorErrorKind, DuplicateType},
};
use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleId, ModuleOrigin};
use crate::hir::resolution::import::ImportDirective;
Expand Down 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: DuplicateType::Global,
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: DuplicateType::Function,
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: DuplicateType::TypeDefinition,
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: DuplicateType::Function,
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: DuplicateType::Module,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(self.file_id));
return None;
}
Expand Down
97 changes: 39 additions & 58 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 @@ -6,16 +6,20 @@ use noirc_errors::FileDiagnostic;
use noirc_errors::Span;
use thiserror::Error;

use std::fmt;

pub enum DuplicateType {
Function,
Module,
Global,
TypeDefinition,
Import,
}

#[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: DuplicateType, first_def: Ident, second_def: Ident },
#[error("unresolved import")]
UnresolvedModuleDecl { mod_name: Ident },
#[error("path resolution error")]
Expand All @@ -32,60 +36,37 @@ impl DefCollectorErrorKind {
}
}

impl fmt::Debug for DuplicateType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
DuplicateType::Function => write!(f, "function"),
DuplicateType::Module => write!(f, "module"),
DuplicateType::Global => write!(f, "global"),
DuplicateType::TypeDefinition => write!(f, "type definition"),
DuplicateType::Import => write!(f, "import"),
}
}
}

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,
DefCollectorErrorKind::Duplicate { typ, first_def, second_def } => {
let primary_message = format!(
"duplicate definitions of {:?} with name {} found",
&typ, &first_def.0.contents
);
diag.add_secondary("second global declaration found here".to_string(), second_span);
diag
{
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", &typ),
first_span,
);
diag.add_secondary(format!("second {:?} found here", &typ), second_span);
diag
}
}
DefCollectorErrorKind::UnresolvedModuleDecl { mod_name } => {
let span = mod_name.0.span();
Expand Down