Fix parsing bugs from last pull request #65

Alexiscomete · 2025-07-17T15:29:04Z

No description provided.

…g mut to context, separating OpI and OpO

Fix priority for VarDec We will need to add a function to dump newlines

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

This PR fixes parsing bugs from the last pull request and refactors core tokenization, AST evaluation, and execution logic. Key changes include:

Introducing a CharsIterator to enhance the tokenization process and updating all related tokenize_* functions.
Extending the Evaluate and Execute traits across AST nodes, wiring up ExecutionContext for runtime execution and error handling.
Adding an integration test harness that loads and runs .skrb scripts from resources/test_programs.

Reviewed Changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/utils.rs	Updated `clear` implementation with ANSI codes and improved comments
src/tokens.rs	Replaced raw `Chars` use with `CharsIterator` for tokenization
src/parse/nodes/vars.rs	Added `Evaluate` impl for `Vd` and refactored variable parsing
src/parse/nodes/operations.rs	Enhanced `Evaluate` implementations and expanded `GraphDisplay`
src/main.rs	Refactored `main` into `execute`, added colorized output and flags
src/tests/programs/from_files.rs	New file-based integration tests for `.skrb` scripts
resources/test_programs/operations.skrb	Added operation tests, but includes a typo in the native call prefix

Copilot · 2025-07-17T15:31:30Z

src/tokens.rs

- let mut current_ch = file_ch.next();
- // let mut operator2 = false;
+ let mut file_ch = CharsIterator {
+ file: &mut file.chars(),


Storing &mut file.chars() borrows a temporary iterator and will not compile or behave correctly. Consider changing CharsIterator.file to own the Chars iterator directly (e.g., file: Chars<'a>) and pass file: file.chars().

Copilot · 2025-07-17T15:31:30Z

src/tests/full_evaluation_tests.rs

+ .evaluate(&mut context)
+ .expect("Should be evaluated without error.")
+ .as_int(&context)
+ .expect("Should be an integi.");


There's a typo in the error message: "integi" should be "integer."

Suggested change

.expect("Should be an integi.");

.expect("Should be an integer.");

Copilot · 2025-07-17T15:31:31Z

src/tests/execute_tests.rs

+ .evaluate(&mut context)
+ .unwrap()
+ .as_int(&context)
+ .expect("Should be an integi.");


The message has a typo: "integi" should be "integer."

Suggested change

.expect("Should be an integi.");

.expect("Should be an integer.");

Copilot · 2025-07-17T15:31:31Z

src/parse/nodes/vars.rs

+ fn evaluate(&self, operation_context: &mut OperationContext) -> OperationO {
+ match self {
+ Self::Vd(vd) => vd.evaluate(operation_context),
+ _ => todo!(),


Using todo!() for all non-Vd variants will panic at runtime if you parse GlobalVar, PrivateVar, or ConstVar. Implement evaluation for those variants or handle them explicitly.

Suggested change

_ => todo!(),

Self::GlobalVar(global_var) => {

// Implement evaluation logic for GlobalVar or return a meaningful error

global_var.evaluate(operation_context)

}

Self::PrivateVar(private_var) => {

// Implement evaluation logic for PrivateVar or return a meaningful error

private_var.evaluate(operation_context)

}

Self::ConstVar(const_var) => {

// Implement evaluation logic for ConstVar or return a meaningful error

const_var.evaluate(operation_context)

}

Copilot · 2025-07-17T15:31:31Z

src/parse/nodes/operations.rs

- ValueBase::Int(value) => *value,
+ ValueBase::Int(value) => Ok(InternalInt::new_boxed(*value)),
+ ValueBase::Bool(value) => Ok(InternalIoi::new_boxed(*value)),
 _ => todo!(),


The Evaluate implementation for ValueBase does not handle Float and String variants and will panic. Add cases for ValueBase::Float and ValueBase::String.

Suggested change

_ => todo!(),

ValueBase::Float(value) => Ok(InternalInt::new_boxed(*value as i64)), // Assuming conversion to integer for compatibility

ValueBase::String(value) => Ok(InternalIoi::new_boxed(value.clone())), // Assuming InternalIoi supports strings

Copilot · 2025-07-17T15:31:32Z

src/main.rs

+ err.show();
+ panic!(
+ "{}",
+ "--- Your program stopped in a unexpected way ---".red()


Grammar fix: it should read "in an unexpected way" instead of "in a unexpected way."

Suggested change

"--- Your program stopped in a unexpected way ---".red()

"--- Your program stopped in an unexpected way ---".red()

Copilot · 2025-07-17T15:31:32Z

resources/test_programs/operations.skrb

+skrb assert_eq r a
+skrb assert_eq r b
+skrb assert_eq r c


The native call prefix should be skr_app (as in other tests), not skrb. Update lines to skr_app assert_eq r a.

Suggested change

skrb assert_eq r a

skrb assert_eq r b

skrb assert_eq r c

skr_app assert_eq r a

skr_app assert_eq r b

skr_app assert_eq r c

Copilot · 2025-07-17T15:31:32Z

src/utils.rs

+ stdout().flush().unwrap();
+ }


[nitpick] Calling unwrap() on flush() can panic in I/O errors. Consider propagating or handling the error instead of unwrapping.

Suggested change

stdout().flush().unwrap();

}

if let Err(e) = stdout().flush() {

eprintln!("Failed to flush stdout: {}", e);

}

Alexiscomete and others added 30 commits June 22, 2025 23:32

Better tokens with column but failing tests

65de490

Fixing yesterday issue about tokens, adding FileNode execution, addin…

517b156

…g mut to context, separating OpI and OpO

Adding errors to outputs

224d8a3

Printing steps

3c35339

Start native calls

13e3961

Good error message for return

1f6108b

Implementation of some small executions pieces

efbd677

Start to implement basic variables

76fff7b

Add scope and variable modification (*backend* only)

f97f5e1

Start variable creation

75a2df4

First working variable in Skribi!

a1034a7

Fix priority for VarDec We will need to add a function to dump newlines

Testing new patterns

5913a54

Add indent

8fa95f9

Implement indentation on most files

2790488

Fix graph display

81f5a9c

Testing

e63bd77

A little hotfix that might not always work

1e186ef

Start implementing variable modification

5b74af7

Variable modification working

9d48c1e

Start reading inputs

3aec43a

Read inputs

fca19c7

Assertions

1929782

Cargo fmt

0ddc185

Apply clippy

a3726db

Apply suggestions from code review

a7d8df8

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'main' into feature/execute/base

7ca25f0

Replace u32 by i32

09d6519

fmt

3d97ca4

Add basic trait and start implementing it

c21e783

Cargo fmt

4b9dfd5

Alexiscomete added 10 commits July 15, 2025 19:03

Implement Int and Ioi types

ed0fb9c

Fix errors linked to the new implementation

5013af1

Apply clippy suggestions

46dfdce

First test with file

cee620c

Second test

09d8acd

Add comment about failing test

093a439

Cargo fmt

624d63b

remove: ugly code in ioi type

5710cd2

3 tests working: fix priority of IdUseV and VarDec

33a91a3

Division and multiplication test

0966532

Alexiscomete requested review from Rignchen and Copilot July 17, 2025 15:29

Copilot AI reviewed Jul 17, 2025

View reviewed changes

This was a user error... (user = me)

6daf825

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix parsing bugs from last pull request #65

Fix parsing bugs from last pull request #65

Uh oh!

Alexiscomete commented Jul 17, 2025

Copilot AI left a comment

Copilot AI Jul 17, 2025

Copilot AI Jul 17, 2025

Copilot AI Jul 17, 2025

Copilot AI Jul 17, 2025

Copilot AI Jul 17, 2025

Copilot AI Jul 17, 2025

Copilot AI Jul 17, 2025

Copilot AI Jul 17, 2025

Labels

1 participant

	.expect("Should be an integi.");
	.expect("Should be an integer.");

- _ => todo!(),
+ Self::GlobalVar(global_var) => {
+ // Implement evaluation logic for GlobalVar or return a meaningful error
+ global_var.evaluate(operation_context)
+ }
+ Self::PrivateVar(private_var) => {
+ // Implement evaluation logic for PrivateVar or return a meaningful error
+ private_var.evaluate(operation_context)
+ }
+ Self::ConstVar(const_var) => {
+ // Implement evaluation logic for ConstVar or return a meaningful error
+ const_var.evaluate(operation_context)
+ }

	_ => todo!(),
	ValueBase::Float(value) => Ok(InternalInt::new_boxed(*value as i64)), // Assuming conversion to integer for compatibility
	ValueBase::String(value) => Ok(InternalIoi::new_boxed(value.clone())), // Assuming InternalIoi supports strings

	"--- Your program stopped in a unexpected way ---".red()
	"--- Your program stopped in an unexpected way ---".red()

- stdout().flush().unwrap();
- }
+ if let Err(e) = stdout().flush() {
+ eprintln!("Failed to flush stdout: {}", e);
+ }

Fix parsing bugs from last pull request #65

Are you sure you want to change the base?

Fix parsing bugs from last pull request #65

Uh oh!

Conversation

Alexiscomete commented Jul 17, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Labels

1 participant