fix: resolve all stress test bugs
- Record equality: add Record case to values_equal in interpreter - Invalid escapes: error on unknown escape sequences in lexer - Unknown effects: validate effect names in check_function with suggestions - Circular types: add DFS cycle detection in check_type_cycles - Parser: require | for enum variants, enabling proper type alias syntax All 265 tests pass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,48 +2,43 @@
|
||||
|
||||
This document summarizes issues found during systematic stress testing of the Lux parser, typechecker, and REPL.
|
||||
|
||||
## Critical Bugs
|
||||
## Critical Bugs (All Fixed)
|
||||
|
||||
### 1. Record Equality Returns False for Equal Records
|
||||
### 1. ~~Record Equality Returns False for Equal Records~~ (FIXED)
|
||||
```lux
|
||||
let r1 = { x: 1, y: 2 }
|
||||
let r2 = { x: 1, y: 2 }
|
||||
r1 == r2 // Returns false! Should be true
|
||||
r1 == r2 // Now returns true
|
||||
```
|
||||
**Impact**: High - breaks expected semantics for record comparison
|
||||
**Location**: Likely in interpreter's equality logic for `Value::Record`
|
||||
**Fix**: Added Record case to `Interpreter::values_equal` in `src/interpreter.rs`
|
||||
|
||||
### 2. Invalid Escape Sequences Silently Accepted
|
||||
### 2. ~~Invalid Escape Sequences Silently Accepted~~ (FIXED)
|
||||
```lux
|
||||
"\z\q\w" // Returns "zqw" - backslashes silently dropped
|
||||
"\x41" // Returns "x41" - hex escapes not supported but no error
|
||||
"\u0041" // Returns "u0041" - unicode escapes not supported but no error
|
||||
"\z" // Now produces error: "Invalid escape sequence: \z"
|
||||
```
|
||||
**Impact**: High - users expect invalid escapes to error
|
||||
**Fix**: Parser should reject unknown escape sequences
|
||||
**Fix**: Modified lexer in `src/lexer.rs` to error on unknown escape sequences
|
||||
|
||||
### 3. Unknown Effects Silently Accepted in Declarations
|
||||
### 3. ~~Unknown Effects Silently Accepted in Declarations~~ (FIXED)
|
||||
```lux
|
||||
fn test(): Unit with {CompletelyFakeEffect} = () // No error!
|
||||
fn test(): Unit with {CompletelyFakeEffect} = () // Now produces error with suggestions
|
||||
```
|
||||
**Impact**: Medium - typos in effect names not caught until call site
|
||||
**Fix**: Validate effect names during function declaration checking
|
||||
**Fix**: Added effect validation in `TypeChecker::check_function` in `src/typechecker.rs`
|
||||
|
||||
### 4. No Forward References for Mutual Recursion
|
||||
### 4. ~~No Forward References for Mutual Recursion~~ (FIXED/DOCUMENTED)
|
||||
```lux
|
||||
fn isEven(n: Int): Bool = if n == 0 then true else isOdd(n - 1) // Error: isOdd undefined
|
||||
fn isEven(n: Int): Bool = if n == 0 then true else isOdd(n - 1)
|
||||
fn isOdd(n: Int): Bool = if n == 0 then false else isEven(n - 1)
|
||||
// Works in files (two-pass type checking), REPL limitation documented
|
||||
```
|
||||
**Impact**: Medium - common pattern not supported
|
||||
**Fix**: Two-pass type checking or explicit forward declarations
|
||||
**Note**: Works when loaded from files due to two-pass type checking. REPL processes line-by-line, so forward references don't work there.
|
||||
|
||||
### 5. Circular Type Definitions Silently Fail
|
||||
### 5. ~~Circular Type Definitions Silently Fail~~ (FIXED)
|
||||
```lux
|
||||
type A = B
|
||||
type B = A
|
||||
// No output, no error
|
||||
// Now produces error: "Circular type definition detected: A -> B -> A"
|
||||
```
|
||||
**Impact**: Medium - should produce clear error about circular definition
|
||||
**Fix**: Added `check_type_cycles` method with DFS cycle detection in `src/typechecker.rs`. Also fixed parser to require `|` for enum variants, allowing `type A = B` to be parsed as a type alias.
|
||||
|
||||
## Parser Issues
|
||||
|
||||
|
||||
@@ -2810,6 +2810,11 @@ impl Interpreter {
|
||||
(Value::Tuple(a), Value::Tuple(b)) => {
|
||||
a.len() == b.len() && a.iter().zip(b).all(|(x, y)| self.values_equal(x, y))
|
||||
}
|
||||
(Value::Record(a), Value::Record(b)) => {
|
||||
a.len() == b.len() && a.iter().all(|(k, v)| {
|
||||
b.get(k).map(|bv| self.values_equal(v, bv)).unwrap_or(false)
|
||||
})
|
||||
}
|
||||
(
|
||||
Value::Constructor {
|
||||
name: n1,
|
||||
@@ -2824,6 +2829,7 @@ impl Interpreter {
|
||||
&& f1.len() == f2.len()
|
||||
&& f1.iter().zip(f2).all(|(x, y)| self.values_equal(x, y))
|
||||
}
|
||||
(Value::Json(a), Value::Json(b)) => a == b,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
10
src/lexer.rs
10
src/lexer.rs
@@ -484,13 +484,21 @@ impl<'a> Lexer<'a> {
|
||||
current_literal.push('}');
|
||||
}
|
||||
_ => {
|
||||
let escape_start = self.pos;
|
||||
let escaped = match self.advance() {
|
||||
Some('n') => '\n',
|
||||
Some('r') => '\r',
|
||||
Some('t') => '\t',
|
||||
Some('\\') => '\\',
|
||||
Some('"') => '"',
|
||||
Some(c) => c,
|
||||
Some('0') => '\0',
|
||||
Some('\'') => '\'',
|
||||
Some(c) => {
|
||||
return Err(LexError {
|
||||
message: format!("Invalid escape sequence: \\{}", c),
|
||||
span: Span::new(escape_start - 1, self.pos),
|
||||
});
|
||||
}
|
||||
None => {
|
||||
return Err(LexError {
|
||||
message: "Unterminated string".into(),
|
||||
|
||||
41
src/main.rs
41
src/main.rs
@@ -1813,6 +1813,46 @@ c")"#;
|
||||
assert_eq!(eval(r#"let x = typeOf("hello")"#).unwrap(), r#""String""#);
|
||||
}
|
||||
|
||||
// Bug fix tests
|
||||
#[test]
|
||||
fn test_record_equality() {
|
||||
assert_eq!(eval("let x = { a: 1, b: 2 } == { a: 1, b: 2 }").unwrap(), "true");
|
||||
assert_eq!(eval("let x = { a: 1 } == { a: 2 }").unwrap(), "false");
|
||||
assert_eq!(eval("let x = { a: 1, b: 2 } == { a: 1, b: 3 }").unwrap(), "false");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_invalid_escape_sequence() {
|
||||
let result = eval(r#"let x = "\z""#);
|
||||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().contains("Invalid escape sequence"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_unknown_effect_error() {
|
||||
let result = eval("fn test(): Unit with {FakeEffect} = ()");
|
||||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().contains("Unknown effect"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_circular_type_definitions() {
|
||||
let result = eval("type A = B\ntype B = A");
|
||||
assert!(result.is_err(), "Should detect circular type definitions");
|
||||
let err = result.unwrap_err();
|
||||
assert!(err.contains("Circular"), "Error should mention 'Circular': {}", err);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mutual_recursion() {
|
||||
let source = r#"
|
||||
fn isEven(n: Int): Bool = if n == 0 then true else isOdd(n - 1)
|
||||
fn isOdd(n: Int): Bool = if n == 0 then false else isEven(n - 1)
|
||||
let result = isEven(10)
|
||||
"#;
|
||||
assert_eq!(eval(source).unwrap(), "true");
|
||||
}
|
||||
|
||||
// Pipe with stdlib tests
|
||||
#[test]
|
||||
fn test_pipe_with_list() {
|
||||
@@ -2948,7 +2988,6 @@ c")"#;
|
||||
mod example_tests {
|
||||
use super::*;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
|
||||
fn check_file(path: &str) -> Result<(), String> {
|
||||
let source = fs::read_to_string(path)
|
||||
|
||||
@@ -478,11 +478,11 @@ impl Parser {
|
||||
self.advance();
|
||||
self.skip_newlines();
|
||||
|
||||
if self.check(TokenKind::Pipe) || self.peek_is_variant() {
|
||||
// Enum type
|
||||
if self.check(TokenKind::Pipe) {
|
||||
// Enum type - requires leading | for variants
|
||||
(TypeDef::Enum(self.parse_variants()?), Vec::new())
|
||||
} else {
|
||||
// Type alias
|
||||
// Type alias - any type expression
|
||||
(TypeDef::Alias(self.parse_type()?), Vec::new())
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -532,6 +532,9 @@ impl TypeChecker {
|
||||
self.collect_declaration(decl);
|
||||
}
|
||||
|
||||
// Check for circular type definitions
|
||||
self.check_type_cycles(program);
|
||||
|
||||
// Second pass: type check all declarations
|
||||
for decl in &program.declarations {
|
||||
self.check_declaration(decl);
|
||||
@@ -544,6 +547,130 @@ impl TypeChecker {
|
||||
}
|
||||
}
|
||||
|
||||
/// Check for circular type alias definitions
|
||||
fn check_type_cycles(&mut self, program: &Program) {
|
||||
use std::collections::HashSet;
|
||||
|
||||
// Build a map of type alias dependencies
|
||||
let mut alias_deps: HashMap<String, Vec<String>> = HashMap::new();
|
||||
|
||||
for decl in &program.declarations {
|
||||
if let Declaration::Type(type_decl) = decl {
|
||||
if let ast::TypeDef::Alias(type_expr) = &type_decl.definition {
|
||||
let deps = self.collect_type_references(type_expr);
|
||||
alias_deps.insert(type_decl.name.name.clone(), deps);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check for cycles using DFS
|
||||
fn has_cycle(
|
||||
name: &str,
|
||||
alias_deps: &HashMap<String, Vec<String>>,
|
||||
visiting: &mut HashSet<String>,
|
||||
visited: &mut HashSet<String>,
|
||||
cycle_path: &mut Vec<String>,
|
||||
) -> bool {
|
||||
if visiting.contains(name) {
|
||||
cycle_path.push(name.to_string());
|
||||
return true;
|
||||
}
|
||||
if visited.contains(name) {
|
||||
return false;
|
||||
}
|
||||
|
||||
visiting.insert(name.to_string());
|
||||
cycle_path.push(name.to_string());
|
||||
|
||||
if let Some(deps) = alias_deps.get(name) {
|
||||
for dep in deps {
|
||||
if alias_deps.contains_key(dep) {
|
||||
if has_cycle(dep, alias_deps, visiting, visited, cycle_path) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
visiting.remove(name);
|
||||
cycle_path.pop();
|
||||
visited.insert(name.to_string());
|
||||
false
|
||||
}
|
||||
|
||||
let mut visited = HashSet::new();
|
||||
|
||||
for type_name in alias_deps.keys() {
|
||||
let mut visiting = HashSet::new();
|
||||
let mut cycle_path = Vec::new();
|
||||
|
||||
if has_cycle(type_name, &alias_deps, &mut visiting, &mut visited, &mut cycle_path) {
|
||||
// Find the span for this type declaration
|
||||
let span = program.declarations.iter().find_map(|d| {
|
||||
if let Declaration::Type(t) = d {
|
||||
if t.name.name == *type_name {
|
||||
return Some(t.span);
|
||||
}
|
||||
}
|
||||
None
|
||||
}).unwrap_or(Span::default());
|
||||
|
||||
self.errors.push(TypeError {
|
||||
message: format!(
|
||||
"Circular type definition detected: {}",
|
||||
cycle_path.join(" -> ")
|
||||
),
|
||||
span,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Collect all type names referenced in a type expression
|
||||
fn collect_type_references(&self, type_expr: &TypeExpr) -> Vec<String> {
|
||||
let mut refs = Vec::new();
|
||||
self.collect_type_refs_helper(type_expr, &mut refs);
|
||||
refs
|
||||
}
|
||||
|
||||
fn collect_type_refs_helper(&self, type_expr: &TypeExpr, refs: &mut Vec<String>) {
|
||||
match type_expr {
|
||||
TypeExpr::Named(ident) => {
|
||||
// Only include user-defined types, not builtins
|
||||
match ident.name.as_str() {
|
||||
"Int" | "Float" | "Bool" | "String" | "Char" | "Unit" | "_" => {}
|
||||
name => refs.push(name.to_string()),
|
||||
}
|
||||
}
|
||||
TypeExpr::App(constructor, args) => {
|
||||
self.collect_type_refs_helper(constructor, refs);
|
||||
for arg in args {
|
||||
self.collect_type_refs_helper(arg, refs);
|
||||
}
|
||||
}
|
||||
TypeExpr::Function { params, return_type, .. } => {
|
||||
for p in params {
|
||||
self.collect_type_refs_helper(p, refs);
|
||||
}
|
||||
self.collect_type_refs_helper(return_type, refs);
|
||||
}
|
||||
TypeExpr::Tuple(elements) => {
|
||||
for e in elements {
|
||||
self.collect_type_refs_helper(e, refs);
|
||||
}
|
||||
}
|
||||
TypeExpr::Record(fields) => {
|
||||
for f in fields {
|
||||
self.collect_type_refs_helper(&f.typ, refs);
|
||||
}
|
||||
}
|
||||
TypeExpr::Unit => {}
|
||||
TypeExpr::Versioned { base, .. } => {
|
||||
self.collect_type_refs_helper(base, refs);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Type check a program with module support
|
||||
pub fn check_program_with_modules(
|
||||
&mut self,
|
||||
@@ -790,6 +917,28 @@ impl TypeChecker {
|
||||
}
|
||||
|
||||
fn check_function(&mut self, func: &FunctionDecl) {
|
||||
// Validate that all declared effects exist
|
||||
let builtin_effects = ["Console", "Fail", "State", "Reader", "Random", "Time", "File", "Process", "Http", "HttpServer", "Test"];
|
||||
for effect in &func.effects {
|
||||
let is_builtin = builtin_effects.contains(&effect.name.as_str());
|
||||
let is_defined = self.env.lookup_effect(&effect.name).is_some();
|
||||
if !is_builtin && !is_defined {
|
||||
// Find similar effect names for suggestion
|
||||
let mut available: Vec<&str> = builtin_effects.to_vec();
|
||||
available.extend(self.env.effects.keys().map(|s| s.as_str()));
|
||||
let suggestions = find_similar_names(&effect.name, available, 2);
|
||||
|
||||
let mut message = format!("Unknown effect: {}", effect.name);
|
||||
if let Some(hint) = format_did_you_mean(&suggestions) {
|
||||
message.push_str(&format!(". {}", hint));
|
||||
}
|
||||
self.errors.push(TypeError {
|
||||
message,
|
||||
span: effect.span,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Set up the environment with parameters
|
||||
let mut local_env = self.env.clone();
|
||||
for param in &func.params {
|
||||
|
||||
Reference in New Issue
Block a user