Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f1c325b
Docs: Report critical stack overflow in argument parsing
logbie Aug 12, 2025
255b3b2
rename file
logbie Aug 12, 2025
7f65226
claude
logbie Aug 12, 2025
fa01d62
refactor(parser): Refine syntax for actions, interfaces, and operators
logbie Aug 12, 2025
96cae27
Add '--parse' alias for '--ast' flag
logbie Aug 12, 2025
aa47a05
test: Add failing test for container property access in methods
logbie Aug 12, 2025
582174e
feat: Fix container property access in methods
logbie Aug 12, 2025
a942646
feat: Complete container property access fix with inheritance support
logbie Aug 12, 2025
fb99a58
Fixes container property access from within methods
logbie Aug 12, 2025
667bfb9
test: Add failing test for stack overflow in nested async operations
logbie Aug 12, 2025
4e51de7
fix: Resolve stack overflow in nested async operations
logbie Aug 12, 2025
d1c3e63
fix: Use cross-platform stack size configuration
logbie Aug 12, 2025
f18d382
fix: Update analyzer to recognize static properties in inheritance chain
logbie Aug 12, 2025
d34fc7b
fix: Improve return-type parsing logic in container actions
logbie Aug 12, 2025
b32c8b4
test: Add failing tests for container parsing issues
logbie Aug 12, 2025
e4d0bd7
feat: Fix container parsing issues - TDD implementation complete
logbie Aug 12, 2025
7e1bebc
hfgh
logbie Aug 12, 2025
d83925d
Refactor conditional logic and remove obsolete tests
logbie Aug 12, 2025
144a2b3
feat(parser): Add test for double colon consumption bug
logbie Aug 12, 2025
1ac1969
feat(parser): Add tests for container/action AST structure and corrup…
logbie Aug 12, 2025
f2bcc9a
update: Add git fetch to allowed commands
logbie Aug 12, 2025
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
19 changes: 19 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Windows (MSVC toolchain) - 8MB stack
[target.'cfg(all(target_os = "windows", target_env = "msvc"))']
rustflags = ["-C", "link-arg=/STACK:8388608"]

# Windows (GNU toolchain) - 8MB stack
[target.'cfg(all(target_os = "windows", target_env = "gnu"))']
rustflags = ["-C", "link-arg=-Wl,--stack,8388608"]

# Linux - 8MB stack
[target.'cfg(target_os = "linux")']
rustflags = ["-C", "link-arg=-Wl,-z,stack-size=8388608"]

# macOS - 8MB stack
[target.'cfg(target_os = "macos")']
rustflags = ["-C", "link-arg=-Wl,-stack_size,0x800000"]

# FreeBSD and other Unix-like systems - 8MB stack
[target.'cfg(all(unix, not(any(target_os = "macos", target_os = "linux"))))']
rustflags = ["-C", "link-arg=-Wl,-z,stack-size=8388608"]
Comment on lines +9 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Recommended approach for Unix-like platforms: avoid link-time stack sizing; use runtime/system knobs

  • Main thread: prefer documenting environment/system requirements (e.g., ulimit -s) or call setrlimit(RLIMIT_STACK) early in main if permissible.
  • Worker threads: set per-thread stacks using std::thread::Builder::stack_size or the RUST_MIN_STACK env var.
  • Keep the macOS ld64 flag and Windows flags; drop the Linux/other Unix link flags for portability.

Would you like a small main.rs helper that raises RLIMIT_STACK at startup on Unix platforms (behind a cfg(unix) + libc dep) and uses thread::Builder for worker threads?

🤖 Prompt for AI Agents
.cargo/config.toml lines 9-19: the file sets link-time stack-size flags for
Linux and other Unix targets which is non-portable and discouraged; remove the
rustflags entries that add link-arg=-Wl,-z,stack-size=... for Linux and the
generic Unix target, keep the macOS ld64 flag (and leave any Windows flags
elsewhere), and instead document in the repo README to require increased
main-thread stack via system settings (ulimit -s) or call
setrlimit(RLIMIT_STACK) early in main on Unix; for worker threads, allocate
stacks with std::thread::Builder::stack_size or recommend setting
RUST_MIN_STACK, and optionally add a small helper behind cfg(unix) that raises
RLIMIT_STACK at startup and examples showing thread::Builder usage.

Comment on lines +17 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Overbroad “unix but not macOS/Linux” block passes ELF flags to non-ELF targets (iOS, etc.)

This cfg matches iOS, Android, and other Unix-family targets. -z,stack-size is an ELF linker flag and is invalid for Mach-O (iOS), which will fail linking. Even on BSDs, support depends on using lld and may still be undesirable.

Apply:

-# FreeBSD and other Unix-like systems - 8MB stack
-[target.'cfg(all(unix, not(any(target_os = "macos", target_os = "linux"))))']
-rustflags = ["-C", "link-arg=-Wl,-z,stack-size=8388608"]

If you specifically want to experiment on FreeBSD with lld, gate narrowly and be prepared to revert:

# FreeBSD (lld) — use with caution; may be ignored or undesirable
[target.'cfg(target_os = "freebsd")']
rustflags = ["-C", "link-arg=-Wl,-z,stack-size=8388608"]
🤖 Prompt for AI Agents
.cargo/config.toml around lines 17 to 19: the current cfg block matches all
unix-like targets except macOS and Linux, which incorrectly passes ELF-specific
linker flags (-Wl,-z,stack-size=8388608) to non-ELF targets (iOS, Android, etc.)
and can break linking; replace the broad cfg with a narrowly targeted one (e.g.,
target_os = "freebsd") if you only intend this for FreeBSD (and only when using
lld), or remove the rustflags entirely; if you keep it for FreeBSD, gate it to
target_os = "freebsd" and document that it’s experimental and may need to be
reverted.

6 changes: 5 additions & 1 deletion .claude/settings.local.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@
"Bash(../target/release/wfl --parse basic_syntax_comprehensive.wfl)",
"Bash(target\\release\\wfl.exe:*)",
"Bash(targetreleasewfl.exe TestProgramstest_length.wfl)",
"Bash(../target/release/wfl.exe test_redefinition_error.wfl:*)"
"Bash(../target/release/wfl.exe test_redefinition_error.wfl:*)",
"Bash(../target/debug/wfl.exe --parse containers_comprehensive.wfl)",
"Bash(.targetdebugwfl.exe --parse TestProgramscontainers_comprehensive.wfl)",
"Bash(.targetreleasewfl.exe TestProgramstest_redefinition_error.wfl)",
"Bash(git fetch:*)"
],
"deny": []
}
Expand Down
723 changes: 0 additions & 723 deletions TestPrograms/args_comprehensive.wfl.lex.txt

This file was deleted.

19 changes: 0 additions & 19 deletions TestPrograms/basic_syntax_comprehensive_debug.txt

This file was deleted.

21 changes: 21 additions & 0 deletions TestPrograms/container_parsing_test.wfl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Minimal test case to reproduce container parsing issues
// This file should fail to parse initially, then pass after fixes

create container SimpleTest:
property name: Text

action greet:
display "Hello"
end

action set_name needs new_name: Text:
store name as new_name
end
end

create new SimpleTest as test:
name is "Test"
end

test.greet()
test.set_name("Fixed")
147 changes: 147 additions & 0 deletions TestPrograms/container_parsing_test.wfl.ast.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
AST output for: TestPrograms/container_parsing_test.wfl
==============================================

Program with 4 statements:

Statement #1: ContainerDefinition {
name: "SimpleTest",
extends: None,
implements: [],
properties: [
PropertyDefinition {
name: "name",
property_type: Some(
Text,
),
default_value: None,
validation_rules: [],
visibility: Public,
is_static: false,
line: 5,
column: 5,
},
],
methods: [
ActionDefinition {
name: "greet",
parameters: [],
body: [
DisplayStatement {
value: Literal(
String(
"Hello",
),
8,
17,
),
line: 9,
column: 5,
},
],
return_type: None,
line: 0,
column: 0,
},
ActionDefinition {
name: "set_name",
parameters: [
Parameter {
name: "new_name",
param_type: Some(
Text,
),
default_value: None,
line: 11,
column: 27,
},
],
body: [
VariableDeclaration {
name: "name",
value: Variable(
"new_name",
12,
23,
),
is_constant: false,
line: 12,
column: 9,
},
],
return_type: None,
line: 0,
column: 0,
},
],
events: [],
static_properties: [],
static_methods: [],
line: 4,
column: 1,
}

Statement #2: ContainerInstantiation {
container_type: "SimpleTest",
instance_name: "test",
arguments: [],
property_initializers: [
PropertyInitializer {
name: "name",
value: Literal(
String(
"Test",
),
17,
13,
),
line: 17,
column: 5,
},
],
line: 16,
column: 1,
}

Statement #3: ExpressionStatement {
expression: MethodCall {
object: Variable(
"test",
20,
1,
),
method: "greet",
arguments: [],
line: 20,
column: 1,
},
line: 21,
column: 1,
}

Statement #4: ExpressionStatement {
expression: MethodCall {
object: Variable(
"test",
21,
1,
),
method: "set_name",
arguments: [
Argument {
name: None,
value: Literal(
String(
"Fixed",
),
21,
15,
),
},
],
line: 21,
column: 1,
},
line: 0,
column: 0,
}

15 changes: 15 additions & 0 deletions TestPrograms/container_property_access_test.wfl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Test for container property access in methods
create container Person:
property name: Text

action get_name:
return name
end
end

create new Person as alice:
name is "Alice"
end

store result as alice.get_name()
display result
111 changes: 111 additions & 0 deletions TestPrograms/container_property_access_test.wfl.ast.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
AST output for: TestPrograms/container_property_access_test.wfl
==============================================

Program with 4 statements:

Statement #1: ContainerDefinition {
name: "Person",
extends: None,
implements: [],
properties: [
PropertyDefinition {
name: "name",
property_type: Some(
Text,
),
default_value: None,
validation_rules: [],
visibility: Public,
is_static: false,
line: 3,
column: 5,
},
],
methods: [
ActionDefinition {
name: "get_name",
parameters: [],
body: [
ExpressionStatement {
expression: Variable(
"Text",
5,
22,
),
line: 6,
column: 9,
},
Comment on lines +29 to +37

@coderabbitai coderabbitai Bot Aug 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent AST structure for method body.

The AST shows an ExpressionStatement containing a Variable("Text") at lines 29-37, which appears to be incorrectly parsed. The Text token seems to be interpreted as a variable reference rather than as part of the return type declaration.

This AST structure suggests a parsing issue where the return type syntax may not be correctly handled. The Text token should be part of the method signature, not parsed as a statement in the method body.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@claude can you investigate this and see if any action needs to be taken here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@claude claude Bot Aug 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude finished @logbie's task —— View job


Investigating AST Structure Issue

CodeRabbit flagged a potential parsing issue in the AST file. I've completed my investigation:

Todo List:

  • Read the AST file to understand the flagged structure
  • Read the corresponding WFL source file
  • Compare AST structure with expected parsing
  • Check other AST files for similar patterns
  • Analyze the parser code for the root cause
  • Determine if action is needed and provide recommendation

🐛 Critical Parser Bug Identified

Root Cause:

The issue is in parse_container_action_definition() at src/parser/mod.rs:4740-4766. There's a logic error in the return type parsing:

  1. Line 4744: Parser consumes the : colon when checking for return type
  2. Line 4746-4762: Parser looks for type identifier after colon, finds none, returns None
  3. Line 4764: Parser calls expect_token(Token::Colon, ...) - but the colon was already consumed!

This causes the parser to get out of sync with the token stream.

Evidence:

The AST shows Variable("Text", 5, 22) with coordinates pointing to line 3, column 22:

property name: Text
             ^ This "Text" token (line 3, col 22)

But it appears in the method body at line 6, column 9 - indicating token stream corruption.

The Fix Required:

In src/parser/mod.rs around line 4764, remove the redundant expect_token(Token::Colon) call since the colon is already consumed in the return type parsing logic.

         } else {
-            self.expect_token(Token::Colon, "Expected ':' after action declaration")?;
             None
         };

Impact:

  • Parsing works: The test program runs successfully despite the AST corruption
  • ⚠️ AST corruption: Method bodies contain spurious tokens from earlier parsing
  • ⚠️ Potential failures: Complex container methods may fail to parse correctly

📋 Recommendation:

Action Required - This is a legitimate parsing bug that should be fixed to prevent AST corruption and potential parsing failures in more complex scenarios.


ReturnStatement {
value: Some(
Variable(
"name",
6,
16,
),
),
line: 6,
column: 9,
},
],
return_type: None,
line: 0,
column: 0,
},
],
events: [],
static_properties: [],
static_methods: [],
line: 2,
column: 1,
}

Statement #2: ContainerInstantiation {
container_type: "Person",
instance_name: "alice",
arguments: [],
property_initializers: [
PropertyInitializer {
name: "name",
value: Literal(
String(
"Alice",
),
11,
13,
),
line: 11,
column: 5,
},
],
line: 10,
column: 1,
}

Statement #3: VariableDeclaration {
name: "result",
value: MethodCall {
object: Variable(
"alice",
14,
17,
),
method: "get_name",
arguments: [],
line: 14,
column: 17,
},
is_constant: false,
line: 14,
column: 1,
}

Statement #4: DisplayStatement {
value: Variable(
"result",
15,
9,
),
line: 15,
column: 9,
}

Loading
Loading