Qiskit / qiskit / 24097557231

88%

main: 88%

Build Type

push

github

Committed by web-flow

Commit Message

Rewrite QPY `ParameterExpression` handling in pure Polish form (#15934) (#15961)

* Rewrite QPY `ParameterExpression` handling in pure Polish form

This patch is a fairly invasive change, but one that fixes the known
edge cases of replay generation from `ParameterExpression` itself.  This
fixes several `pickle`, `copy`/`deepcopy` and QPY bugs around
`ParameterExpression`.

The Python-space QPY loader is modified in a small way to allow it to
handle QPY-format permitted "replay" elements that act between two bare
values; this was always allowed by the QPY spec, but couldn't be
generated by Python-space QPY/`ParameterExpression`.  Doing so actually
significantly simplifies the loading logic, since it no longer needs to
reflect certain arithmetic operations.  The new Rust-space
`ParameterExpression` uses binary operations with two numeric operands
(e.g. `Add(2, 0)`) to safely propagate bare values through the QPY
replay.  This was not previously necessary in Python space because there
was no public interface to create a bare-numeric `ParameterExpression`
without replayable expressions having been tracked.  Similarly,
cancelled-out symbols are propagated in the replay with `Sub(x, x)`
operations, which already must work across all compliant QPY loaders,
and both Rust- and Python-space `ParameterExpression.

History
-------

The QPY replay was _intended_ to be a pure Polish-notation
representation of the operations to take to rebuild the expression.
This mostly worked fine at the time, but the patch was written under a
lot of stress and without full CI tooling, due to it being in response
to a security bug.  This led to some less-than-ideal parts of the format
specification, such as the start/end recursion opcodes in the QPY format
not actually being necessary; they are actually zero-operand no-ops to
the stack, but this wasn't noticed during the patch due to time
pressure.

Further, as `ParameterExpression` moved to Rust, the tracked internal
"replay" lis... (continued)

Coverage Stats

167 of 262 new or added lines in 4 files covered. (63.74%)

39 existing lines in 5 files now uncovered.

104002 of 118732 relevant lines covered (87.59%)

1002446.19 hits per line