Ensure QuantumCircuit.append validates captures in control-flow (#10974)

* Add definition of `Store` instruction

This does not yet add the implementation of `QuantumCircuit.store`,
which will come later as part of expanding the full API of
`QuantumCircuit` to be able to support these runtime variables.

The `is_lvalue` helper is added generally to the `classical.expr` module
because it's generally useful, while `types.cast_kind` is moved from
being a private method in `expr` to a public-API function so `Store` can
use it.  These now come with associated unit tests.

* Add variable-handling methods to `QuantumCircuit`

This adds all the new `QuantumCircuit` methods discussed in the
variable-declaration RFC[^1], and threads the support for them through
the methods that are called in turn, such as `QuantumCircuit.append`.
It does yet not add support to methods such as `copy` or `compose`,
which will be done in a follow-up.

The APIs discussed in the RFC necessitated making `Var` nodes hashable.
This is done in this commit, as it is logically connected.  These nodes
now have enforced immutability, which is technically a minor breaking
change, but in practice required for the properties of such expressions
to be tracked correctly through circuits.

A helper attribute `Var.standalone` is added to unify the handling of
whether a variable is an old-style existing-memory wrapper, or a new
"proper" variable with its own memory.

[^1]: Qiskit/RFCs#50

* Support manual variables `QuantumCircuit` copy methods

This commit adds support to the `QuantumCircuit` methods `copy` and
`copy_empty_like` for manual variables.  This involves the non-trivial
extension to the original RFC[^1] that variables can now be
uninitialised; this is somewhat required for the logic of how the
`Store` instruction works and the existence of
`QuantumCircuit.copy_empty_like`; a variable could be initialised with
the result of a `measure` that no longer exists, therefore it must be
possible for variables to be uninitialised.

This was not originally intended to be possible in the design document,
but is somewhat required for logical consistency.  A method
`add_uninitialized_var` is added, so that the behaviour of
`copy_empty_like` is not an awkward special case only possible through
that method, but instead a complete part of the data model that must be
reasoned about.  The method however is deliberately a bit less
ergononmic to type and to use, because really users _should_ use
`add_var` in almost all circumstances.

[^1]: Qiskit/RFCs#50

* Ensure `QuantumCircuit.append` validates captures in control-flow

This adds an inner check to the control-flow operations that their
blocks do not contain input variables, and to `QuantumCircuit.append`
that any captures within blocks are validate (in the sense of the
variables existing in the outer circuit).

In order to avoid an `import` on every call to `QuantumCircuit.append`
(especially since we're already eating the cost of an extra
`isinstance` check), this reorganises the import structure of
`qiskit.circuit.controlflow` to sit strictly _before_
`qiskit.circuit.quantumcircuit` in the import tree.  Since those are key
parts of the circuit data structure, that does make sense, although by
their nature the structures are of course recursive at runtime.

* Update documentation

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Catch simple error case in '_prepare_new_var'

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Add partial release note

---------

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

Author: jakelishman

qiskit/circuit/__init__.py

@@ -279,6 +279,7 @@
    InstructionSet
    Operation
    EquivalenceLibrary
+   Store
 
 Control Flow Operations
 -----------------------
@@ -375,6 +376,7 @@
 from .delay import Delay
 from .measure import Measure
 from .reset import Reset
+from .store import Store
 from .parameter import Parameter
 from .parametervector import ParameterVector
 from .parameterexpression import ParameterExpression

qiskit/circuit/classical/expr/__init__.py

@@ -160,6 +160,11 @@
 suitable "key" functions to do the comparison.
 
 .. autofunction:: structurally_equivalent
+
+Some expressions have associated memory locations, and others may be purely temporary.
+You can use :func:`is_lvalue` to determine whether an expression has an associated memory location.
+
+.. autofunction:: is_lvalue
 """
 
 __all__ = [
@@ -172,6 +177,7 @@
     "ExprVisitor",
     "iter_vars",
     "structurally_equivalent",
+    "is_lvalue",
     "lift",
     "cast",
     "bit_not",
@@ -191,7 +197,7 @@
 ]
 
 from .expr import Expr, Var, Value, Cast, Unary, Binary
-from .visitors import ExprVisitor, iter_vars, structurally_equivalent
+from .visitors import ExprVisitor, iter_vars, structurally_equivalent, is_lvalue
 from .constructors import (
     lift,
     cast,

qiskit/circuit/classical/expr/constructors.py

@@ -35,65 +35,27 @@
     "lift_legacy_condition",
 ]
 
-import enum
 import typing
 
 from .expr import Expr, Var, Value, Unary, Binary, Cast
+from ..types import CastKind, cast_kind
 from .. import types
 
 if typing.TYPE_CHECKING:
     import qiskit
 
 
-class _CastKind(enum.Enum):
-    EQUAL = enum.auto()
-    """The two types are equal; no cast node is required at all."""
-    IMPLICIT = enum.auto()
-    """The 'from' type can be cast to the 'to' type implicitly.  A ``Cast(implicit=True)`` node is
-    the minimum required to specify this."""
-    LOSSLESS = enum.auto()
-    """The 'from' type can be cast to the 'to' type explicitly, and the cast will be lossless.  This
-    requires a ``Cast(implicit=False)`` node, but there's no danger from inserting one."""
-    DANGEROUS = enum.auto()
-    """The 'from' type has a defined cast to the 'to' type, but depending on the value, it may lose
-    data.  A user would need to manually specify casts."""
-    NONE = enum.auto()
-    """There is no casting permitted from the 'from' type to the 'to' type."""
-
-
-def _uint_cast(from_: types.Uint, to_: types.Uint, /) -> _CastKind:
-    if from_.width == to_.width:
-        return _CastKind.EQUAL
-    if from_.width < to_.width:
-        return _CastKind.LOSSLESS
-    return _CastKind.DANGEROUS
-
-
-_ALLOWED_CASTS = {
-    (types.Bool, types.Bool): lambda _a, _b, /: _CastKind.EQUAL,
-    (types.Bool, types.Uint): lambda _a, _b, /: _CastKind.LOSSLESS,
-    (types.Uint, types.Bool): lambda _a, _b, /: _CastKind.IMPLICIT,
-    (types.Uint, types.Uint): _uint_cast,
-}
-
-
-def _cast_kind(from_: types.Type, to_: types.Type, /) -> _CastKind:
-    if (coercer := _ALLOWED_CASTS.get((from_.kind, to_.kind))) is None:
-        return _CastKind.NONE
-    return coercer(from_, to_)
-
-
 def _coerce_lossless(expr: Expr, type: types.Type) -> Expr:
     """Coerce ``expr`` to ``type`` by inserting a suitable :class:`Cast` node, if the cast is
     lossless.  Otherwise, raise a ``TypeError``."""
-    kind = _cast_kind(expr.type, type)
-    if kind is _CastKind.EQUAL:
+    kind = cast_kind(expr.type, type)
+    if kind is CastKind.EQUAL:
         return expr
-    if kind is _CastKind.IMPLICIT:
+    if kind is CastKind.IMPLICIT:
         return Cast(expr, type, implicit=True)
-    if kind is _CastKind.LOSSLESS:
+    if kind is CastKind.LOSSLESS:
         return Cast(expr, type, implicit=False)
-    if kind is _CastKind.DANGEROUS:
+    if kind is CastKind.DANGEROUS:
         raise TypeError(f"cannot cast '{expr}' to '{type}' without loss of precision")
     raise TypeError(f"no cast is defined to take '{expr}' to '{type}'")
 
@@ -198,7 +160,7 @@ def cast(operand: typing.Any, type: types.Type, /) -> Expr:
             Cast(Value(5, types.Uint(32)), types.Uint(8), implicit=False)
     """
     operand = lift(operand)
-    if _cast_kind(operand.type, type) is _CastKind.NONE:
+    if cast_kind(operand.type, type) is CastKind.NONE:
         raise TypeError(f"cannot cast '{operand}' to '{type}'")
     return Cast(operand, type)
 

qiskit/circuit/classical/expr/expr.py

@@ -115,38 +115,57 @@ class Var(Expr):
     associated name; and an old-style variable that wraps a :class:`.Clbit` or
     :class:`.ClassicalRegister` instance that is owned by some containing circuit.  In general,
     construction of variables for use in programs should use :meth:`Var.new` or
-    :meth:`.QuantumCircuit.add_var`."""
+    :meth:`.QuantumCircuit.add_var`.
+
+    Variables are immutable after construction, so they can be used as dictionary keys."""
 
     __slots__ = ("var", "name")
 
+    var: qiskit.circuit.Clbit | qiskit.circuit.ClassicalRegister | uuid.UUID
+    """A reference to the backing data storage of the :class:`Var` instance.  When lifting
+    old-style :class:`.Clbit` or :class:`.ClassicalRegister` instances into a :class:`Var`,
+    this is exactly the :class:`.Clbit` or :class:`.ClassicalRegister`.  If the variable is a
+    new-style classical variable (one that owns its own storage separate to the old
+    :class:`.Clbit`/:class:`.ClassicalRegister` model), this field will be a :class:`~uuid.UUID`
+    to uniquely identify it."""
+    name: str | None
+    """The name of the variable.  This is required to exist if the backing :attr:`var` attribute
+    is a :class:`~uuid.UUID`, i.e. if it is a new-style variable, and must be ``None`` if it is
+    an old-style variable."""
+
     def __init__(
         self,
         var: qiskit.circuit.Clbit | qiskit.circuit.ClassicalRegister | uuid.UUID,
         type: types.Type,
         *,
         name: str | None = None,
     ):
-        self.type = type
-        self.var = var
-        """A reference to the backing data storage of the :class:`Var` instance.  When lifting
-        old-style :class:`.Clbit` or :class:`.ClassicalRegister` instances into a :class:`Var`,
-        this is exactly the :class:`.Clbit` or :class:`.ClassicalRegister`.  If the variable is a
-        new-style classical variable (one that owns its own storage separate to the old
-        :class:`.Clbit`/:class:`.ClassicalRegister` model), this field will be a :class:`~uuid.UUID`
-        to uniquely identify it."""
-        self.name = name
-        """The name of the variable.  This is required to exist if the backing :attr:`var` attribute
-        is a :class:`~uuid.UUID`, i.e. if it is a new-style variable, and must be ``None`` if it is
-        an old-style variable."""
+        super().__setattr__("type", type)
+        super().__setattr__("var", var)
+        super().__setattr__("name", name)
 
     @classmethod
     def new(cls, name: str, type: types.Type) -> typing.Self:
         """Generate a new named variable that owns its own backing storage."""
         return cls(uuid.uuid4(), type, name=name)
 
+    @property
+    def standalone(self) -> bool:
+        """Whether this :class:`Var` is a standalone variable that owns its storage location.  If
+        false, this is a wrapper :class:`Var` around a pre-existing circuit object."""
+        return isinstance(self.var, uuid.UUID)
+
     def accept(self, visitor, /):
         return visitor.visit_var(self)
 
+    def __setattr__(self, key, value):
+        if hasattr(self, key):
+            raise AttributeError(f"'Var' object attribute '{key}' is read-only")
+        raise AttributeError(f"'Var' object has no attribute '{key}'")
+
+    def __hash__(self):
+        return hash((self.type, self.var, self.name))
+
     def __eq__(self, other):
         return (
             isinstance(other, Var)
@@ -160,6 +179,23 @@ def __repr__(self):
             return f"Var({self.var}, {self.type})"
         return f"Var({self.var}, {self.type}, name='{self.name}')"
 
+    def __getstate__(self):
+        return (self.var, self.type, self.name)
+
+    def __setstate__(self, state):
+        var, type, name = state
+        super().__setattr__("type", type)
+        super().__setattr__("var", var)
+        super().__setattr__("name", name)
+
+    def __copy__(self):
+        # I am immutable...
+        return self
+
+    def __deepcopy__(self, memo):
+        # ... as are all my consituent parts.
+        return self
+
 
 @typing.final
 class Value(Expr):

qiskit/circuit/classical/expr/visitors.py

@@ -215,3 +215,66 @@ def structurally_equivalent(
             True
     """
     return left.accept(_StructuralEquivalenceImpl(right, left_var_key, right_var_key))
+
+
+class _IsLValueImpl(ExprVisitor[bool]):
+    __slots__ = ()
+
+    def visit_var(self, node, /):
+        return True
+
+    def visit_value(self, node, /):
+        return False
+
+    def visit_unary(self, node, /):
+        return False
+
+    def visit_binary(self, node, /):
+        return False
+
+    def visit_cast(self, node, /):
+        return False
+
+
+_IS_LVALUE = _IsLValueImpl()
+
+
+def is_lvalue(node: expr.Expr, /) -> bool:
+    """Return whether this expression can be used in l-value positions, that is, whether it has a
+    well-defined location in memory, such as one that might be writeable.
+
+    Being an l-value is a necessary but not sufficient for this location to be writeable; it is
+    permissible that a larger object containing this memory location may not allow writing from
+    the scope that attempts to write to it.  This would be an access property of the containing
+    program, however, and not an inherent property of the expression system.
+
+    Examples:
+        Literal values are never l-values; there's no memory location associated with (for example)
+        the constant ``1``::
+
+            >>> from qiskit.circuit.classical import expr
+            >>> expr.is_lvalue(expr.lift(2))
+            False
+
+        :class:`~.expr.Var` nodes are always l-values, because they always have some associated
+        memory location::
+
+            >>> from qiskit.circuit.classical import types
+            >>> from qiskit.circuit import Clbit
+            >>> expr.is_lvalue(expr.Var.new("a", types.Bool()))
+            True
+            >>> expr.is_lvalue(expr.lift(Clbit()))
+            True
+
+        Currently there are no unary or binary operations on variables that can produce an l-value
+        expression, but it is likely in the future that some sort of "indexing" operation will be
+        added, which could produce l-values::
+
+            >>> a = expr.Var.new("a", types.Uint(8))
+            >>> b = expr.Var.new("b", types.Uint(8))
+            >>> expr.is_lvalue(a) and expr.is_lvalue(b)
+            True
+            >>> expr.is_lvalue(expr.bit_and(a, b))
+            False
+    """
+    return node.accept(_IS_LVALUE)

qiskit/circuit/classical/types/__init__.py

@@ -15,6 +15,8 @@
 Typing (:mod:`qiskit.circuit.classical.types`)
 ==============================================
 
+Representation
+==============
 
 The type system of the expression tree is exposed through this module.  This is inherently linked to
 the expression system in the :mod:`~.classical.expr` module, as most expressions can only be
@@ -41,11 +43,18 @@
 Note that :class:`Uint` defines a family of types parametrised by their width; it is not one single
 type, which may be slightly different to the 'classical' programming languages you are used to.
 
+
+Working with types
+==================
+
 There are some functions on these types exposed here as well.  These are mostly expected to be used
 only in manipulations of the expression tree; users who are building expressions using the
 :ref:`user-facing construction interface <circuit-classical-expressions-expr-construction>` should
 not need to use these.
 
+Partial ordering of types
+-------------------------
+
 The type system is equipped with a partial ordering, where :math:`a < b` is interpreted as
 ":math:`a` is a strict subtype of :math:`b`".  Note that the partial ordering is a subset of the
 directed graph that describes the allowed explicit casting operations between types.  The partial
@@ -66,6 +75,20 @@
 .. autofunction:: is_subtype
 .. autofunction:: is_supertype
 .. autofunction:: greater
+
+
+Casting between types
+---------------------
+
+It is common to need to cast values of one type to another type.  The casting rules for this are
+embedded into the :mod:`types` module.  You can query the casting kinds using :func:`cast_kind`:
+
+.. autofunction:: cast_kind
+
+The return values from this function are an enumeration explaining the types of cast that are
+allowed from the left type to the right type.
+
+.. autoclass:: CastKind
 """
 
 __all__ = [
@@ -77,7 +100,9 @@
     "is_subtype",
     "is_supertype",
     "greater",
+    "CastKind",
+    "cast_kind",
 ]
 
 from .types import Type, Bool, Uint
-from .ordering import Ordering, order, is_subtype, is_supertype, greater
+from .ordering import Ordering, order, is_subtype, is_supertype, greater, CastKind, cast_kind