gemini-code-assist[bot] commented on code in PR #18341:
URL: https://github.com/apache/tvm/pull/18341#discussion_r2375404488
##########
python/tvm/script/parser/core/evaluator.py:
##########
@@ -324,10 +324,18 @@ def _eval_compare(self, fields: Dict[str, Any]) -> Any:
res : Any
The evaluation result.
"""
- value = self._eval_expr(fields["left"])
- for op, rhs in zip(fields["ops"], fields["comparators"]):
- value = _eval_op(op, values=[value, self._eval_expr(rhs)])
- return value
+ values = [self._eval_expr(fields["left"])]
+ values.extend([self._eval_expr(rhs) for rhs in fields["comparators"]])
+ result = None
+ assert len(fields["ops"]) == len(values) - 1
+
+ for index, op in enumerate(fields["ops"]):
+ sub_result = _eval_op(op, values=[values[index], values[index +
1]])
+ if result is None:
+ result = sub_result
+ else:
+ result = _eval_op(doc.And(), values=[result, sub_result])
+ return result
Review Comment:
The current implementation evaluates all comparator expressions upfront.
This deviates from Python's chained comparison semantics, which use
short-circuiting and evaluate expressions sequentially. For an expression like
`a < b < f()` where `f()` has side effects, `f()` should not be called if `a <
b` is false. Eagerly evaluating all expressions can lead to incorrect behavior
in such cases.
The implementation should be updated to evaluate expressions one by one
within a loop, carrying over the value of the previous right-hand side for the
next comparison. This ensures that each intermediate expression is evaluated
only once and in the correct order, matching Python's behavior.
```python
left_val = self._eval_expr(fields["left"])
result = None
# Per Python's chained comparison semantics, intermediate expressions
# are evaluated only once and in order. This loop structure ensures
that.
# For example, `a < b < c` is equivalent to `a < b and b < c`.
for op, comparator_expr in zip(fields["ops"], fields["comparators"]):
right_val = self._eval_expr(comparator_expr)
sub_result = _eval_op(op, values=[left_val, right_val])
if result is None:
result = sub_result
else:
result = _eval_op(doc.And(), values=[result, sub_result])
left_val = right_val
return result
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
