-
Notifications
You must be signed in to change notification settings - Fork 19
Add ScopeManager
#232
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 ScopeManager
#232
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Pull Request Overview
This PR introduces a dedicated ScopeManager to centralize variable scope and context handling, and refactors the existing QasmVisitor to delegate all scope operations to it. Key changes include:
- Addition of
ScopeManagerclass with push/pop scope and context methods. - Refactoring of
QasmVisitor,transformer.py, andsubroutines.pyto replace in-class scope logic with calls toScopeManager. - Updates to
Variablemodel, tests, module instantiation, and changelog to support the new scoping system.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pyqasm/scope.py | New ScopeManager class for managing variable scopes/contexts. |
| src/pyqasm/visitor.py | Replaced inline scope logic with ScopeManager calls. |
| src/pyqasm/transformer.py | Updated to use ScopeManager in transform_function_qubits. |
| src/pyqasm/subroutines.py | Refactored subroutine argument checks to use ScopeManager. |
| src/pyqasm/elements.py | Extended Variable dataclass with shadow, is_alias, is_qubit. |
| src/pyqasm/expressions.py | Switched to scope_manager methods for scope checks. |
| src/pyqasm/modules/base.py | Now instantiates QasmVisitor with a ScopeManager. |
| src/pyqasm/init.py | Documented LoopLimitExceededError in autosummary. |
| CHANGELOG.md | Added entry for scoping refactor. |
| tests/qasm3/test_alias.py | Updated expected error message after scope refactor. |
| tests/qasm3/subroutines/test_subroutines.py | Renamed test parameters and added a skipped test for qubit renaming. |
Comments suppressed due to low confidence (3)
src/pyqasm/elements.py:95
- The docstring refers to
is_register, but the attribute was renamed tois_qubit. Update the docstring to match the new attribute name.
is_register (bool): Flag indicating if the variable is a register.
tests/qasm3/subroutines/test_subroutines.py:303
- [nitpick] This test is marked as skipped, creating a coverage gap for the qubit renaming behavior. Instead of skipping, consider fixing the underlying bug or adding a TODO and tracking in an issue so the test can run against the intended behavior.
@pytest.mark.skip(reason="Bug: qubit in function scope conflicts with global scope")
src/pyqasm/init.py:48
- The autosummary lists
LoopLimitExceededErrorbut it is not imported in this module; addfrom pyqasm.exceptions import LoopLimitExceededErroror remove it from the summary to prevent documentation build errors.
LoopLimitExceededError
Fixes #230
Summary of changes
ScopeManagerclass to handle variable scopes and contexts, including methods for pushing/popping scopes, checking variable visibility, and updating variable values. (src/pyqasm/scope_manager.py)QasmVisitorclass to use theScopeManagerfor all scope-related operations, replacing its internal scope and context management logic. (src/pyqasm/visitor.py) [1] [2] [3]aliasbug