Skip to content

Migrate error deduplication to node-ID based approach, fixes #71 #108

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

Open
Arowolokehinde wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Migrate error deduplication to node-ID based approach, fixes #71 #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 24 additions & 0 deletions core/type-checker/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ impl Display for VisibilityContext {
}
}

/// Categorizes errors that require deduplication.
/// Only covers the 5 error types that can have duplicate reports.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) enum ErrorKind {
UnknownType,
UndefinedFunction,
UnknownIdentifier,
UndefinedStruct,
UndefinedEnum,
}

/// Represents a type checking error with source location.
/// All type errors are tied to AST nodes and must have a location.
#[derive(Debug, Clone, Error)]
Expand Down Expand Up @@ -377,6 +388,19 @@ impl TypeCheckError {
| TypeCheckError::AssociatedFunctionCalledAsMethod { location, .. } => location,
}
}

/// Returns the ErrorKind for deduplicated error types.
/// Returns None for errors that don't need deduplication.
pub(crate) fn kind(&self) -> Option<ErrorKind> {
match self {
TypeCheckError::UnknownType { .. } => Some(ErrorKind::UnknownType),
TypeCheckError::UndefinedFunction { .. } => Some(ErrorKind::UndefinedFunction),
TypeCheckError::UnknownIdentifier { .. } => Some(ErrorKind::UnknownIdentifier),
TypeCheckError::UndefinedStruct { .. } => Some(ErrorKind::UndefinedStruct),
TypeCheckError::UndefinedEnum { .. } => Some(ErrorKind::UndefinedEnum),
_ => None,
}
}
}

#[cfg(test)]
Expand Down
108 changes: 61 additions & 47 deletions core/type-checker/src/type_checker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use inference_ast::nodes::{
use rustc_hash::{FxHashMap, FxHashSet};

use crate::{
errors::{RegistrationKind, TypeCheckError, TypeMismatchContext, VisibilityContext},
errors::{ErrorKind, RegistrationKind, TypeCheckError, TypeMismatchContext, VisibilityContext},
symbol_table::{FuncInfo, Import, ImportItem, ImportKind, ResolvedImport, SymbolTable},
type_info::{NumberType, TypeInfo, TypeInfoKind},
typed_context::TypedContext,
Expand All @@ -35,7 +35,7 @@ pub(crate) struct TypeChecker {
symbol_table: SymbolTable,
errors: Vec<TypeCheckError>,
glob_resolution_in_progress: FxHashSet<u32>,
reported_error_keys: FxHashSet<String>,
reported_errors: FxHashSet<(ErrorKind, u32)>,
}

impl TypeChecker {
Expand Down Expand Up @@ -440,21 +440,27 @@ impl TypeChecker {
.lookup_type(&generic_type.base.name())
.is_none()
{
self.push_error_dedup(TypeCheckError::UnknownType {
name: generic_type.base.name(),
location: generic_type.base.location,
});
self.push_error_dedup(
TypeCheckError::UnknownType {
name: generic_type.base.name(),
location: generic_type.base.location,
},
generic_type.base.id,
);
}
// Validate each parameter in the generic type
for param in &generic_type.parameters {
// Check if it's a declared type parameter or a known type
if !type_param_names.contains(&param.name())
&& self.symbol_table.lookup_type(&param.name()).is_none()
{
self.push_error_dedup(TypeCheckError::UnknownType {
name: param.name(),
location: param.location,
});
self.push_error_dedup(
TypeCheckError::UnknownType {
name: param.name(),
location: param.location,
},
param.id,
);
}
}
}
Expand All @@ -465,10 +471,13 @@ impl TypeChecker {
return;
}
if self.symbol_table.lookup_type(&identifier.name).is_none() {
self.push_error_dedup(TypeCheckError::UnknownType {
name: identifier.name.clone(),
location: identifier.location,
});
self.push_error_dedup(
TypeCheckError::UnknownType {
name: identifier.name.clone(),
location: identifier.location,
},
identifier.id,
);
}
}
}
Expand Down Expand Up @@ -960,10 +969,13 @@ impl TypeChecker {
None
}
} else {
self.push_error_dedup(TypeCheckError::UndefinedEnum {
name: enum_name,
location: type_member_access_expression.location,
});
self.push_error_dedup(
TypeCheckError::UndefinedEnum {
name: enum_name,
location: type_member_access_expression.location,
},
type_member_access_expression.id,
);
None
}
}
Expand Down Expand Up @@ -1200,10 +1212,13 @@ impl TypeChecker {
);
s.clone()
} else {
self.push_error_dedup(TypeCheckError::UndefinedFunction {
name: function_call_expression.name(),
location: function_call_expression.location,
});
self.push_error_dedup(
TypeCheckError::UndefinedFunction {
name: function_call_expression.name(),
location: function_call_expression.location,
},
function_call_expression.id,
);
if let Some(arguments) = &function_call_expression.arguments {
for arg in arguments {
self.infer_expression(&arg.1.borrow(), ctx);
Expand Down Expand Up @@ -1302,10 +1317,13 @@ impl TypeChecker {
ctx.set_node_typeinfo(struct_expression.id, struct_type.clone());
return Some(struct_type);
}
self.push_error_dedup(TypeCheckError::UndefinedStruct {
name: struct_expression.name(),
location: struct_expression.location,
});
self.push_error_dedup(
TypeCheckError::UndefinedStruct {
name: struct_expression.name(),
location: struct_expression.location,
},
struct_expression.id,
);
None
}
Expression::PrefixUnary(prefix_unary_expression) => {
Expand Down Expand Up @@ -1522,10 +1540,13 @@ impl TypeChecker {
ctx.set_node_typeinfo(identifier.id, var_ty.clone());
Some(var_ty)
} else {
self.push_error_dedup(TypeCheckError::UnknownIdentifier {
name: identifier.name.clone(),
location: identifier.location,
});
self.push_error_dedup(
TypeCheckError::UnknownIdentifier {
name: identifier.name.clone(),
location: identifier.location,
},
identifier.id,
);
None
}
}
Expand Down Expand Up @@ -2075,26 +2096,19 @@ impl TypeChecker {
substitutions
}

/// Push an error, deduplicating errors for the same unknown type/function/identifier.
/// Push an error, deduplicating errors for the same AST node.
/// This prevents duplicate errors when registration fails but inference continues.
fn push_error_dedup(&mut self, error: TypeCheckError) {
let key = match &error {
TypeCheckError::UnknownType { name, .. } => Some(format!("UnknownType:{name}")),
TypeCheckError::UndefinedFunction { name, .. } => {
Some(format!("UndefinedFunction:{name}"))
}
TypeCheckError::UnknownIdentifier { name, .. } => {
Some(format!("UnknownIdentifier:{name}"))
}
TypeCheckError::UndefinedStruct { name, .. } => Some(format!("UndefinedStruct:{name}")),
TypeCheckError::UndefinedEnum { name, .. } => Some(format!("UndefinedEnum:{name}")),
_ => None,
};
if let Some(key) = key {
if self.reported_error_keys.contains(&key) {
///
/// # Arguments
/// * `error` - The error to report
/// * `node_id` - The AST node ID where the error occurred
fn push_error_dedup(&mut self, error: TypeCheckError, node_id: u32) {
if let Some(kind) = error.kind() {
let key = (kind, node_id);
if self.reported_errors.contains(&key) {
return;
}
self.reported_error_keys.insert(key);
self.reported_errors.insert(key);
}
self.errors.push(error);
}
Expand Down
12 changes: 6 additions & 6 deletions tests/src/type_checker/error_recovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,8 @@ mod error_recovery_tests {
let error_msg = error.to_string();
let count = error_msg.matches("unknown type `UnknownType`").count();
assert_eq!(
count, 1,
"UnknownType error should appear exactly once due to deduplication, but appeared {} times in: {}",
count, 4,
"UnknownType error should appear 4 times (once per usage at different AST nodes), but appeared {} times in: {}",
count, error_msg
);
}
Expand All @@ -461,8 +461,8 @@ mod error_recovery_tests {
.matches("undefined function `missing_func`")
.count();
assert_eq!(
count, 1,
"missing_func error should appear exactly once due to deduplication, but appeared {} times in: {}",
count, 2,
"missing_func error should appear 2 times (once per call at different AST nodes), but appeared {} times in: {}",
count, error_msg
);
}
Expand All @@ -482,8 +482,8 @@ mod error_recovery_tests {
.matches("undeclared variable `unknown_var`")
.count();
assert_eq!(
count, 1,
"unknown_var error should appear exactly once due to deduplication, but appeared {} times in: {}",
count, 3,
"unknown_var error should appear 3 times (once per usage at different AST nodes), but appeared {} times in: {}",
count, error_msg
);
}
Expand Down