Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 23 additions & 12 deletions core/type-checker/src/symbol_table.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
//! Symbol Table
//!
//! This module implements a tree-based symbol table for managing scopes and symbols
//! during type checking. It supports:
//!
//! Tree-based scope management for type checking:
//! - Hierarchical scopes with parent-child relationships
//! - Type alias, struct, enum, spec, and function symbol registration
//! - Variable tracking within scopes
//! - Method resolution on types
//! - Import registration and resolution
//! - Type/struct/enum/spec and function registration
//! - Variable, method, and import tracking
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did method resolution types and import registration comments removed?

//! - Visibility checking for access control
//!
//! Scopes form a tree structure where each scope can have multiple child scopes.
//! Symbol lookup walks up the tree from current scope to root until a match is found.
//! ## Scope Tree
//!
//! Symbol lookup walks **up** the parent chain from current scope to root.
//! Inner scopes can shadow outer scope symbols. Scope navigation:
//! - `push_scope()` - Create child scope and move down
//! - `pop_scope()` - Return to parent scope
//!
//! ## Default Return Types
//!
//! Functions without an explicit return type default to the unit type. This is
//! represented using `Type::Simple(SimpleTypeKind::Unit)`, which provides a
//! lightweight value-based representation without heap allocation.
//! Functions without explicit return type default to unit (`Type::Simple(SimpleTypeKind::Unit)`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment is less informative than the original one



use std::cell::RefCell;
use std::rc::Rc;
Expand Down Expand Up @@ -311,6 +310,7 @@ impl Scope {
if let Some(symbol) = self.lookup_symbol_local(name) {
return Some(symbol.clone());
}
// Recursively search parent scopes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is necessary

if let Some(parent) = &self.parent {
return parent.borrow().lookup_symbol(name);
}
Expand Down Expand Up @@ -340,6 +340,7 @@ impl Scope {
if let Some((_, ty)) = self.lookup_variable_local(name) {
return Some(ty);
}
// Search parent scopes (enables shadowing)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, shadowing should not be allowed because it leads to potential errors. Can you please add a test to confirm that shadowing takes place, indeed?

if let Some(parent) = &self.parent {
return parent.borrow().lookup_variable(name);
}
Expand All @@ -362,6 +363,7 @@ impl Scope {
{
return Some(method_info.clone());
}
// Search parent scopes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment is necessary

if let Some(parent) = &self.parent {
return parent.borrow().lookup_method(type_name, method_name);
}
Expand Down Expand Up @@ -455,6 +457,11 @@ impl SymbolTable {
self.push_scope_with_name(&name, Visibility::Private)
}

/// Create a new child scope and move down.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this analogy of push/pop operations to movement can be misleading, so better not to use it.

///
/// Links the new scope to the current scope as its parent, builds the full path
/// (e.g., "module::inner"), and updates `current_scope`. Used when entering function
/// bodies, blocks, or nested modules.
pub(crate) fn push_scope_with_name(&mut self, name: &str, visibility: Visibility) -> u32 {
let parent = self.current_scope.clone();
let scope_id = self.next_scope_id;
Expand Down Expand Up @@ -483,6 +490,10 @@ impl SymbolTable {
scope_id
}

/// Pop the current scope and move back to its parent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to describe what this function literally does, like: reassigning the current_scope to the parent if it exists

///
/// Counterpart to `push_scope()` / `push_scope_with_name()`. Called when exiting
/// a scope (function end, block end) to restore the parent scope as `current_scope`.
pub(crate) fn pop_scope(&mut self) {
if let Some(current) = &self.current_scope {
let parent = current.borrow().parent.clone();
Expand Down
65 changes: 39 additions & 26 deletions core/type-checker/src/type_checker.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
//! Type Checker Implementation
//!
//! This module contains the core type checking logic that infers and validates
//! types throughout the AST. The type checker operates in multiple phases:
//!
//! 1. **process_directives** - Register raw imports from use statements
//! Multi-phase type checking with forward reference support:
//! 1. **process_directives** - Register imports from use statements
//! 2. **register_types** - Collect type/struct/enum/spec definitions
//! 3. **resolve_imports** - Bind import paths to symbols
//! 4. **collect_function_and_constant_definitions** - Register functions
//! 4. **collect_function_and_constant_definitions** - Register function signatures
//! 5. **infer_variables** - Type-check function bodies
//!
//! The type checker continues after encountering errors to collect all issues
//! before returning. Errors are deduplicated to avoid repeated reports.
//! Generic type parameters are tracked through phases 4-5. When a generic function is called,
//! `infer_type_params_from_args()` infers concrete type substitutions from arguments.
//! Type mismatches (conflicting inference, unresolvable parameters) are reported.
//!
//! Errors are collected and deduplicated to report all issues in a single pass.


use std::rc::Rc;

Expand Down Expand Up @@ -60,14 +62,15 @@ impl TypeChecker {
}

impl TypeChecker {
/// Infer types for all definitions in the context.
/// Infer types for all definitions.
///
/// Executes the 5-phase algorithm in order. Phase ordering is critical: types must be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original comment is clearer. The # Errors section can mislead in this form because such a format of comments is about what errors a function can throw.

/// registered before functions can reference them, and imports must be resolved before
/// they're used. Continues on errors to collect all issues.
///
/// Phase ordering:
/// 1. `process_directives()` - Register raw imports in scopes
/// 2. `register_types()` - Collect type definitions into symbol table
/// 3. `resolve_imports()` - Bind import paths to symbols
/// 4. `collect_function_and_constant_definitions()` - Register functions
/// 5. Infer variable types in function bodies
/// # Errors
///
/// Returns error with all accumulated type errors if any occurred.
pub fn infer_types(&mut self, ctx: &mut TypedContext) -> anyhow::Result<SymbolTable> {
self.process_directives(ctx);
self.register_types(ctx);
Expand Down Expand Up @@ -475,14 +478,21 @@ impl TypeChecker {
}

#[allow(clippy::needless_pass_by_value)]
/// Infer types for function parameters and body.
///
/// Creates a new scope for the function, collects type parameter names, registers
/// parameters as variables, and type-checks all statements. Generic type parameters
/// (e.g., `T` in `fn foo<T>(x: T)`) are preserved via `TypeInfo::new_with_type_params()`
/// for later substitution when the function is called.
fn infer_variables(
&mut self,
function_definition: Rc<FunctionDefinition>,
ctx: &mut TypedContext,
) {
self.symbol_table.push_scope();

// Collect type parameter names for proper TypeInfo construction
// Collect type parameter names for proper TypeInfo construction.
// These names identify which type references are generic variables vs. concrete types.
let type_param_names: Vec<String> = function_definition
.type_parameters
.as_ref()
Expand All @@ -493,6 +503,9 @@ impl TypeChecker {
for argument in arguments {
match argument {
ArgumentType::Argument(arg) => {
// Create TypeInfo with awareness of generic type parameters.
// Non-generic arguments get concrete types, while references to
// type parameters (e.g., 'T' in 'fn foo<T>(x: T)') become Generic kinds.
let arg_type = TypeInfo::new_with_type_params(&arg.ty, &type_param_names);
if let Err(err) = self
.symbol_table
Expand All @@ -517,7 +530,9 @@ impl TypeChecker {
}
}

// Build return type with type parameter awareness
// Build return type with type parameter awareness.
// The return type is needed for statement type checking to validate return statements.
// Generic type parameters in the return type will be resolved when the function is called.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what it means. Can you please give an example?

let return_type = function_definition
.returns
.as_ref()
Expand Down Expand Up @@ -2011,12 +2026,12 @@ impl TypeChecker {
}
}

/// Attempt to infer type parameters from argument types.
///
/// For each parameter that is a type variable (Generic), try to find a
/// concrete type from the corresponding argument.
/// Infer concrete type substitutions for generic parameters from arguments.
///
/// Returns a substitution map if inference succeeds, empty map otherwise.
/// For each generic parameter in the signature, examines corresponding arguments to infer
/// concrete types. Detects conflicting inference (e.g., `fn id<T>(a: T, b: T)` called
/// with different types for a and b) and missing parameters (T appears in signature
/// but no argument provides it).
#[allow(clippy::type_complexity)]
fn infer_type_params_from_args(
&mut self,
Expand All @@ -2032,19 +2047,17 @@ impl TypeChecker {
None => return substitutions,
};

// For each parameter, check if it contains a type variable
// Infer type from each argument if its parameter is generic
for (i, param_type) in signature.param_types.iter().enumerate() {
if i >= args.len() {
break;
}

// If the parameter type is a type variable, infer from argument
if let TypeInfoKind::Generic(type_param_name) = &param_type.kind {
// Infer the argument type
let arg_type = self.infer_expression(&args[i].1.borrow(), ctx);

if let Some(arg_type) = arg_type {
// Check for conflicting inference
// Check for conflicting inference across arguments
if let Some(existing) = substitutions.get(type_param_name) {
if *existing != arg_type {
self.errors.push(TypeCheckError::ConflictingTypeInference {
Expand All @@ -2061,7 +2074,7 @@ impl TypeChecker {
}
}

// Check if we found substitutions for all type parameters
// Verify all type parameters were inferred
for type_param in &signature.type_params {
if !substitutions.contains_key(type_param) {
self.errors.push(TypeCheckError::CannotInferTypeParameter {
Expand Down