test(#38): cover vector BinaryOp arithmetic and scalar-vector promotion (phase 4)#262
test(#38): cover vector BinaryOp arithmetic and scalar-vector promotion (phase 4)#262Jaskirat-s7 wants to merge 14 commits intoarxlang:mainfrom
Conversation
…n zero-init paths - Add Float32 to test_if_else_stmt and test_if_only_stmt parametrize lists - Add test_variable_declaration_no_initializer for Int32 and Float32 - Fix: VariableDeclaration visitor checked 'node.value is not None' but astx defaults value to astx.Undefined(); added isinstance check so uninitialized vars reach the zero-init path (llvmliteir.py lines 3023-3059)
…tion_no_initializer
Add PrintExpr and expected_output assertion ('0' for Int32,
'0.000000' for Float32/Float64) so the test proves default-to-zero
behavior at runtime, not just that the build does not crash.
Also add Float64 to the parametrize list.
… review
- add PrintExpr + expected_output assertion ('0' for ints, '0.000000'
for floats) to prove zero-default at runtime, not just build success
- remove unused literal_type parameter from signature and parametrize
- add Int8, Int16, Int64 alongside Int32/Float32/Float64 for consistency
Add test_while_float_condition to test_while.py which: - Initializes a Float32 variable to 3.0 - Uses it directly as the WhileStmt condition (triggers fcmp_ordered conversion) - Decrements it each iteration until 0.0 - Asserts 'done' is printed proving the loop ran and terminated Covers previously uncovered lines 1362-1363 in llvmliteir.py (fcmp_ordered float-to-i1 cast path inside WhileStmt visitor).
Add 4 parametrized tests to test_binary_op.py targeting previously uncovered float comparison paths in LLVMLiteIRVisitor: - test_binary_op_float_le -> covers line 1115 (fcmp_ordered <=) - test_binary_op_float_ge -> covers line 1125 (fcmp_ordered >=) - test_binary_op_float_eq -> covers line 1165 (fcmp_ordered ==) - test_binary_op_float_ne -> covers line 1188 (fcmp_ordered !=) Each test has two parametrized cases (true/false branches) with expected_output assertions proving semantically correct output.
…le tests
- merge test_binary_op_float_le/ge/eq/ne into single parametrized
test_binary_op_float_comparison accepting op, true_label, false_label
- replace UnaryOp('++') with VariableAssignment+BinaryOp('+') in
test_while_expr to support Float32/Float64 (++ generates 'add' not
'fadd' for floats causing invalid LLVM IR)
- add Float32 and Float64 to test_while_expr parametrize list
…promotion
Add test_vector_binary_op.py with 17 targeted tests covering previously
uncovered lines 886-932, 938-1023 in llvmliteir.py:
Integer vector ops: parametrize +,-,*,/ → add,sub,mul,sdiv
Float vector ops: parametrize +,-,*,/ → fadd,fsub,fmul,fdiv
Error cases: unsupported op, size mismatch, element type mismatch
Scalar-vector promotion: matching-type splat (both sides),
fptrunc (double→float), fpext (float→double) — both sides
All tests assert type, lane count, element type, and opname
(functionality, not just execution). Uses pytest.mark.parametrize
throughout.
Jaskirat-s7
force-pushed
the
feature-issue-38-phase4
branch
from
14f43f1 to
c017ef7
Compare
|
@yuvimittal , here is the exact breakdown of what those 305 lines actually are:
only 84 lines are actual test logic and the rest is project-mandated docstring format, whitespace, and two reusable helpers. For 17 tests, that's ~5 lines of logic per test ,which is what heavy use of pytest.mark.parametrize gives you. |
- Remove redundant cast to LLVMLiteIRVisitor (redundant-cast) - Use narrower type: ignore[method-assign] instead of [assignment]
| update = astx.VariableAssignment( | ||
| name="a", | ||
| value=astx.BinaryOp( | ||
| op_code="+", |
There was a problem hiding this comment.
why did we change '++' to '+' operator here?
There was a problem hiding this comment.
this change is from phase 3 (PR #258), not phase 4 — it's showing here because phase 4 is stacked on top of phase 3.The actual change in this phase 4 PR is only tests/test_vector_binary_op.py
(1 new file, 305 insertions). The test_while.py diff is carried over from the earlier phase.
As for the ++ → + change itself: ASTx's BinaryOp uses + as the op_code for addition. The ++ was incorrect and it's not a recognized op_code in ASTx. The test constructs a = a + 1 via VariableAssignment with a BinaryOp(op_code="+", ...), which is the standard way to express a loop counter increment in ASTx.
| @@ -0,0 +1,303 @@ | |||
| """ | |||
| title: Tests for vector BinaryOp arithmetic and scalar-vector promotion. | |||
There was a problem hiding this comment.
someone is already working on tests for vector
There was a problem hiding this comment.
Hi @yuvimittal, thanks for flagging that. I am aware and i actually reached out to @yogendra-17 last week asking which specific lines/areas they're targeting so we could divide the work cleanly and avoid overlap. That was a week ago and there's been no response from their side with any specifics.
I have progressive work in this area phase 2 and phase 3 are already good , and this phase 4 builds on top of that. The vector BinaryOp paths were completely untested and this PR brings coverage from 86% to 89%. i am happy to adjust if there's any overlap, but want to keep the momentum going.
There was a problem hiding this comment.
@Jaskirat-s7 , i have already raised a PR for vector.
|
@yogendra-17 , can you please tell me what file did you changed and what exactly are these 599 insertions in your draft PR and what logic they implement so that we can agree on a common one PR.. |
|
@yogendra-17 , give me your discord , we will sort this conflict there.. |
|
@Jaskirat-s7 , i have messaged you there |
|
@yuvimittal , Closing this PR. |
3 participants
Summary
Phase 4 of the coverage improvement effort (Issue #38). Adds test_vector_binary_op.py with 17 parametrized tests covering previously untested vector BinaryOp arithmetic and scalar-vector promotion paths in llvmliteir.py (lines 886–1023).
Coverage: 86% → 89%
What's covered
+,-,*,/+,-,*,/Reviewer standards
pytest.mark.parametrize— used throughoutProof