Skip to content

Rename duplicate argument variables internally #355

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.

Sign up for GitHub

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

Open
micpap25 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Rename duplicate argument variables internally #355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8012e40
rename duplicate argument variable internally
micpap25 Aug 5, 2026
a66cee5
Implement some suggestions
micpap25 Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/pyqasm/elements.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ def is_internal_qubit_register(qubit_name: str) -> bool:
)


INTERNAL_QUANTUM_ARGUMENT = "__(QUANTUM_ARGUMENT)__"
"""Reserved string used to prevent shadowing for parameters in function definitions.
The register appears as a suffix to a variable name ("variable__(QUANTUM_ARGUMENT)").
A user is not able to define a function parameter with this suffix due to the parentheses.
"""


class InversionOp(Enum):
"""
Enum for specifying the inversion action of a gate.
Expand Down
14 changes: 10 additions & 4 deletions src/pyqasm/subroutines.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from openqasm3.printer import dumps

from pyqasm.analyzer import Qasm3Analyzer
from pyqasm.elements import Variable
from pyqasm.elements import INTERNAL_QUANTUM_ARGUMENT, Variable
from pyqasm.exceptions import ValidationError, raise_qasm3_error
from pyqasm.expressions import Qasm3ExprEvaluator
from pyqasm.transformer import Qasm3Transformer
Expand Down Expand Up @@ -521,6 +521,12 @@ def process_quantum_arg( # pylint: disable=too-many-locals
"""
actual_arg_name = Qasm3SubroutineProcessor.get_fn_actual_arg_name(actual_arg)
formal_reg_name = formal_arg.name.name
internal_reg_name = formal_reg_name
# If our actual variable is the same as the function argument,
# give the function argument a temporary name for internal use
if actual_arg_name == formal_reg_name:

@ryanhill1 ryanhill1 Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_arg can collide with a name already in use:

qubit[2] q;
qubit[2] r;
def f(qubit q, qubit q_arg) { h q; x q_arg; }
f(q[0], r[1]);

main emits h q[0]; x r[1];. Here it's ValueError: Variable 'q_arg' already exists in current scope — and a raw ValueError, not a ValidationError.

Suggestion: use a name that can't appear in user code. INTERNAL_QUBIT_REGISTER in elements.py ("__PYQASM_QUBITS__") is the existing pattern for this.

internal_reg_name += INTERNAL_QUANTUM_ARGUMENT

formal_qubit_size = Qasm3ExprEvaluator.evaluate_expression(
formal_arg.size, reqd_type=IntType, const_expr=True
)[0]
Expand All @@ -536,7 +542,7 @@ def process_quantum_arg( # pylint: disable=too-many-locals
error_node=fn_defn.arguments,
span=formal_arg.span,
)
formal_qreg_size_map[formal_reg_name] = formal_qubit_size
formal_qreg_size_map[internal_reg_name] = formal_qubit_size

# we expect that actual arg is qubit type only
# note that we ONLY check in global scope as
Expand Down Expand Up @@ -603,10 +609,10 @@ def process_quantum_arg( # pylint: disable=too-many-locals
)

for idx, qid in enumerate(resolved_qids):
qubit_transform_map[(formal_reg_name, idx)] = (resolved_reg_name, qid)
qubit_transform_map[(internal_reg_name, idx)] = (resolved_reg_name, qid)

return Variable(
name=formal_reg_name,
name=internal_reg_name,
base_type=QubitDeclaration,
base_size=formal_qubit_size,
dims=None,
Expand Down
22 changes: 18 additions & 4 deletions src/pyqasm/visitor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

from pyqasm.analyzer import Qasm3Analyzer
from pyqasm.elements import (
INTERNAL_QUANTUM_ARGUMENT,
INTERNAL_QUBIT_REGISTER,
Capture,
ClbitDepthNode,
Expand Down Expand Up @@ -1499,11 +1500,24 @@ def _visit_generic_gate_operation( # pylint: disable=too-many-branches, too-man
for transform_map, size_map in zip(
reversed(self._function_qreg_transform_map), reversed(self._function_qreg_size_map)
):
operation.qubits = (
Qasm3Transformer.transform_function_qubits( # type: ignore [assignment]
operation, transform_map, size_map
try:
operation.qubits = (
Qasm3Transformer.transform_function_qubits( # type: ignore [assignment]
operation, transform_map, size_map
)
)
except KeyError:

@ryanhill1 ryanhill1 Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is compensated here, but needed in three places. The rename happens in subroutines.py, so qubit_transform_map is keyed on q_arg while the function body still says q. This try/except patches the mismatch at one use site — but there are three:

visitor.py:754    _visit_reset                    <- not patched
visitor.py:871    _visit_barrier                  <- not patched
visitor.py:1504   _visit_generic_gate_operation

So these still fail, now with a raw KeyError instead of the located ValidationError they gave before:

def f(qubit q) { reset q; }                      // KeyError: ('q', 0)   (barrier too)

def inner(qubit p) { h p; }
def outer(qubit q) { inner(q); }
outer(q[1]);                                     // KeyError: 'q'  — works on main

Suggestion: keep the keys and the body consistent from the start — register the transform map under the name the body uses, or rename the body's references when you rename the parameter. Then no call site needs a retry and all three fix together.

Worth noting too: except KeyError catches it from anywhere inside transform_function_qubits, including genuine bugs.

for qubit in operation.qubits:
# Each qubit may be an IndexedIdentifier or an Identifier
if isinstance(qubit, qasm3_ast.IndexedIdentifier):
qubit.name.name += INTERNAL_QUANTUM_ARGUMENT
else:
qubit.name += INTERNAL_QUANTUM_ARGUMENT
operation.qubits = (
Qasm3Transformer.transform_function_qubits( # type: ignore [assignment]
operation, transform_map, size_map
)
)
)

operation.qubits = self._get_op_bits(operation, qubits=True) # type: ignore

Expand Down
Loading