-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for parsing and analysing negative values #9
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,20 +76,10 @@ fn handle_pow( | |
/// # Returns | ||
/// | ||
/// This function returns a `Result` object with an empty Ok value if the operation is successful. | ||
/// | ||
/// # Example | ||
/// | ||
/// ``` | ||
/// use analyze_rounding::visit; | ||
/// use FormulaConfig; | ||
/// | ||
/// let expr = Expr::Number(5); | ||
/// let formula_config = FormulaConfig::new(); | ||
/// visit(&expr, true, &formula_config); | ||
/// ``` | ||
fn visit(expr: &Expr, rounding_direction: bool, formula_config: &mut FormulaConfig) -> Result<()> { | ||
match expr { | ||
Expr::Number(_) | Expr::Id(_) | Expr::Error => (), | ||
Expr::Negative(nexpr) => visit(nexpr, !rounding_direction, formula_config)?, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification The search results indicate that there are no direct unit tests for
To ensure the new handling of
Analysis chainEnsure the new handling of Scripts ExecutedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that there are unit tests covering the new handling of `Expr::Negative` in the `visit` function.
# Test: Search for unit tests related to `Expr::Negative`. Expect: At least one test.
rg --type rust 'test.*Expr::Negative'
Length of output: 39 Script: #!/bin/bash
# Description: Verify the presence of unit tests covering the new handling of `Expr::Negative` in the `visit` function.
# Step 1: Locate the definition of the `visit` function.
ast-grep --lang rust --pattern 'fn visit($_, $_, $_) -> $_ { $$$ }'
# Step 2: Search for references to `Expr::Negative` in test files.
rg --type rust 'Expr::Negative'
Length of output: 2840 |
||
Expr::Op(left, op, right) => { | ||
let (left_rounding, right_rounding) = match op { | ||
Opcode::Add => (rounding_direction, rounding_direction), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
use std::cell::RefCell; | ||
|
||
use super::ast::{Expr, Opcode, Rounding}; | ||
|
||
fn is_negative_value(expr: Box<Expr>) -> (bool, Box<Expr>) { | ||
match *expr { | ||
Expr::Negative(ne) => match *ne { | ||
Expr::Negative(internal) => is_negative_value(internal), | ||
_ => (true, ne), | ||
} | ||
// Need to handle this case separately because in case of nested negatives the | ||
// inner most operation will not be simplified | ||
Expr::Op(l, o, r) => { | ||
let simplified_internal_expr = simplify_sign(Box::new(Expr::Op(l, o, r))); | ||
|
||
match *simplified_internal_expr { | ||
Expr::Negative(ne) => (true, ne), | ||
_ => (false, simplified_internal_expr), | ||
} | ||
}, | ||
_ => (false, expr), | ||
} | ||
} | ||
Comment on lines
+5
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review the logic in Consider simplifying the nested match statements to improve readability and potentially enhance performance. |
||
|
||
fn simplify_add(lnv: (bool, Box<Expr>), rnv: (bool, Box<Expr>)) -> Box<Expr> { | ||
let expr = match (lnv.0, rnv.0) { | ||
(true, false) => Expr::Op(rnv.1, Opcode::Sub, lnv.1), | ||
(true, true) => Expr::Negative(Box::new(Expr::Op(lnv.1, Opcode::Add, rnv.1))), | ||
(false, true) => Expr::Op(lnv.1, Opcode::Sub, rnv.1), | ||
(false, false) => Expr::Op(lnv.1, Opcode::Add, rnv.1), | ||
}; | ||
|
||
Box::new(expr) | ||
} | ||
Comment on lines
+25
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification The
Analysis chainValidate the correctness of the Scripts ExecutedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that there are unit tests covering the `simplify_add` function.
# Test: Search for unit tests related to `simplify_add`. Expect: At least one test.
rg --type rust 'test.*simplify_add'
Length of output: 37 Script: #!/bin/bash
# Locate the definition of the `simplify_add` function to confirm its location.
ast-grep --lang rust --pattern 'fn simplify_add($_, $_) -> $_ { $$$ }'
# Search for any references to `simplify_add` in the codebase to identify potential indirect tests.
rg 'simplify_add'
Length of output: 1054 Script: #!/bin/bash
# Locate the context in which `simplify_add` is referenced within `src/analyzer/simplify_expr.rs`.
rg 'simplify_add' src/analyzer/simplify_expr.rs -A 5 -B 5
# Search for any test files or modules that might contain tests for `simplify_add`.
rg 'simplify_add' --type rust --glob '*test*'
Length of output: 1001 |
||
|
||
fn simplify_sub(lnv: (bool, Box<Expr>), rnv: (bool, Box<Expr>)) -> Box<Expr> { | ||
let expr = match (lnv.0, rnv.0) { | ||
(true, false) => Expr::Negative(Box::new(Expr::Op(lnv.1, Opcode::Add, rnv.1))), | ||
(true, true) => Expr::Op(rnv.1, Opcode::Sub, lnv.1), | ||
(false, true) => Expr::Op(lnv.1, Opcode::Add, rnv.1), | ||
(false, false) => Expr::Op(lnv.1, Opcode::Sub, rnv.1), | ||
}; | ||
|
||
Box::new(expr) | ||
} | ||
Comment on lines
+36
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check the logic in Consider reviewing the handling of negative values to ensure that the function behaves as expected under all conditions. |
||
|
||
fn simplify_mul(lnv: (bool, Box<Expr>), rnv: (bool, Box<Expr>), r: RefCell<Rounding>) -> Box<Expr> { | ||
let expr = if lnv.0 ^ rnv.0 { | ||
Expr::Negative(Box::new(Expr::Op(lnv.1, Opcode::Mul(RefCell::new(Rounding::Init)), rnv.1))) | ||
} else { | ||
Expr::Op(lnv.1, Opcode::Mul(r), rnv.1) | ||
}; | ||
|
||
Box::new(expr) | ||
} | ||
|
||
fn simplify_div(lnv: (bool, Box<Expr>), rnv: (bool, Box<Expr>), r: RefCell<Rounding>) -> Box<Expr> { | ||
let expr = if lnv.0 ^ rnv.0 { | ||
Expr::Negative(Box::new(Expr::Op(lnv.1, Opcode::Div(RefCell::new(Rounding::Init)), rnv.1))) | ||
} else { | ||
Expr::Op(lnv.1, Opcode::Div(r), rnv.1) | ||
}; | ||
|
||
Box::new(expr) | ||
} | ||
|
||
fn simplify_pow(lnv: (bool, Box<Expr>), rnv: (bool, Box<Expr>)) -> Box<Expr> { | ||
let expr = if rnv.0 { | ||
Expr::Op( | ||
Box::new(Expr::Number(1)), | ||
Opcode::Div(RefCell::new(Rounding::Init)), | ||
Box::new(Expr::Op(lnv.1, Opcode::Pow, rnv.1)) | ||
) | ||
} else { | ||
Expr::Op(lnv.1, Opcode::Pow, rnv.1) | ||
}; | ||
|
||
Box::new(expr) | ||
} | ||
|
||
/// Simplifies the signs to bring the negative sign from values to the operations | ||
/// and finally bring it out to the expression level if possible | ||
/// It also reagganges the addition and substration formula to make them look better | ||
/// with the sign. For example (-a + b) will be re-arranged to (b - a) | ||
pub fn simplify_sign(expr: Box<Expr>) -> Box<Expr> { | ||
match *expr { | ||
Expr::Op(left, op, right) => { | ||
let simplified_left = match *left { | ||
Expr::Op(..) => simplify_sign(left), | ||
_ => left | ||
}; | ||
|
||
let simplified_right = match *right { | ||
Expr::Op(..) => simplify_sign(right), | ||
_ => right | ||
}; | ||
|
||
let lnv = is_negative_value(simplified_left); | ||
let rnv = is_negative_value(simplified_right); | ||
|
||
match op { | ||
Opcode::Add => simplify_add(lnv, rnv), | ||
Opcode::Sub => simplify_sub(lnv, rnv), | ||
Opcode::Mul(r) => simplify_mul(lnv, rnv, r), | ||
Opcode::Div(r) => simplify_div(lnv, rnv, r), | ||
Opcode::Pow => simplify_pow(lnv, rnv), | ||
} | ||
}, | ||
_ => expr | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,15 +6,15 @@ use lalrpop_util::ParseError; | |
|
||
grammar; | ||
|
||
Tier<Op,NextTier>: Box<Expr> = { | ||
Tier<Op,NextTier> Op NextTier => Box::new(Expr::Op(<>)), | ||
Tier<Op, NextTier>: Box<Expr> = { | ||
Tier<Op, NextTier> Op NextTier => Box::new(Expr::Op(<>)), | ||
NextTier | ||
}; | ||
|
||
pub Expr = Tier<ExprOp, Factor>; | ||
Factor = Tier<FactorOp, Term>; | ||
|
||
ExprOp: Opcode = { // (3) | ||
ExprOp: Opcode = { | ||
"+" => Opcode::Add, | ||
"-" => Opcode::Sub, | ||
}; | ||
|
@@ -27,12 +27,17 @@ FactorOp: Opcode = { | |
|
||
Term: Box<Expr> = { | ||
Num => Box::new(Expr::Number(<>)), | ||
"-" <Num> => Box::new(Expr::Negative(Box::new(Expr::Number(<>)))), | ||
|
||
Id => Box::new(Expr::Id(<>)), | ||
"-" <Id> => Box::new(Expr::Negative(Box::new(Expr::Id(<>)))), | ||
|
||
"-(" <Expr> ")" => Box::new(Expr::Negative(<>)), | ||
"(" <Expr> ")" | ||
}; | ||
|
||
Num: i32 = { | ||
r"[0-9]+" =>? i32::from_str(<>) | ||
Num: u32 = { | ||
r"[0-9]+" =>? u32::from_str(<>) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Add tests for parsing large numbers to ensure error handling is properly tested.
Consider adding tests to cover this scenario. Analysis chainValidate the error handling for large numbers during parsing. Scripts ExecutedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that the error handling for large numbers is correctly implemented and tested.
# Test: Search for unit tests related to error handling for large numbers. Expect: At least one test.
rg --type rust 'test.*"number is too big"'
Length of output: 44 Script: #!/bin/bash
# Description: Verify the presence of general number parsing tests and error handling for large numbers.
# Step 1: Search for general number parsing tests.
rg --type rust 'test.*number'
# Step 2: Examine the parser code for error handling related to large numbers.
rg --type rust 'u32::from_str' src/parser/arithmetic.lalrpop
# Step 3: Check for any relevant documentation or comments in the parser code.
rg --type rust 'large number' src/parser/arithmetic.lalrpop
Length of output: 241 Script: #!/bin/bash
# Description: Further investigate the handling of number parsing and error handling for large numbers.
# Step 1: Extract and review the `test_display_number` function.
rg --type rust 'fn test_display_number' -A 20
# Step 2: Search for error handling mechanisms in the parser code where `u32::from_str` is used.
rg --type rust 'u32::from_str' -A 5 src/parser/arithmetic.lalrpop
# Step 3: Search for any additional tests related to number parsing.
rg --type rust 'test.*parse.*number'
Length of output: 1235 Tip Codebase Verification Ensure the error handling for identifier parsing is robust and tested. Currently, there are no unit tests covering the error message "number is too big" in the arithmetic parser.
Analysis chainEnsure the error handling for identifier parsing is robust and tested. Scripts ExecutedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that the error handling for identifier parsing is robust and tested.
# Test: Search for unit tests related to error handling for identifiers. Expect: At least one test.
rg --type rust 'test.*"number is too big"'
Length of output: 44 Script: #!/bin/bash
# Description: Verify that the error handling for identifier parsing is robust and tested.
# Test: Search for unit tests related to parsing errors in general.
rg --type rust 'test.*parse_error'
# Test: Search for any error messages related to parsing numbers.
rg --type rust 'number is too big'
# Inspect the parser code for error handling mechanisms.
rg --type rust 'from_str' src/parser
Length of output: 223 Script: #!/bin/bash
# Description: Verify that the error handling for identifier parsing in the arithmetic parser is robust and tested.
# Test: Search for unit tests related to the arithmetic parser.
rg --type rust 'test.*arithmetic'
# Inspect the arithmetic parser code for error handling mechanisms.
rg --type rust 'Result' src/parser/arithmetic.lalrpop
Length of output: 89 Script: #!/bin/bash
# Description: Verify that the error handling for identifier parsing in the arithmetic parser is robust and tested.
# Test: Search for any other related tests or error handling mechanisms.
rg --type rust 'test.*' src/parser
# Check for comments or documentation related to error handling in the arithmetic parser.
rg --type rust 'error' src/parser/arithmetic.lalrpop
Length of output: 167 Script: #!/bin/bash
# Description: Verify the context of the error message "number is too big" in the arithmetic parser and search for any indirect references or tests.
# Inspect the specific lines in src/parser/arithmetic.lalrpop where the error message "number is too big" is used.
rg --type rust 'number is too big' src/parser/arithmetic.lalrpop -A 5 -B 5
# Search the entire codebase for any indirect references or tests related to the error message "number is too big".
rg --type rust 'number is too big'
Length of output: 412 |
||
.map_err(|_| ParseError::User { | ||
error: "number is too big" | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,7 +89,7 @@ fn find_less_greater_than_one(expr: &Expr, formula_config: &mut FormulaConfig) { | |
|
||
fn visit(expr: &Expr, formula_config: &mut FormulaConfig) { | ||
match expr { | ||
Expr::Number(_) | Expr::Id(_) | Expr::Error => (), | ||
Expr::Number(_) | Expr::Id(_) | Expr::Negative(_) | Expr::Error => (), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification There are no unit tests specifically covering the
Analysis chainEnsure the new handling of Scripts ExecutedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that there are unit tests covering the new handling of `Expr::Negative` in the `visit` function.
# Test: Search for unit tests related to `Expr::Negative`. Expect: At least one test.
rg --type rust 'test.*Expr::Negative'
Length of output: 39 Script: #!/bin/bash
# Description: Search for the `Expr::Negative` variant in the codebase to locate its usage.
# Search for the definition and usage of `Expr::Negative`.
rg 'Expr::Negative'
# Search for test files that might be testing the `visit` function or `Expr` variants.
rg --type rust 'fn visit'
# Search for any tests that might be indirectly testing `Expr::Negative`.
rg --type rust 'Negative'
Length of output: 2920 Script: #!/bin/bash
# Description: Search for test functions in the codebase to locate any indirect tests for `Expr::Negative`.
# Search for test functions in the codebase.
rg --type rust '#\[test\]'
# Search for test modules in the codebase.
rg --type rust 'mod test'
Length of output: 930 Script: #!/bin/bash
# Description: Search within the test functions for references to `Expr::Negative` or the `visit` function.
# Search for references to `Expr::Negative` within test functions.
rg --type rust 'Expr::Negative' src/analyzer.rs src/printer/latex_generator.rs src/analyzer/ast.rs
# Search for references to the `visit` function within test functions.
rg --type rust 'visit' src/analyzer.rs src/printer/latex_generator.rs src/analyzer/ast.rs
Length of output: 878 |
||
Expr::Op(left, op, right) => { | ||
if let Opcode::Pow = op { | ||
// We ignore if the following fail | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
Ensure that the
simplify_sign
function is thoroughly tested, especially since it manipulates the AST directly.simplify_sign
function is not directly tested in the codebase.src/analyzer.rs
does not contain tests forsimplify_sign
.Analysis chain
Ensure that the
simplify_sign
function is thoroughly tested, especially since it manipulates the AST directly.Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 118
Script:
Length of output: 46
Script:
Length of output: 525
Script:
Length of output: 47
Script:
Length of output: 98
Script:
Length of output: 718