Add `Operation::blocks` method.

[kevinhartman]

Summary

This gives us a way to get the blocks of a control flow operation as an iterator of CircuitData. If called on a non-control-flow instruction, it returns an empty Vec.

Details and comments

This is not intended as a final API for this, which will likely instead return an iterator of &CircuitData once control flow operations have been fully ported to Rust and own their blocks without requiring conversion.

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core
• @kevinhartman
• @mtreinish

[coveralls]

Pull Request Test Coverage Report for Build 10942872876

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 8 of 29 (27.59%) changed or added relevant lines in 2 files are covered.
• 1890 unchanged lines in 64 files lost coverage.
• Overall coverage decreased (-2.1%) to 87.14%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/circuit/src/packed_instruction.rs	0	3	0.0%
crates/circuit/src/operations.rs	8	26	30.77%
<!--	Total:	8	29	27.59%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit/quantum_info/operators/mixins/multiply.py	1	93.75%
qiskit/pulse/instructions/directives.py	1	97.22%
qiskit/synthesis/one_qubit/one_qubit_decompose.py	1	70.67%
qiskit/quantum_info/operators/mixins/group.py	1	92.59%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	1	88.61%
qiskit/quantum_info/operators/channel/quantum_channel.py	1	72.14%
qiskit/pulse/transforms/alignments.py	1	96.45%
qiskit/quantum_info/operators/mixins/adjoint.py	1	90.0%
qiskit/transpiler/passes/layout/sabre_pre_layout.py	1	98.88%
crates/qasm2/src/lex.rs	1	92.48%
<!--	Total:	1890		-->

Totals
Change from base Build 10598013395:	-2.1%
Covered Lines:	25505
Relevant Lines:	29269

---

💛 - Coveralls

[kevinhartman]

This seems generally fine to me, though given that control_flow is a part of the Operation, does it make sense just to similarly define this somewhere higher up as well, so we need to do less unpacking of the internal objects to get here?

It may. Do you have any thoughts on adding a new variant to OperationRef for control flow? That might not be the right choice for this PR, but I wonder if it'd be a better API long-term to be able to unpack a control flow operation and access its blocks with something like:

if let OperationRef::ControlFlow(op) = op {
    match op.op {
        ControlFlowOp::ForLoop | ControlFlowOp::WhileLoop => { todo!() },
        _ => (),
    }

    // No need to unwrap an option because we know we have a control flow op.
    for block in op.blocks() {
        todo!()
    }
}

I don't think we'd need any support from PackedOperation for this.

[jakelishman]

I'm not sure what you mean by not needing support from PackedOperation - OperationRef is PackedOperation for all intents and purposes; it's just "proper" enum form of a reference to it. (There's space for 4 more variants in PackedOperation without any trouble at all, and the existing three Python forms could be compressed into a single one if we needed to anyway.)

I guess "longer term" depends a lot on the degree of length here. Long long term, I think control-flow operations may want to move out of being in a PackedInstruction at all, and be first-class in the data description (especially in DAGCircuit, less clearly in CircuitData). Shorter term, it could be fine to separate it off, but if the primary reason to do that is to avoid an Option here, maybe we just get rid of the Option in the method and leave the PR as it is? If it isn't a control-flow operation, we can just return an empty iterator, which is an equally valid return value; the caller of this function is presumably already calling Operation::control_flow before anyway.

[kevinhartman]

Long long term, I think control-flow operations may want to move out of being in a PackedInstruction at all, and be first-class in the data description (especially in DAGCircuit, less clearly in CircuitData). Shorter term, it could be fine to separate it off, but if the primary reason to do that is to avoid an Option here, maybe we just get rid of the Option in the method and leave the PR as it is? If it isn't a control-flow operation, we can just return an empty iterator, which is an equally valid return value; the caller of this function is presumably already calling Operation::control_flow before anyway.

Heh, that's all fair. I ended up moving blocks to Operation to make it easy to access in the meantime (hopefully justified in the same way that Operation::control_flow is part of that interface). The return type of blocks is now Vec<CircuitData> (mostly because I think returning an abstract Iterator from a trait method is not stable in Rust), and the caller will get an empty Vec if the operation does not have any blocks for any valid reason.