Recursion

[KrosFire]

No description provided.

[Copilot]

Pull request overview

This PR adds support for recursive functions to the Amber language analyzer (alpha050 version). The key change is refactoring the function analysis flow to register function symbols in the symbol table before analyzing the function body, which allows functions to reference themselves recursively.

Key Changes:

• Moved function symbol registration to occur before body analysis in src/analysis/alpha050/global.rs
• Added type compatibility rule allowing Int to match Number type expectations
• Added 6 new test files demonstrating various recursion patterns (factorial, fibonacci, ackermann, array sum, mutual recursion, post-order)

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/analysis/alpha050/global.rs	Refactored to insert function symbol into symbol table before analyzing function body, enabling recursive calls
src/analysis/types.rs	Added type matching rule for Int as subtype of Number
tests/analysis/alpha050.rs	Added test case for recursion with Ackermann function
tests/analysis/snapshots/r#mod__analysis__alpha050__recursion_ackermann.snap.new	Snapshot of expected symbol table output for recursion test
tests/analysis/recursion_ackermann.ab	Test file for Ackermann function (doubly-recursive)
tests/analysis/recursion_factorial.ab	Test file for factorial function (simple recursion)
tests/analysis/recursion_fibonacci.ab	Test file for Fibonacci function (double recursion)
tests/analysis/recursion_array.ab	Test file for recursive array sum
tests/analysis/recursion_mutual.ab	Test file for mutually recursive functions
tests/analysis/recursion_post_order.ab	Test file for post-order recursion with side effects

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Using 0..=usize::MAX as the definition scope allows the function to be referenced from anywhere in the file, including before its definition. This enables recursion but also permits forward references that may be unintended.

Consider using name_span.start..=usize::MAX instead, which would make the function visible from the point of its declaration forward (including within its own body for recursion) while preventing forward references from earlier in the file. This would be more consistent with typical scoping rules.

Suggested change

	0..=usize::MAX,
	name_span.start..=usize::MAX,

[Copilot]

Potential index out of bounds error when accessing new_generic_types[idx]. The variable idx represents the position in the args array, but new_generic_types only contains entries for generic and optional arguments without explicit types.

For example, with fun foo(a: Num, b), args has 2 elements but new_generic_types has only 1. When processing argument at index 0 (a: Num), the code would try to access new_generic_types[0] in the Typed branch (line 236), which doesn't make sense since typed arguments don't add to new_generic_types. When processing argument at index 1 (b), it would try new_generic_types[1] which is out of bounds.

The old code used new_generic_types.remove(0) which correctly dequeued generics in order. Consider maintaining a separate counter for generic indices or reverting to the dequeue approach.

Suggested change

	arguments: args
	.iter()
	.enumerate()
	.filter_map(|(idx, (arg, span))| match arg {
	FunctionArgument::Generic((is_ref, _), (name, _)) => {
	Some((
	analysis::FunctionArgument {
	name: name.clone(),
	data_type: DataType::Generic(
	new_generic_types[idx],
	),
	is_optional: false,
	is_ref: *is_ref,
	},
	*span,
	))
	}
	FunctionArgument::Typed(
	(is_ref, _),
	(name, _),
	(ty, _),
	) => Some((
	analysis::FunctionArgument {
	name: name.clone(),
	data_type: ty.clone(),
	is_optional: false,
	is_ref: *is_ref,
	},
	*span,
	)),
	FunctionArgument::Optional(
	(is_ref, _),
	(name, _),
	ty,
	_,
	) => Some((
	analysis::FunctionArgument {
	name: name.clone(),
	data_type: match ty {
	Some((ty, _)) => ty.clone(),
	None => {
	DataType::Generic(new_generic_types[idx])
	}
	},
	is_optional: true,
	is_ref: *is_ref,
	},
	*span,
	)),
	FunctionArgument::Error => None,
	})
	.collect::<Vec<_>>(),
	arguments: {
	let mut new_generic_types = new_generic_types.clone();
	args
	.iter()
	.enumerate()
	.filter_map(|(_idx, (arg, span))| match arg {
	FunctionArgument::Generic((is_ref, _), (name, _)) => {
	Some((
	analysis::FunctionArgument {
	name: name.clone(),
	data_type: DataType::Generic(
	new_generic_types.remove(0),
	),
	is_optional: false,
	is_ref: *is_ref,
	},
	*span,
	))
	}
	FunctionArgument::Typed(
	(is_ref, _),
	(name, _),
	(ty, _),
	) => Some((
	analysis::FunctionArgument {
	name: name.clone(),
	data_type: ty.clone(),
	is_optional: false,
	is_ref: *is_ref,
	},
	*span,
	)),
	FunctionArgument::Optional(
	(is_ref, _),
	(name, _),
	ty,
	_,
	) => Some((
	analysis::FunctionArgument {
	name: name.clone(),
	data_type: match ty {
	Some((ty, _)) => ty.clone(),
	None => {
	DataType::Generic(new_generic_types.remove(0))
	}
	},
	is_optional: true,
	is_ref: *is_ref,
	},
	*span,
	)),
	FunctionArgument::Error => None,
	})
	.collect::<Vec<_>>()
	},

[Copilot]

The comment "When both operands are unknown generics, they should get the broad constraint" appears to be copy-pasted from another test and is not relevant to this recursion test. The ackermann function uses typed parameters (m: Num, n: Num) and doesn't involve generic constraints. Consider updating the comment to describe what this test is actually verifying (e.g., "Test that recursive function calls are properly analyzed and symbol table is correctly populated").

Suggested change

	// When both operands are unknown generics, they should get the broad constraint
	// Test that recursive function calls are properly analyzed and symbol table is correctly populated