Skip to content

Commit 76b2f78

Browse files
can1357sylvestre
andauthored
expr: fix eager evaluation of parenthesized dead branches (#11395)
Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
1 parent 90c7896 commit 76b2f78

2 files changed

Lines changed: 27 additions & 23 deletions

File tree

src/uu/expr/src/syntax_tree.rs

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -624,9 +624,6 @@ pub struct AstNode {
624624
#[derive(Debug, Clone)]
625625
#[cfg_attr(test, derive(Eq, PartialEq))]
626626
pub enum AstNodeInner {
627-
Evaluated {
628-
value: NumOrStr,
629-
},
630627
Leaf {
631628
value: MaybeNonUtf8String,
632629
},
@@ -650,15 +647,6 @@ impl AstNode {
650647
Parser::new(input).parse()
651648
}
652649

653-
pub fn evaluated(self) -> ExprResult<Self> {
654-
Ok(Self {
655-
id: get_next_id(),
656-
inner: AstNodeInner::Evaluated {
657-
value: self.eval()?,
658-
},
659-
})
660-
}
661-
662650
pub fn eval(&self) -> ExprResult<NumOrStr> {
663651
// This function implements a recursive tree-walking algorithm, but uses an explicit
664652
// stack approach instead of native recursion to avoid potential stack overflow
@@ -669,9 +657,6 @@ impl AstNode {
669657

670658
while let Some(node) = stack.pop() {
671659
match &node.inner {
672-
AstNodeInner::Evaluated { value, .. } => {
673-
result_stack.insert(node.id, Ok(value.clone()));
674-
}
675660
AstNodeInner::Leaf { value, .. } => {
676661
result_stack.insert(node.id, Ok(value.to_owned().into()));
677662
}
@@ -896,9 +881,7 @@ impl<'a, S: AsRef<MaybeNonUtf8Str>> Parser<'a, S> {
896881
value: self.next()?.into(),
897882
},
898883
b"(" => {
899-
// Evaluate the node just after parsing to we detect arithmetic
900-
// errors before checking for the closing parenthesis.
901-
let s = self.parse_expression()?.evaluated()?;
884+
let s = self.parse_expression()?;
902885

903886
match self.next() {
904887
Ok(b")") => {}
@@ -1070,9 +1053,7 @@ mod test {
10701053
AstNode::parse(&["(", "1", "+", "2", ")", "*", "3"]),
10711054
Ok(op(
10721055
BinOp::Numeric(NumericOp::Mul),
1073-
op(BinOp::Numeric(NumericOp::Add), "1", "2")
1074-
.evaluated()
1075-
.unwrap(),
1056+
op(BinOp::Numeric(NumericOp::Add), "1", "2"),
10761057
"3"
10771058
))
10781059
);

tests/by-util/test_expr.rs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,29 @@ fn test_and() {
217217
new_ucmd!().args(&["", "&", ""]).fails().stdout_only("0\n");
218218
}
219219

220+
#[test]
221+
fn test_parenthesized_short_circuit_dead_branches() {
222+
new_ucmd!()
223+
.args(&["1", "|", "(", "1", "/", "0", ")"])
224+
.succeeds()
225+
.stdout_only("1\n");
226+
227+
new_ucmd!()
228+
.args(&["0", "&", "(", "1", "/", "0", ")"])
229+
.fails_with_code(1)
230+
.stdout_only("0\n");
231+
232+
new_ucmd!()
233+
.args(&["1", "|", "(", "0", "&", "(", "1", "/", "0", ")", ")"])
234+
.succeeds()
235+
.stdout_only("1\n");
236+
237+
new_ucmd!()
238+
.args(&["0", "&", "(", "1", "|", "(", "1", "/", "0", ")", ")"])
239+
.fails_with_code(1)
240+
.stdout_only("0\n");
241+
}
242+
220243
#[test]
221244
fn test_length_fail() {
222245
new_ucmd!().args(&["length", "αbcdef", "1"]).fails();
@@ -580,11 +603,11 @@ fn test_num_str_comparison() {
580603
}
581604

582605
#[test]
583-
fn test_eager_evaluation() {
606+
fn test_missing_closing_parenthesis_reports_syntax_error() {
584607
new_ucmd!()
585608
.args(&["(", "1", "/", "0"])
586609
.fails()
587-
.stderr_contains("division by zero");
610+
.stderr_contains("expecting ')' after '0'");
588611
}
589612

590613
#[test]

0 commit comments

Comments
 (0)