Skip to content

[Misc] Add test verifying static if dead branch is not compiled.#402

Draft
hughperkins wants to merge 2 commits intomainfrom
hp/test-static-if-dead-branch
Draft

[Misc] Add test verifying static if dead branch is not compiled.#402
hughperkins wants to merge 2 commits intomainfrom
hp/test-static-if-dead-branch

Conversation

@hughperkins
Copy link
Collaborator

Places an unsupported import statement in the dead branch of a qd.static(True) / qd.static(False) if-statement. If the transformer incorrectly walks the dead branch, it raises QuadrantsSyntaxError.

Issue: #

Brief Summary

copilot:summary

Walkthrough

copilot:walkthrough

…rmer

Places an unsupported `import` statement in the dead branch of a
qd.static(True) / qd.static(False) if-statement. If the transformer
incorrectly walks the dead branch, it raises QuadrantsSyntaxError.
@hughperkins hughperkins marked this pull request as draft March 7, 2026 23:54
@hughperkins
Copy link
Collaborator Author

Opus 4.6 review:

This is a very focused branch — just 1 commit of actual new work (plus a merge commit bringing in latest main). The sole change is a new test in tests/python/test_static.py.
What the test does:
test_static_if_dead_branch_not_walked verifies that when qd.static(True) or qd.static(False) is used in an if statement, the dead (not-taken) branch is not walked by the AST
transformer. It proves this by placing an import os statement in the dead branch — since import is unsupported by the AST transformer and would raise a QuadrantsSyntaxError, the
test would fail if the dead branch were visited. It then asserts the live branch executed correctly.
Thoughts:

  1. Clean and well-scoped. The test is a clear, minimal reproduction that directly targets the behavior it's testing. Using an unsupported AST construct (import) as the canary
    is a clever, lightweight way to verify the dead branch is truly skipped.
  2. Good coverage of both directions. It tests both static(True) (else-branch dead) and static(False) (if-branch dead), which is thorough.
  3. No implementation change. This is a pure test addition — it seems like it's verifying existing behavior rather than accompanying a code fix. That's fine as a regression
    guard, but it's worth confirming: is this documenting already-working behavior, or was there a recent fix on main that this is meant to cover?
  4. Minor nit: The # noqa: F401 comments are a nice touch to suppress unused-import linter warnings.

Overall, it's a solid, targeted test. Ready to merge as-is.

@hughperkins hughperkins marked this pull request as ready for review March 12, 2026 03:23
@hughperkins hughperkins marked this pull request as draft March 12, 2026 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant